Help with Finite state machine

[Altera_Forum]

Hi guys, as an altera pupil i am still struggling with getting FSM to work properly i wonder if it is my testbench that has a problem or the code itself. This is the idea: 

when key_0 is pressed let LEDG(1) light (on) ----closed state when key_1 is pressed let LEDG(0) light (on)-----opened state when key_2 is pressed let LEDR(1) light (on)------locked state when key_3 is pressed it unlocks and enters into error state causing LEDG(1), LEDG(0), LEDR(0) and LEDR(1) to light(all on)  This is how my code looks at the moment  library IEEE; use IEEE.STD_LOGIC_1164.ALL;  use IEEE.STD_LOGIC_UNSIGNED.ALL;   entity uppgift_dorr is port( clk : in std_logic; --clock signal  reset_n : in std_logic; --reset signal  key_0, key_1, key_2, key_3 : in std_logic;   LEDR : out std_logic_vector(1 downto 0);  LEDG : out std_logic_vector(1 downto 0)    );  end uppgift_dorr; architecture Behavioral of uppgift_dorr is  --Defines the type for states in the state machine type state_type is (closed,opened,locked,error); --Declare the signal with the corresponding state type. signal Current_State, Next_State : state_type;   begin  -- Synchronous Process p0: process(clk, reset_n)  begin  if( reset_n = '0' ) then --Synchronous Reset  Current_State <= opened;   elsif (rising_edge(clk)) then --Rising edge of Clock  Current_State <= Next_State;  end if; end process;  ----------------------------------------------------------------------------- -- Combinational Process p1: Process(Current_State, key_0, key_1, key_2, key_3)  begin  case Current_State is  when opened =>   LEDG <= "01"; LEDR <= "00";   if ( key_0 = '0' ) then  Next_State <= closed;   else   Next_State <= opened;  end if;     when closed =>  LEDG <= "10"; LEDR <= "01";  if ( key_1 = '0') then  Next_State <= opened;   elsif (key_2 = '0') then  Next_State <= locked;  else  Next_State <= closed;  end if;    when locked =>   LEDG <= "00"; LEDR <= "10";   if ( key_3 = '0') then -- Låser upp dörren  Next_State <= closed;  elsif(key_2 = '0')then -- tray to lock, error  Next_State <= error;   else   Next_State <= locked;  end if;     when error =>   LEDG <= "11"; LEDR <= "11";  Next_State <= error;         end case;  --end if; end process;    end Behavioral; -------------------------------------------- The test bench looks this way:  LIBRARY ieee;  USE ieee.std_logic_1164.all;    ENTITY uppgift_dorr_vhd_tst IS END uppgift_dorr_vhd_tst; ARCHITECTURE uppgift_dorr_arch OF uppgift_dorr_vhd_tst IS -- constants  -- signals  SIGNAL clk : STD_LOGIC := '0'; SIGNAL key_0 : STD_LOGIC; SIGNAL key_1 : STD_LOGIC; SIGNAL key_2 : STD_LOGIC; SIGNAL key_3 : STD_LOGIC; SIGNAL LEDG : STD_LOGIC_VECTOR(1 DOWNTO 0); SIGNAL LEDR : STD_LOGIC_VECTOR(1 DOWNTO 0); SIGNAL reset_n : STD_LOGIC := '0'; COMPONENT uppgift_dorr  PORT (  clk : IN STD_LOGIC;  key_0 : IN STD_LOGIC;  key_1 : IN STD_LOGIC;  key_2 : IN STD_LOGIC;  key_3 : IN STD_LOGIC;  LEDG : OUT STD_LOGIC_VECTOR(1 DOWNTO 0);  LEDR : OUT STD_LOGIC_VECTOR(1 DOWNTO 0);  reset_n : IN STD_LOGIC  ); END COMPONENT; BEGIN  i1 : uppgift_dorr  PORT MAP ( -- list connections between master ports and signals  clk => clk,  key_0 => key_0,  key_1 => key_1,  key_2 => key_2,  key_3 => key_3,  LEDG => LEDG,  LEDR => LEDR,  reset_n => reset_n  );    clk <= NOT clk after 20 ns; -- 50MHz  reset_n <= '0', '1' after 100 ns;   init : PROCESS  -- variable declarations  BEGIN   key_0 <= '1';  WAIT FOR 50 ns;  key_1 <= '1';  WAIT FOR 50 ns;  key_2 <= '1';  WAIT FOR 50 ns;  key_3 <= '1';  WAIT FOR 50 ns;  -------------------------    key_0 <= '0';  WAIT FOR 50 ns;  key_1 <= '0';  WAIT FOR 50 ns;  key_2 <= '0';  WAIT FOR 50 ns;  key_3 <= '0';  WAIT FOR 50 ns;    WAIT;  END PROCESS init;  always : PROCESS  -- optional sensitivity list  -- ( )  -- variable declarations  BEGIN   -- code executes for every event on sensitivity list  WAIT;  END PROCESS always;  END uppgift_dorr_arch; ----------------------------------------------------------- Can anyone help?

