I have been trying to design an RS-232 receiver taking an FSM approach. I will admit that I do not have a very well-rounded understanding of VHDL, so I have been working on the code on the fly and learning as I go. However, I believe I've hit a brick wall at this point.
My issue is that I have two processes in my code, one to trigger the next state and the other to perform the combinational logic. My code is as follows:
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
entity ASyncReceiverV4 is
Port ( DataIn : in STD_LOGIC;
Enable : in STD_LOGIC;
CLK : in STD_LOGIC;
BadData : out STD_LOGIC;
DataOut : out STD_LOGIC_VECTOR (7 downto 0));
end ASyncReceiverV4;
architecture Behavioral of ASyncReceiverV4 is
type states is (StartBitCheck, ReadData, StopBitCheck);
signal currentState, nextState : states;
begin
process(CLK)
begin
if rising_edge(CLK) then
currentState <= nextState;
end if;
end process;
process(CLK)
variable counter : integer := 0;
variable dataIndex : integer := 0;
begin
case currentState is
when StartBitCheck =>
if Enable = '1' then
if (DataIn = '0' and counter < 8) then
counter := counter + 1;
elsif (DataIn = '0' and counter = 8) then
BadData <= '0';
nextState <= ReadData;
counter := 0;
else
nextState <= StartBitCheck;
end if;
end if;
when ReadData =>
if Enable = '1' then
if counter < 16 then
counter := counter + 1;
elsif (counter = 16 and dataIndex < 8) then
DataOut(dataIndex) <= DataIn;
counter := 0;
dataIndex := dataIndex + 1;
elsif dataIndex = 8 then
dataIndex := 0;
nextState <= StopBitCheck;
else
nextState <= ReadData;
end if;
end if;
when StopBitCheck =>
if Enable = '1' then
if DataIn = '1' then
if counter < 16 then
counter := counter + 1;
nextState <= StopBitCheck;
elsif counter = 16 then
counter := 0;
nextState <= StartBitCheck;
end if;
else
DataOut <= "11111111";
BadData <= '1';
nextState <= StartBitCheck;
end if;
end if;
end case;
end process;
end Behavioral;
For whatever reason, based on my simulations, it seems that my processes are out of sync. Although things are only supposed to occur at the rising edge of the clock, I have transitions occurring at the falling edge. Furthermore, it seems like things are not changing according to the value of counter.
The Enable input is high in all of my simulations. However, this is just to keep it simple for now, it will eventually be fed the output of a 153,600 Baud generator (the Baud generator will be connected to the Enable input). Hence, I only want things to change when my Baud generator is high. Otherwise, do nothing. Am I taking the right approach for that with my code?
I can supply a screenshot of my simulation if that would be helpful. I am also not sure if I am including the correct variables in my process sensitivity list. Also, am I taking the right approach with my counter and dataIndex variables? What if I made them signals as part of my architecture before any of my processes?
Any help on this would be very much so appreciated!
The easiest way to fix this, while also producing the easiest to read code, would be to move to a 1-process state machine like so (warning: not complied may contain syntax errors):
Note that while this no longer contains language issues, there are still odd things in the logic.
You cannot enter the state
StopBitCheck
untilcounter
is 16 because the state transition is in an elsif ofcounter < 16
. Therefore, theif counter < 16
inStopBitCheck
is unreachable.Also note that the state transition to
StopBitCheck
happens on the same cycle that you would normally sample data, so the sampling ofDataIn
withinStopBitCheck
will be a cycle late. Worse, were you ever to get bad data (DataIn/='1'
inStopBitCheck
),counter
would still be at 16,StartBitCheck
would always go to the else clause, and the state machine would lock up.Some more explanation on what was wrong before:
Your simulation has things changing on the negative clock edge because your combinatorial process only has the clock on the sensitivity list. The correct sensitivity list for the combinatorial process would include only
DataIn
,Enable
,currentState
, and your two variables,counter
anddataIndex
. Variables can't be a part of your sensitivity list because they are out of scope for the sensitivity list (also you do not want to trigger your process off of itself, more on that in a moment).The sensitivity list is, however, mostly just a crutch for simulators. When translated to real hardware, processes are implemented in LUTs and Flip Flops. Your current implementation will never synthesize because you incorporate feedback (signals or variables that get assigned a new value as a function of their old value) in unclocked logic, producing a combinatorial loop.
counter
anddataIndex
are part of your state data. The state machine is simpler to understand because they are split off from the explicit state, but they are still part of the state data. When you make a two process state machine (again, I recommend 1 process state machines, not 2) you must split all state data into Flip Flops used to store it (such ascurrentState
) and the output of the LUT that generates the next value (such asnextState
). This means that for your two process machine, the variablescounter
anddataIndex
must instead becomecurrentCounter
,nextCounter
,currentDataIndex
, andnextDataIndex
and treated like your state assignment. Note that if you choose to implement changes to keep a 2-process state machine, the logic errors mentioned above the line will still apply.EDIT:
Yes, resetting the
counter
back to 0 before moving intoStopBitCheck
might be a good idea, but you also need to consider that you are waiting the full 16 counts after sampling the last data bit before you even transition intoStopBitCheck
. It is only becausecounter
is not reset that the sample is only off by one clock instead of 16. You may want to modify yourReadData
action to transition toStopBitCheck
as you sample the last bit when dataIndex=7 (as well as resetcounter
to 0) like so:A pure state machine only stores a state. It generates its output purely from the state, or from a combination of the state and the inputs. Because
counter
anddataIndex
are stored, they are part of the state. If you wanted to enumerate every single state you would have something like:and you would end up with 8*16*3 = 384 states (actually somewhat less because only
ReadData
usesdataIndex
, so some of the 384 are wholly redundant states). As you can no doubt see, it is much simpler to just declare 3 separate signals since the different parts of the state data are used differently. In a two process state machine, people often forget that the signal actually namedstate
isn't the only state data that needs to be stored in the clocked process.In a 1-process machine, of course, this isn't an issue because everything that is assigned is by necessity stored in flip flops (the intermediary combinational logic isn't enumerated in signals). Also, do note that 1-process state machines and 2-process state machines will synthesize to the same thing (assuming they were both implemented correctly), although I'm of the opinion that is comparatively easier to read and more difficult to mess up a 1-process machine.
I also removed the else clauses that maintained the current state assignment in the 1-process example I provided; they are important when assigning a combinational
nextState
, but the clocked signalstate
will keep its old value whenever it isn't given a new assignment without inferring a latch.