Forum Discussion

Altera_Forum's avatar
Altera_Forum
Icon for Honored Contributor rankHonored Contributor
18 years ago

Problems with FSM's and Counters

I'm having some major problems with this VHDL code. It's supposed to simulate an intersection traffic lights but the behavior that the waveform exhibits doesn't make any sense for the simplest cases. All it should do is alternate between the states for the amount of time given. 16 clock cycles for all of NS split among two states and 8 clock cycles for all of EW split among two states. The waveform behavior is not correct at all, certain states trigger when they shouldn't ie I should never have a Red light for NS and a green light for NS at the same time. 

LIBRARY ieee;
USE ieee.std_logic_1164.all;
USE ieee.numeric_std.all ;
ENTITY TrafficLight2 IS
	PORT(NS,EW,NSL,EWL: IN STD_LOGIC_VECTOR(1 DOWNTO 0);--cars that want to go in those directions
		 Clk,Reset: IN STD_LOGIC;
		 GNS,GEW,RNS,REW,GNSL,GEWL: OUT STD_LOGIC);--GNS = greens for North and South, GEW = greens for East and West, GNSL = greens for North and South left lanes, GEWL = greens for East and Weset left lanes
END TrafficLight2;
ARCHITECTURE Behavior OF TrafficLight2 IS
TYPE State_type IS(NSStart,EWStart,GoNS,GoEW,GoNSL,GoEWL);--GoNS=North and South green,GoEW=East and West green,GoNSL=North Left and South Left green,GoEWL=East Left and South Left green
SIGNAL state: State_type;
SIGNAL count: INTEGER RANGE 0 to 16;
BEGIN
	PROCESS(Clk)
	BEGIN
		IF Reset = '1' THEN
			state<=NSStart;
		ELSIF(Clk'EVENT AND Clk='1') THEN
			CASE state IS
			WHEN NSStart =>
			count <= 16;
			state <= GoNS;
			WHEN EWStart =>
			count <= 8;
			state <= GoEW;
			WHEN GoNS =>
				IF count=8 THEN
					state <=GoNSL;
				ELSE
					count <= count - 1;
				END IF;
			WHEN GoEW =>
				IF count=4 THEN
					state <=GoEWL;
				ELSE
					count <= count - 1;
				END IF;
			WHEN GoNSL =>
				IF count=0 THEN
					state<=EWStart;
				ELSE
					count <= count -1;
				END IF;
			WHEN GoEWL =>
				IF count=0 THEN
					state<=NSStart;
				ELSE
					count <=count-1;
				END IF;
			END CASE;
		END IF;
	END PROCESS;
GNS<='1' WHEN state = GoNS ELSE '0';
GEW<='1' WHEN state = GoEW ELSE '0';
RNS<='1' WHEN state = GoEW OR state = GoEWL ELSE '0';
REW<='1' WHEN state = GoNS OR state = GoNSL ELSE '0';
GNSL<='1' WHEN state = GoNSL ELSE '0';
GEWL<='1' WHEN state = GoEWL ELSE '0';
END Behavior;

10 Replies

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    It appears to me that your fsm euns through the stages very well, but I do not see, where the inputs are set.

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    you need to have two processes to control your state machine, one is using for switching another for decoding

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    I'm not really sure what you mean by that but I did some cleaning up, the problem is that there are random blips that seem to pop up for no reason. Here's my code and a waveform to show what I'm talkin about.

    http://img209.imageshack.us/img209/2018/samplelq6.jpg 

    LIBRARY ieee;
USE ieee.std_logic_1164.all;
USE ieee.numeric_std.all ;
ENTITY TrafficLight2 IS
    PORT(NS,EW,NSL,EWL: IN STD_LOGIC_VECTOR(1 DOWNTO 0);--cars that want to go in those directions
         Clk,Reset,EmNS,EmEW,EmNSL,EmEWL,PEW,PNS: IN STD_LOGIC;
         GNS,GEW,RNS,REW,GNSL,GEWL,YNS,YEW: OUT STD_LOGIC);--GNS = greens for North and South, GEW = greens for East and West, GNSL = greens for North and South left lanes, GEWL = greens for East and Weset left lanes
END TrafficLight2;
ARCHITECTURE Behavior OF TrafficLight2 IS
TYPE State_type IS(NSStart,EWStart,GoNS,GoEW,GoNSL,GoEWL,Em);--GoNS=North and South green,GoEW=East and West green,GoNSL=North Left and South Left green,GoEWL=East Left and South Left green
SIGNAL state: State_type;
SIGNAL count: INTEGER RANGE 0 to 31;
BEGIN
    PROCESS(Clk)
    BEGIN
        IF Reset = '1' THEN
            state<=NSStart;
        ELSIF(Clk'EVENT AND Clk='1') THEN
            CASE state IS
            WHEN NSStart =>
            count <= (TO_INTEGER(UNSIGNED(NS))+TO_INTEGER(UNSIGNED(NSL)))*4+4;
            state <= GoNS;
            WHEN EWStart =>
            count <= (TO_INTEGER(UNSIGNED(EW))+TO_INTEGER(UNSIGNED(EWL)))*4+4;
            state <= GoEW;
            WHEN GoNS =>
				IF EmNS ='1' OR EmEW ='1' OR EmEWL ='1' OR EmNSL = '1' THEN
					state<=Em;
                ELSIF count= TO_INTEGER(UNSIGNED(NS))*2+2 THEN
                    state <=GoNSL;
                ELSIF PEW='1' THEN
                    count <= count - 2;
				ELSE
					count <= count - 1;
                END IF;
            WHEN GoEW =>
				IF EmNS ='1' OR EmEW ='1' OR EmEWL ='1' OR EmNSL = '1' THEN
					state<=Em;
                ELSIF count=TO_INTEGER(UNSIGNED(EW))*2+2  THEN
                    state <=GoEWL;
				ELSIF PNS='1' THEN
                    count <= count - 2;
                ELSE
                    count <= count - 1;
                END IF;
            WHEN GoNSL =>
				IF EmNS ='1' OR EmEW ='1' OR EmEWL ='1' OR EmNSL = '1' THEN
					state<=Em;
                ELSIF count=0 THEN
                    state<=EWStart;
				ELSIF PEW='1' THEN
                    count <= count - 2;
                ELSE
                    count <= count -1;
                END IF;
            WHEN GoEWL =>
				IF EmNS ='1' OR EmEW ='1' OR EmEWL ='1' OR EmNSL = '1' THEN
					state<=Em;    
				ELSIF count=0 THEN
                    state<=NSStart;
				ELSIF PNS='1' THEN
                    count <= count - 2;
                ELSE
                    count <=count-1;
                END IF;
			WHEN Em=>
				IF EmNS ='1' OR EmEW ='1' OR EmEWL ='1' OR EmNSL = '1' THEN
					state<=Em;
				ELSE
					state<=NSStart;
				END IF;
            END CASE;
        END IF;
    END PROCESS;
GNS<='1' WHEN (state = GoNS OR EmNS ='1') ELSE '0';
GEW<='1' WHEN (state = GoEW OR EmEW ='1') ELSE '0';
--YNS<='1' WHEN (count = 1 OR count = 2) AND (state = GoNS or state = GoNSL or state=Em) ELSE '0';
--YEW<='1' WHEN (count = 1 OR count = 2) AND (state = GoEW or state = GoEWL or state=Em) ELSE '0';
RNS<='1' WHEN (state = GoEW OR state = GoEWL OR count = 0 OR EmEW ='1' OR EmEWL='1') ELSE '0';
REW<='1' WHEN (state = GoNS OR state = GoNSL OR count = 0 OR EmNS ='1' OR EmNSL='1') ELSE '0';
GNSL<='1' WHEN (state = GoNSL OR EmNSL ='1') ELSE '0';
GEWL<='1' WHEN (state = GoEWL OR EmEWL ='1') ELSE '0';
END Behavior;
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    process(clk, reset)

    begin

    if reset = active then

    current_state <= idle;

    elsif (clk = '1' and clk'event) then

    current_state <= next state;

    end if

    end process;

    process(clk, reset)

    begin

    if reset = active then

    next_state <= idle;

    elsif(clk = '1' and clk'event) then

    case(current_state) is

    when idle =>

    if <condition1> then

    next_state <= S1;

    else

    next_state <= idle;

    when S1 =>

    if <condition2> then

    next_state <= S2

    else

    next_state <= S1;

    when others => null;

    end case;

    end if;

    end process;

    Something like this, first process is switching the states and second using to control the machine and decode logic;

    good lack
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    --- Quote Start ---

    ... the problem is that there are random blips that seem to pop up for no reason. Here's my code and a waveform to show what I'm talkin about.

    . . . 

    RNS<='1' WHEN (state = GoEW OR state = GoEWL OR count = 0 OR EmEW ='1' OR EmEWL='1') ELSE '0';
REW<='1' WHEN (state = GoNS OR state = GoNSL OR count = 0 OR EmNS ='1' OR EmNSL='1') ELSE '0';

    --- Quote End ---

    I can't tell for sure because of the timescale in your waveforms, but it appears that RNS and REW have glitches. These are combinational signals. Combinational signals can glitch unless you carefully design them so that only one LUT or macrocell input at a time can change.

    To avoid glitches, you can assert these signals one clock cycle early and pass them through a register. The output of the register will have the timing you originally intended without the glitches.

    If you register all your outputs adding one clock cycle of delay for everything, then the relative timing between outputs will be unchanged. You will have an extra clock cycle of latency from input to output, but that might be OK. Your application might not require asserting the combinational signals one clock cycle early to preserve the latency you originally intended.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    Right now my clock cycles are too small to really even notice the glitching but it makes the waveform look messy to have it there. I understand what you mean about the signals being combinational but I don't understand what you mean about the signals being passed 1 clock cycle early and having them get put through a register. They are all outside of the process block and they need to know the states of the system. Also since I'm using behavioral vhdl I'm not quite sure how I should put them through a register.

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    --- Quote Start ---

    I understand what you mean about the signals being combinational but I don't understand what you mean about the signals being passed 1 clock cycle early and having them get put through a register. They are all outside of the process block and they need to know the states of the system.

    --- Quote End ---

    Let's say you have the sequence of states X, Y, Z, Z1 with each state lasting only one clock cycle and no possible branches to or from other states for this part of the state sequence.

    If you want output RNS to be asserted during state Y, you can use "RNS<='1' WHEN (state = Y)". The combinational logic creating the RNS signal might have glitches, but it will otherwise be asserted during the time that the state code equals Y plus the propagation delay to create the RNS signal from the state code.

    To avoid glitches on signal RNS, you can instead use "RNS_combinational<='1' WHEN (state = X)" and: 

    process (Clk)
begin
	if (rising_edge(Clk)) then
		RNS <= RNS_combinational;
	end if;
end process;

    RNS will be the output of a register that is asserted one clock cycle after RNS_combinational. RNS will be asserted while the state code equals Y as originally intended.

    When there are state branches, outputs dependent on inputs, states asserted for multiple consecutive clock cycles, etc., the solution is not that simple. But the basic idea still applies that you can assert a state machine output one clock cycle early and pass it through a register to get the timing you originally intended.

    --- Quote Start ---

    If you register all your outputs adding one clock cycle of delay for everything, then the relative timing between outputs will be unchanged. You will have an extra clock cycle of latency from input to output, but that might be OK. Your application might not require asserting the combinational signals one clock cycle early to preserve the latency you originally intended.

    --- Quote End ---

    Now let's say you want output RNS to be asserted during state Y and output REW to be asserted during state Z. You could assert RNS_combinational during state X and REW_combinational during state Y so that registered RNS and REW assert during the intended states Y and Z. But if all you really care is that REW assert one clock cycle after RNS, you can leave the RNS_combinational and REW_combinational assertions during the original states Y and Z and let the registered versions assert in clock cycles Z and Z1, one clock cycle later than originally intended. That might be the best solution if things like state branches in the X, Y, Z part of the state machine make it too much trouble to create a way to assert the combinational versions of the signals one clock cycle early.

    --- Quote Start ---

    Also since I'm using behavioral vhdl I'm not quite sure how I should put them through a register.

    --- Quote End ---

    If you are synthesizing the VHDL for programmable logic, then it is technically more correct to say that you are using register transfer language (RTL) instead of behavioral VHDL. If you don't write the VHDL in an RTL style that the synthesis tool understands to infer physical device resources like LUTs and registers, you might get poor synthesis results. Use a reference like those mentioned at http://www.alteraforum.com/forum/showthread.php?t=1025 to see examples of acceptable coding styles for things like state machines and registers. You can also get examples from the VHDL templates in the Quartus text editor, which is where I got the template for the register process above.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    This looks like a clocking problem, where the atate and counter both change while the clock is active and then the new counter value then causes another state change, then another, then another, etc.

    If you are not using an edge triggered state flip flop, it may be the cause.

    I am an old designer trying to get up to speed, but it looks like VHDL is too complex to be very useful for this level of abstraction even if it is the only game in town.

    I have a prototype that I will not be able to fully develop, but it is on a web site if anyone would care to try it. It is based on a tool that I wrote for my own use then evolved thru C, C+, and now C++.

    http://mysite.verizon.net/vzeosqt4/simplesimin

    I am attaching a tutorial example FSM counter with the VHDL commented out with // and the input text for my programs at the end so you can see the difference.

    //I am including a new testcase here. I coded a VHDL example for a FSM. Hope its interesting.

    // -- cocurrent process#1: state registers

    // state_reg: process(clock, reset)

    // begin

    //if (reset='1') then

    // current_state <= S0;

    //elsif (clock'event and clock='1') then

    // current_state <= next_state;

    //end if;

    // end process;

    // -- cocurrent process#2: combinational logic

    // comb_logic: process(current_state, a)

    // begin

    //-- use case statement to show the

    //-- state transistion

    //case current_state is

    // when S0 => x <= '0';

    // if a='0' then

    // next_state <= S0;

    // elsif a ='1' then

    // next_state <= S1;

    // end if;

    // when S1 => x <= '0';

    // if a='0' then

    // next_state <= S1;

    // elsif a='1' then

    // next_state <= S2;

    // end if;

    // when S2 => x <= '0';

    // if a='0' then

    // next_state <= S2;

    // elsif a='1' then

    // next_state <= S3;

    // end if;

    // when S3 => x <= '1';

    // if a='0' then

    // next_state <= S3;

    // elsif a='1' then

    // next_state <= S0;

    // end if;

    // when others =>

    // x <= '0';

    // next_state <= S0;

    //end case;

    // -- cocurrent process#1: state registers

    // state_reg: process(clock, reset)

    // begin

    //if (reset='1') then

    // current_state <= S0;

    //elsif (clock'event and clock='1') then

    // current_state <= next_state;

    //end if;

    // end process;

    // -- cocurrent process#2: combinational logic

    // comb_logic: process(current_state, a)

    // begin

    //-- use case statement to show the

    //-- state transistion

    //case current_state is

    // when S0 => x <= '0';

    // if a='0' then

    // next_state <= S0;

    // elsif a ='1' then

    // next_state <= S1;

    // end if;

    // when S1 => x <= '0';

    // if a='0' then

    // next_state <= S1;

    // elsif a='1' then

    // next_state <= S2;

    // end if;

    // when S2 => x <= '0';

    // if a='0' then

    // next_state <= S2;

    // elsif a='1' then

    // next_state <= S3;

    // end if;

    // when S3 => x <= '1';

    // if a='0' then

    // next_state <= S3;

    // elsif a='1' then

    // next_state <= S0;

    // end if;

    // when others =>

    // x <= '0';

    // next_state <= S0;

    //end case;

    clk.0 @ 0.2

    clk.1 @ 1.2

    S0.0 @0

    S1.1 @ 0 //Set each state to a unique value because current_state will be set to the appropriate value

    S2.2 @ 0

    S3.3 @0

    a.1 @ 4.6

    a.0 @ 5.6

    reset.1 @ 4

    reset.0 @ 5

    \c1?!clk

    \c2?clk

    current_state:S0 ? (current_state == S3)&a | reset

    current_state:S1 ? (current_state == S0)&a

    current_state:S2 ? (current_state == S1)&a

    current_state:S3 ? (current_state == S2)&a

    I started to add a GUI for input, but ran out of gas. Also the output could be made much prettier with maybe netlist output for final design usage.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    Hey i have implemented a traffic light controller in the past.. i will advise you to use an ASM, then you can follow the processes and see where isn't functioning properly as required.. Its much easier with ASM.

    Also when i designed my counter i used P_count and N_count, e.g:

    n_count <= p_count + 1

    the problem might be coming from the counters, even though i havnt looked at ur code in details, because as soon as ur counter is wrong, the entire system will be wrong.

    Mail me privately if you need my codes...

    All the best!
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    18 years ago

    >there are random blips

    This seems nothing special, very common bahaviour with combinatinal logic - you have to make sure that all physical outs are synched.