Emperor Napoleon Posted July 23, 2013 Report Share Posted July 23, 2013 I wanted to decode a quadrature rotary signal, so I started with the quadrature decoder code from opencores http://opencores.org/project,quadraturecountI was never able to get that code to count, even after ruling out other sources of problems So I modified the code in a way that made sense to myself, it turned out much shorter then the original and best of all it works correctly Yes, It's not enough that it works, I need outside confirmation that I did not do something bad. My question is, did I get lucky, and am I ignoring possible timing issues, race conditions?Under what conditions can I know that writing to a signal visible to outside circuits is safe? My head kind of hurts from thinking about it. Code follows, thankslibrary IEEE;use IEEE.STD_LOGIC_1164.ALL;use IEEE.STD_LOGIC_ARITH.ALL;use IEEE.STD_LOGIC_SIGNED.ALL;library work;--How we 'talk' to the outside world:entity QuadratureCounterPorts is Port ( clock : in std_logic; --system clock, i.e. 10MHz oscillator QuadA : in std_logic; --first input from quadrature device (i.e. optical disk encoder) QuadB : in std_logic; --second input from quadrature device (i.e. optical disk encoder) CounterValue : out std_logic_vector(23 downto 0); --just an example debuggin output setCount : in std_logic_vector(23 downto 0); writeEnable : in std_logic );end QuadratureCounterPorts;--What we 'do':architecture behave of QuadratureCounterPorts is signal Count : unsigned(23 downto 0); signal clockD : std_logic; signal Quad : std_logic_vector(1 downto 0); signal lastQuad : std_logic_vector(1 downto 0); function gray(a:std_logic_vector) return std_logic_vector isbegin if a="00" then return "00"; elsif a="10" then return "01"; elsif a="11" then return "10"; elsif a="01" then return "11"; end if; return "11";end function; begin --architecture QuadratureCounter -- do our actual work every clock cycle process(clock) begin --keep track of the counter if ( rising_edge(clock) ) then lastQuad <= Quad; Quad <= gray((0=>QuadA,1=>QuadB)); if((unsigned(Quad) = unsigned(lastQuad)+1)) then Count <= Count + 1; end if; if((unsigned(Quad) = unsigned(lastQuad)-1)) then Count <= Count - 1; end if; if(writeEnable = '1') then --Count <= unsigned(setCount); end if; end if; --clock'event CounterValue <= std_logic_vector(Count); end process; --(clock) end behave; Link to comment Share on other sites More sharing options...
alvieboy Posted July 23, 2013 Report Share Posted July 23, 2013 It could help if you post a simulation result, and a synthesis report... I assume the issue with OpenCores design is yours, not the design. Those designs are most of the time tested and proved right. I don't understand your code at all, not the implementation (despite it might work). You also don't seem to debounce your inputs: it's this done on purpose ? You also use double-buffer for the output (counter and countervalue are both flipflops). Link to comment Share on other sites More sharing options...
Emperor Napoleon Posted July 23, 2013 Author Report Share Posted July 23, 2013 Ok let me explain.So quadrature is like a 2 bit gray code. Function gray() decodes the gray code into an int. Thus decoded values 0,1,2,3 correspond to sequential phases in the progression of a quadrature signal. Every clock cycle I read the value of the 2 quadrature signals. I set "quad" to the decoded current value. Then I set "lastQuad" to the value of "quad" on the previous cycle. By comparing the current value to the last value, i can see if the quadrature signal has progressed forward or backward. If quad = lastQuad+1, then it has moved forwardIf quad = lastQuad-1 then if has moved backward I don't think debouncing is necessary since I only read the signal values once a clock cycle. Will this really buffer the output a single clock cycle? I thought within a process any values written are immediately visible to following statements? thanks... Link to comment Share on other sites More sharing options...
hamster Posted July 23, 2013 Report Share Posted July 23, 2013 I've got a PMODENC (http://www.digilentinc.com/Products/Detail.cfm?Prod=PMOD-ENC) in the mail, so this is quite timely for me! If you have a problem could be that you haven't denounced and synchronised the inputs. At first glace this shouldn't be a problem , as the encoder is using grey code any jitter/bounce shouldn't cause an error, just make the count value bounce up and down for a few ms. What will be a problem is that "quad" is tested in multiple places. If you look in the FPGA viewer you will see that the incoming signal gets buffered then distributed to multiple CLBs. This can lead to odd effects. Say the latches for "LastQuad" are further away from the buffer than the logic for "unsigned(Quad) = unsigned(lastQuad)+1". If "Quad" is in transition then "count" might get updated but lastQuad latches the old value. This will cause double counting. Alternatively if the latches for "LastQuad" are closer than the logic for "unsigned(Quad) = unsigned(lastQuad)+1" then LastQuad might get updated but the value for "Count" may not. This will cause counts to get skipped. How likely this is depends on the different path delays and the clock speed of the design. It would be an interesting experiment to run your design at the highest clock speed that meets timing, and see how often it fails (e.g. If you know that 0 degrees should have a count of "0" then spin the encoder and see how often it gets out of sync). The really big problem that occurs if you don't synchronise your signals. If the input signal changes just before the value of "count" is latched you can attempt to capture the value for count while signals are still propagating - that could cause 'count' to go from say "111111111111111111111111" to "111100000000000000000000". That might not be a big problem in most cases, but you might not want it to happen on the autopilot for your jet :-) It would be an interesting experiment to see if you could cause that failure too.... As purely a matter of style, perhaps it would be good move the "lastQuad <= quad;" statement to the end of the "if" block. That takes the "delayed assignment" of lastQuad out of play. Also as a matter of style (and hopefully discussion), do you think that converting the quadrature values into numeric values actually makes sense? Maybe soemthing like case lastQuad is when "00" => up <= quad(0); down <= quad(1); when "01" => up <= quad(1); down <= not quad(0); when "11" => up <= not quad(0); down <= not quad(0); others => up <= not quad(1); down <= quad(0);end case; And then act on 'up' and 'down' to update the counter. That way you can even get it right should both signals change at the same time for some weird reason. It just occurred to me that you can also un-grey the value of quad using logic "ungrey <= quadA & (quadB xor quadA);"... Link to comment Share on other sites More sharing options...
Emperor Napoleon Posted July 24, 2013 Author Report Share Posted July 24, 2013 Thanks hamster, I have read your comments several times now, I think it makes sense. If I understand right, the only thing missing is that reading the inputs should be synchronized, latched, and then processed in the next clock cycle. I'm not particularly concerned about the case when both input signals change in the same cycle, it would seem to indicate the encoder is either spinning too fast (not really possible) or falling apart. Yeah, the decoding the "gray code" into an int is just what made sense to me Link to comment Share on other sites More sharing options...
hamster Posted July 24, 2013 Report Share Posted July 24, 2013 If I understand right, the only thing missing is that reading the inputs should be synchronized, latched, and then processed in the next clock cycle. Yeah, that about sums it up - and uses far less words :-) Link to comment Share on other sites More sharing options...
hamster Posted July 30, 2013 Report Share Posted July 30, 2013 I got my encoder, and did it this way:------------------------------------------------------------------------------------ Engineer: Mike Field <hamster@snap.net.nz>-- -- Module Name: pmodenc - Behavioral -- Description: Process the quadrature signals from the rotary encoder on -- the Digilent PMODENC ------------------------------------------------------------------------------------library IEEE;use IEEE.STD_LOGIC_1164.ALL;use IEEE.NUMERIC_STD.ALL;entity pmodenc is Port ( clk : in STD_LOGIC; quad : in STD_LOGIC_VECTOR (1 downto 0); value : out STD_LOGIC_VECTOR (7 downto 0));end pmodenc;architecture Behavioral of pmodenc is signal sr : std_logic_vector(7 downto 0) := (others => '0'); signal count : unsigned(7 downto 0) := (others => '0');begin value <= std_logic_vector(count);process(clk) begin if rising_edge(clk) then case sr(3 downto 0) is when "0100" => count <= count+1; when "1101" => count <= count+1; when "1011" => count <= count+1; when "0010" => count <= count+1; when "1000" => count <= count-1; when "1110" => count <= count-1; when "0111" => count <= count-1; when "0001" => count <= count-1; when others => end case; sr <= quad & sr(sr'high downto 2); end if; end process;end Behavioral; Link to comment Share on other sites More sharing options...
Emperor Napoleon Posted July 30, 2013 Author Report Share Posted July 30, 2013 Not that it really matters but I think "sr" is longer than it needs to be.. Right? Link to comment Share on other sites More sharing options...
hamster Posted July 30, 2013 Report Share Posted July 30, 2013 Not longer than it needs to be, it is just long enough, It is done deliberately. I get a two stage synchroniser for zero more code, avoiding all the ugly timing problems that would occur if the inputs change at the wrong time. Link to comment Share on other sites More sharing options...
alex Posted July 30, 2013 Report Share Posted July 30, 2013 I just run a simulation with sr being 4 bits and it seemed to work fine. Essentially sr holds the current and previous quadrature bits (2+2) EDIT: nevermind I see what you're doing. This introduces a sort of hysteresis when you reverse the rotation direction (several clicks before the counter starts moving the other way). Link to comment Share on other sites More sharing options...
hamster Posted July 30, 2013 Report Share Posted July 30, 2013 Four bits will work fine in simulation - but thinking about it a little more this might actually be two bits too long. async signal quad(1) -> register sr(7) -> register sr(5) -> register sr(3) -> logic.async signal quad(0) -> register sr(6) -> register sr(4) -> register sr(2) -> logic. Yeah, it is two bits longer than needed : Link to comment Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.