[Altera_Forum]

If it doesnt work, you need to debug it. Thats what the testbench is for. Does the state machine go through all states as expected? have you studied the waveforms.

[Altera_Forum]

--- Quote Start ---  If it doesnt work, you need to debug it. Thats what the testbench is for. Does the state machine go through all states as expected? have you studied the waveforms.  --- Quote End ---    ----------------------- What is strange about the wave form is that i do not see the value 11 in the section LEDR and LEDG probably some state is missing. This is what i have difficulty to figure out. Help please?

[Altera_Forum]

You need to trace back the problem. You can show all the states on the waveform. Have you added all the signals from the unit under test?

[Altera_Forum]

--- Quote Start ---  You need to trace back the problem. You can show all the states on the waveform. Have you added all the signals from the unit under test?  --- Quote End ---   ------------------------------ Yes i have done so i have added all the unit under test. I am trying to see how i can figure out the problem, Ooops!!!

[Altera_Forum]

--- Quote Start ---  ------------------------------ Yes i have done so i have added all the unit under test. I am trying to see how i can figure out the problem, Ooops!!!  --- Quote End ---    your code looks ok to me but your testbench does not cover well your state transitions.

[Altera_Forum]

--- Quote Start ---  your code looks ok to me but your testbench does not cover well your state transitions.  --- Quote End ---   ---------------------------------------------------- Hi again i have added the transition states but i still do not have all the states showing up. 00, 01, 10, 11 where could the problem be? Help please!!

[Altera_Forum]

--- Quote Start ---  ---------------------------------------------------- Hi again i have added the transition states but i still do not have all the states showing up. 00, 01, 10, 11 where could the problem be? Help please!!   --- Quote End ---    try one of these two patterns.  init : PROCESS BEGIN wait for 100 ns; key_0 <= '1'; WAIT FOR 500 ns; key_0 <= '0'; wait for 300 ns; key_1 <= '1'; WAIT FOR 600 ns; key_1 <= '0'; wait for 200 ns; key_2 <= '1'; WAIT FOR 850 ns; key_2 <= '0'; wait for 200 ns; key_3 <= '1'; END PROCESS init;   or  init : PROCESS(reset_n,clk) variable shift_reg : std_logic_vector(19 downto 0); BEGIN if reset_n = '0' then shift_reg := x"01000"; elsif rising_edge(clk) then shift_reg(0) := shift_reg(19) xor shift_reg(16); shift_reg(19 downto 1) := shift_reg(18 downto 0); key_0 <= shift_reg(0) or shift_reg(1) or shift_reg(2); key_1 <= shift_reg(4) or shift_reg(7) or shift_reg(9); key_2 <= shift_reg(2) or shift_reg(10) or shift_reg(12); key_3 <= shift_reg(13) or shift_reg(11) or shift_reg(19); end if; END PROCESS init;

[Altera_Forum]

--- Quote Start ---  try one of these two patterns.  init : PROCESS BEGIN wait for 100 ns; key_0 <= '1'; WAIT FOR 500 ns; key_0 <= '0'; wait for 300 ns; key_1 <= '1'; WAIT FOR 600 ns; key_1 <= '0'; wait for 200 ns; key_2 <= '1'; WAIT FOR 850 ns; key_2 <= '0'; wait for 200 ns; key_3 <= '1'; END PROCESS init;   or  init : PROCESS(reset_n,clk) variable shift_reg : std_logic_vector(19 downto 0); BEGIN if reset_n = '0' then shift_reg := x"01000"; elsif rising_edge(clk) then shift_reg(0) := shift_reg(19) xor shift_reg(16); shift_reg(19 downto 1) := shift_reg(18 downto 0); key_0 <= shift_reg(0) or shift_reg(1) or shift_reg(2); key_1 <= shift_reg(4) or shift_reg(7) or shift_reg(9); key_2 <= shift_reg(2) or shift_reg(10) or shift_reg(12); key_3 <= shift_reg(13) or shift_reg(11) or shift_reg(19); end if; END PROCESS init;   --- Quote End ---   -------------------------------------------------------------------------------------------------------------------------------- Hi Kaz, thank you once again for the code i have been able to have all the states appearing now but the problem is that it does not work effectively on the DE2-115. The LEDG and LEDR are coming on at the same time. I will like it to work as i stated in my initial help question:  https://www.alteraforum.com/forum/attachment.php?attachmentid=8266

[Altera_Forum]

I am not surprised that it doesn't seem to work in hardware since you depend on manual switching.  

manual switching is too slow and leads to rebounce thus when you press any switch it will go through 0/1 for sometime until it settles.  The remedy is to add switch debounce logic.

[Altera_Forum]

--- Quote Start ---  I am not surprised that it doesn't seem to work in hardware since you depend on manual switching.  manual switching is too slow and leads to rebounce thus when you press any switch it will go through 0/1 for sometime until it settles.  The remedy is to add switch debounce logic.  --- Quote End ---   ------------------------------------ Wow Kaz you are genius, thank you for the help i will try to read about switch debounce logic and see how it can be added to the code, since i am just a beginner in vhdl and using of the Altera DE2-115 hardware. Once more i am grateful

[Altera_Forum]

--- Quote Start ---  ------------------------------------ Wow Kaz you are genius, thank you for the help i will try to read about switch debounce logic and see how it can be added to the code, since i am just a beginner in vhdl and using of the Altera DE2-115 hardware. Once more i am grateful  --- Quote End ---    I am no expert on debouncing logic. some boards have it as part of switch design outside fpga. but you can try this(not tested)  per each switch  signal count_0 : unsigned(17 downto 0) := (others => '0'); signal sw_0_d, sw_0_final : std_logic; --in a clocked process sw_0_d <= sw_0; if sw_0 = sw_0_d then count_0 <= count_0 +1; else count_0 <= (others => '0'); end if; if count_0 > 250000 then -- about 5 msec bounce time sw_0_final <= sw_0; end if;

[Altera_Forum]

--- Quote Start ---  I am no expert on debouncing logic. some boards have it as part of switch design outside fpga. but you can try this(not tested)  per each switch  signal count_0 : unsigned(17 downto 0) := (others => '0'); signal sw_0_d, sw_0_final : std_logic; --in a clocked process sw_0_d <= sw_0; if sw_0 = sw_0_d then count_0 <= count_0 +1; else count_0 <= (others => '0'); end if; if count_0 > 250000 then -- about 5 msec bounce time sw_0_final <= sw_0; end if;   --- Quote End ---   ------------------------------------------------------------------ Sorry to bother you guys again , Kaz, i am having difficulties to cause the code to function without errors, please where exactly in the clock process or per each switch in my code should i insert the code your posted. Thanks for helping

[Altera_Forum]

--- Quote Start ---  ------------------------------------------------------------------ Sorry to bother you guys again , Kaz, i am having difficulties to cause the code to function without errors, please where exactly in the clock process or per each switch in my code should i insert the code your posted. Thanks for helping  --- Quote End ---    use a separate clocked process per each key then in your state machine use sw_0_final,sw_1_final,...etc instead of sw_0,sw_1 ...etc. declare 3 counters and other signals at top of your design.

[Altera_Forum]

--- Quote Start ---  use a separate clocked process per each key then in your state machine use sw_0_final,sw_1_final,...etc instead of sw_0,sw_1 ...etc. declare 3 counters and other signals at top of your design.  --- Quote End ---   ------------------------------------------------------------------------------------------- please Kaz, i really do not understand how to go about this as i am a beginner in VHDL. Please do you mind showing me an example with reference to my code above. The 3 counters how is that going to be done and other signals?

[Altera_Forum]

--- Quote Start ---  ------------------------------------------------------------------------------------------- please Kaz, i really do not understand how to go about this as i am a beginner in VHDL. Please do you mind showing me an example with reference to my code above. The 3 counters how is that going to be done and other signals?  --- Quote End ---    here is the code. Note it will fail your simulation unless you change the pattern of keys to let counter reach 25000 clocks so it is suitable for hardware mostly.   library IEEE; use IEEE.STD_LOGIC_1164.ALL; use IEEE.numeric_std.ALL; entity uppgift_dorr is port( clk : in std_logic; --clock signal reset_n : in std_logic; --reset signal key_0, key_1, key_2, key_3 : in std_logic; LEDR : out std_logic_vector(1 downto 0); LEDG : out std_logic_vector(1 downto 0) ); end uppgift_dorr; architecture Behavioral of uppgift_dorr is --Defines the type for states in the state machine type state_type is (closed,opened,locked,error); --Declare the signal with the corresponding state type. signal Current_State, Next_State : state_type; signal count_0 : unsigned(17 downto 0) := (others => '0'); signal count_1 : unsigned(17 downto 0) := (others => '0'); signal count_2 : unsigned(17 downto 0) := (others => '0'); signal count_3 : unsigned(17 downto 0) := (others => '0'); signal key_0_d,key_0_final: std_logic; signal key_1_d,key_1_final: std_logic; signal key_2_d,key_2_final: std_logic; signal key_3_d,key_3_final: std_logic; begin --debounce process begin wait until clk = '1'; key_0_d <= key_0; if key_0 = key_0_d then count_0 <= count_0 +1; else count_0 <= (others => '0'); end if; if count_0 > 250000 then -- about 5 msec bounce time key_0_final <= key_0; end if; key_1_d <= key_1; if key_1 = key_1_d then count_1 <= count_1 +1; else count_1 <= (others => '0'); end if; if count_1 > 250000 then -- about 5 msec bounce time key_1_final <= key_1; end if; key_2_d <= key_2; if key_2 = key_2_d then count_2 <= count_2 +1; else count_2 <= (others => '0'); end if; if count_2 > 250000 then -- about 5 msec bounce time key_2_final <= key_2; end if; key_3_d <= key_3; if key_3 = key_3_d then count_3 <= count_3 +1; else count_3 <= (others => '0'); end if; if count_3 > 250000 then -- about 5 msec bounce time key_3_final <= key_3; end if; end process; -- Synchronous Process p0: process(clk, reset_n) begin if( reset_n = '0' ) then --Synchronous Reset Current_State <= opened; elsif (rising_edge(clk)) then --Rising edge of Clock Current_State <= Next_State; end if; end process; ----------------------------------------------------------------------------- -- Combinational Process p1: Process(Current_State, key_0_final, key_1_final, key_2_final, key_3_final) begin case Current_State is when opened => LEDG <= "01"; LEDR <= "00"; if ( key_0_final = '0' ) then Next_State <= closed; end if; when closed => LEDG <= "10"; LEDR <= "01"; if ( key_1_final = '0') then Next_State <= opened; elsif (key_2_final = '0') then Next_State <= locked; end if; when locked => LEDG <= "00"; LEDR <= "10"; if ( key_3_final = '0') then -- Låser upp dörren Next_State <= closed; elsif(key_2_final = '0')then -- tray to lock, error Next_State <= error; end if; when error => LEDG <= "11"; LEDR <= "11"; end case; end process; end Behavioral;

[Altera_Forum]

--- Quote Start ---  here is the code. Note it will fail your simulation unless you change the pattern of keys to let counter reach 25000 clocks so it is suitable for hardware mostly.   library IEEE; use IEEE.STD_LOGIC_1164.ALL; use IEEE.numeric_std.ALL; entity uppgift_dorr is port( clk : in std_logic; --clock signal reset_n : in std_logic; --reset signal key_0, key_1, key_2, key_3 : in std_logic; LEDR : out std_logic_vector(1 downto 0); LEDG : out std_logic_vector(1 downto 0) ); end uppgift_dorr; architecture Behavioral of uppgift_dorr is --Defines the type for states in the state machine type state_type is (closed,opened,locked,error); --Declare the signal with the corresponding state type. signal Current_State, Next_State : state_type; signal count_0 : unsigned(17 downto 0) := (others => '0'); signal count_1 : unsigned(17 downto 0) := (others => '0'); signal count_2 : unsigned(17 downto 0) := (others => '0'); signal count_3 : unsigned(17 downto 0) := (others => '0'); signal key_0_d,key_0_final: std_logic; signal key_1_d,key_1_final: std_logic; signal key_2_d,key_2_final: std_logic; signal key_3_d,key_3_final: std_logic; begin --debounce process begin wait until clk = '1'; key_0_d <= key_0; if key_0 = key_0_d then count_0 <= count_0 +1; else count_0 <= (others => '0'); end if; if count_0 > 250000 then -- about 5 msec bounce time key_0_final <= key_0; end if; key_1_d <= key_1; if key_1 = key_1_d then count_1 <= count_1 +1; else count_1 <= (others => '0'); end if; if count_1 > 250000 then -- about 5 msec bounce time key_1_final <= key_1; end if; key_2_d <= key_2; if key_2 = key_2_d then count_2 <= count_2 +1; else count_2 <= (others => '0'); end if; if count_2 > 250000 then -- about 5 msec bounce time key_2_final <= key_2; end if; key_3_d <= key_3; if key_3 = key_3_d then count_3 <= count_3 +1; else count_3 <= (others => '0'); end if; if count_3 > 250000 then -- about 5 msec bounce time key_3_final <= key_3; end if; end process; -- Synchronous Process p0: process(clk, reset_n) begin if( reset_n = '0' ) then --Synchronous Reset Current_State <= opened; elsif (rising_edge(clk)) then --Rising edge of Clock Current_State <= Next_State; end if; end process; ----------------------------------------------------------------------------- -- Combinational Process p1: Process(Current_State, key_0_final, key_1_final, key_2_final, key_3_final) begin case Current_State is when opened => LEDG <= "01"; LEDR <= "00"; if ( key_0_final = '0' ) then Next_State <= closed; end if; when closed => LEDG <= "10"; LEDR <= "01"; if ( key_1_final = '0') then Next_State <= opened; elsif (key_2_final = '0') then Next_State <= locked; end if; when locked => LEDG <= "00"; LEDR <= "10"; if ( key_3_final = '0') then -- Låser upp dörren Next_State <= closed; elsif(key_2_final = '0')then -- tray to lock, error Next_State <= error; end if; when error => LEDG <= "11"; LEDR <= "11"; end case; end process; end Behavioral;   --- Quote End ---   ---------------------------------------------------------- Hi Kaz, i lack words to show my appreciation thank you so much for the help i am going to test it on the DE2 board.

[Altera_Forum]

--- Quote Start ---  ---------------------------------------------------------- Hi Kaz, i lack words to show my appreciation thank you so much for the help i am going to test it on the DE2 board.  --- Quote End ---    -------------------------------------------------------------------------------------------------------------------- Hi Kaz, i have an error message in accordance with the code: what could be the reason? i have added an end at the end of the process but it still did not help vhdl gurus you are also free to give a helping hand, thanks  Error (10500): VHDL syntax error at uppgift_dorr.vhd(92) near text "begin"; expecting "end", or "(", or an identifier ("begin" is a reserved keyword), or a concurrent statement

[Altera_Forum]

Which code are you referring to. The code posted by Kaz compiles just fine. Have you tried to modify it?

[Altera_Forum]

--- Quote Start ---  Which code are you referring to. The code posted by Kaz compiles just fine. Have you tried to modify it?  --- Quote End ---   --------------------------------------------------- No i have done no modifications to it, you can see below:   library IEEE; use IEEE.STD_LOGIC_1164.ALL; use IEEE.NUMERIC_STD.ALL; entity uppgift_dorr is port( clk : in std_logic; --clock signal reset_n : in std_logic; --reset signal key_0, key_1, key_2, key_3 : in std_logic; LEDR : out std_logic_vector(1 downto 0); LEDG : out std_logic_vector(1 downto 0) ); end uppgift_dorr; architecture Behavioral of uppgift_dorr is --Defines the type for states in the state machine type state_type is (closed,opened,locked,error); --Declare the signal with the corresponding state type. signal Current_State, Next_State : state_type; --Additional signals to work with the debounce process signal count_0 : unsigned(17 downto 0) := (others => '0'); signal count_1 : unsigned(17 downto 0) := (others => '0'); signal count_2 : unsigned(17 downto 0) := (others => '0'); signal count_3 : unsigned(17 downto 0) := (others => '0'); signal key_0_d,key_0_final: std_logic; signal key_1_d,key_1_final: std_logic; signal key_2_d,key_2_final: std_logic; signal key_3_d,key_3_final: std_logic; --Debounce process begin --debounce process(clk) begin wait until clk = '1'; key_0_d <= key_0; if key_0 = key_0_d then count_0 <= count_0 +1; else count_0 <= (others => '0'); end if; if count_0 > 250000 then -- about 5 msec bounce time key_0_final <= key_0; end if; key_1_d <= key_1; if key_1 = key_1_d then count_1 <= count_1 +1; else count_1 <= (others => '0'); end if; if count_1 > 250000 then -- about 5 msec bounce time key_1_final <= key_1; end if; key_2_d <= key_2; if key_2 = key_2_d then count_2 <= count_2 +1; else count_2 <= (others => '0'); end if; if count_2 > 250000 then -- about 5 msec bounce time key_2_final <= key_2; end if; key_3_d <= key_3; if key_3 = key_3_d then count_3 <= count_3 +1; else count_3 <= (others => '0'); end if; if count_3 > 250000 then -- about 5 msec bounce time key_3_final <= key_3; end if; end process; -------------- begin -- Synchronous Process p0: process(clk, reset_n) begin if( reset_n = '0' ) then --Synchronous Reset Current_State <= opened; elsif (rising_edge(clk)) then --Rising edge of Clock Current_State <= Next_State; end if; end process; ----------------------------------------------------------------------------- -- Combinational Process p1: Process(Current_State, key_0_final, key_1_final, key_2_final, key_3_final) begin case Current_State is when opened => LEDG <= "01"; LEDR <= "00"; if ( key_0_final = '0' ) then Next_State <= closed; --else -- Next_State <= opened; end if; when closed => LEDG <= "10"; LEDR <= "01"; if ( key_1_final = '0') then Next_State <= opened; elsif (key_2_final = '0') then Next_State <= locked; --recent change it was key_2 locked else Next_State <= closed; ---here was closed end if; when locked => LEDG <= "00"; LEDR <= "10"; if ( key_3_final = '0') then -- Låser upp dörren Next_State <= locked; elsif(key_2_final = '0')then -- tray to lock, error Next_State <= error; else Next_State <= closed; end if; LEDR <= "00"; when error => LEDG <= "11"; LEDR <= "11"; Next_State <= error; end case; end process; end Behavioral;

[Altera_Forum]

But somehow you have misscopied it (or modified it) 

There is an extra "begin" on line 97, and the first debounce process has (clk) as a sensitivity list when it should have no sensitivity list. Otherwise the code is the same.  It also contains extra comments that Kaz's doesnt have either. I suspect you didnt copy it - you tried to merge it into your own copy.