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]

--- Quote Start ---  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.  --- Quote End ---   ------------------------------------------------------------------------------------------------- Now i deleted my old code and copied kaz's exact code, but i still have inferring latches see below: what code the problem?  Info: ******************************************************************* Info: Running Quartus II 64-Bit Analysis & Synthesis  Info: Version 13.0.1 Build 232 06/12/2013 Service Pack 1 SJ Web Edition  Info: Processing started: Thu Jan 02 14:39:19 2014 Info: Command: quartus_map --read_settings_files=on --write_settings_files=off uppgift_dorr -c uppgift_dorr Warning (20028): Parallel compilation is not licensed and has been disabled Info (12021): Found 2 design units, including 1 entities, in source file uppgift_dorr.vhd  Info (12022): Found design unit 1: uppgift_dorr-Behavioral  Info (12023): Found entity 1: uppgift_dorr Info (12127): Elaborating entity "uppgift_dorr" for the top level hierarchy Warning (10631): VHDL Process Statement warning at uppgift_dorr.vhd(103): inferring latch(es) for signal or variable "Next_State", which holds its previous value in one or more paths through the process Info (10041): Inferred latch for "Next_State.error" at uppgift_dorr.vhd(103) Info (10041): Inferred latch for "Next_State.locked" at uppgift_dorr.vhd(103) Info (10041): Inferred latch for "Next_State.opened" at uppgift_dorr.vhd(103) Info (10041): Inferred latch for "Next_State.closed" at uppgift_dorr.vhd(103) Warning (13012): Latch Next_State.opened_380 has unsafe behavior  Warning (13013): Ports D and ENA on the latch are fed by the same signal key_1_final Warning (13012): Latch Next_State.locked_365 has unsafe behavior  Warning (13013): Ports D and ENA on the latch are fed by the same signal key_1_final Warning (13012): Latch Next_State.closed_395 has unsafe behavior  Warning (13013): Ports D and ENA on the latch are fed by the same signal Current_State.opened Warning (13012): Latch Next_State.error_350 has unsafe behavior  Warning (13013): Ports D and ENA on the latch are fed by the same signal key_3_final Info (286030): Timing-Driven Synthesis is running Info (16010): Generating hard_block partition "hard_block:auto_generated_inst"  Info (16011): Adding 0 node(s), including 0 DDIO, 0 PLL, 0 transceiver and 0 LCELL Info (21057): Implemented 156 device resources after synthesis - the final resource count might be different  Info (21058): Implemented 6 input pins  Info (21059): Implemented 4 output pins  Info (21061): Implemented 146 logic cells Info: Quartus II 64-Bit Analysis & Synthesis was successful. 0 errors, 10 warnings  Info: Peak virtual memory: 479 megabytes  Info: Processing ended: Thu Jan 02 14:39:24 2014  Info: Elapsed time: 00:00:05  Info: Total CPU time (on all processors): 00:00:04

[Altera_Forum]

Adding the default assignment at the beginning of the p1 process: 

 next_state <= current_state;  removes the warnings.

[Altera_Forum]

--- Quote Start ---  Adding the default assignment at the beginning of the p1 process:  next_state <= current_state;  removes the warnings.  --- Quote End ---   --------------------------------------------  Thanks Tricky and Kaz i am beginning to see some green light.

[Altera_Forum]

The reason you get warnings is to do with the combinatorial process. A cleaner alternative is to use one state, since then all changes are on the clock edge. see code below.  

I also now believe my debounce counter of 18 bits is an overkill and I assume 10 bits may be enough. I also added a constant set to 10 for simulation and to 1000 for hardware debounce.   library IEEE; use IEEE.STD_LOGIC_1164.ALL; use IEEE.numeric_std.ALL; entity uppgift_dorr is port( clk : in std_logic; reset_n : in std_logic; 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 type state_type is (closed,opened,locked,error); signal State: state_type; constant max : integer := 10; -- 10 for simulation, 1000 for hardware signal count_0 : unsigned(9 downto 0) := (others => '0'); signal count_1 : unsigned(9 downto 0) := (others => '0'); signal count_2 : unsigned(9 downto 0) := (others => '0'); signal count_3 : unsigned(9 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 > max then 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 > max then 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 > max then 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 > max then key_3_final <= key_3; end if; end process; -- Synchronous Process p0: process(clk, reset_n) begin if( reset_n = '0' ) then State <= opened; elsif (rising_edge(clk)) then case State is when opened => LEDG <= "01"; LEDR <= "00"; if ( key_0_final = '0' ) then State <= closed; end if; when closed => LEDG <= "10"; LEDR <= "01"; if ( key_1_final = '0') then State <= opened; elsif (key_2_final = '0') then State <= locked; end if; when locked => LEDG <= "00"; LEDR <= "10"; if ( key_3_final = '0') then State <= closed; elsif(key_2_final = '0')then State <= error; end if; when error => LEDG <= "11"; LEDR <= "11"; end case; end if; end process; end Behavioral;

[Altera_Forum]

--- Quote Start ---  The reason you get warnings is to do with the combinatorial process. A cleaner alternative is to use one state, since then all changes are on the clock edge. see code below.  I also now believe my debounce counter of 18 bits is an overkill and I assume 10 bits may be enough. I also added a constant set to 10 for simulation and to 1000 for hardware debounce.   library IEEE; use IEEE.STD_LOGIC_1164.ALL; use IEEE.numeric_std.ALL; entity uppgift_dorr is port( clk : in std_logic; reset_n : in std_logic; 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 type state_type is (closed,opened,locked,error); signal State: state_type; constant max : integer := 10; -- 10 for simulation, 1000 for hardware signal count_0 : unsigned(9 downto 0) := (others => '0'); signal count_1 : unsigned(9 downto 0) := (others => '0'); signal count_2 : unsigned(9 downto 0) := (others => '0'); signal count_3 : unsigned(9 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 > max then 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 > max then 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 > max then 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 > max then key_3_final <= key_3; end if; end process; -- Synchronous Process p0: process(clk, reset_n) begin if( reset_n = '0' ) then State <= opened; elsif (rising_edge(clk)) then case State is when opened => LEDG <= "01"; LEDR <= "00"; if ( key_0_final = '0' ) then State <= closed; end if; when closed => LEDG <= "10"; LEDR <= "01"; if ( key_1_final = '0') then State <= opened; elsif (key_2_final = '0') then State <= locked; end if; when locked => LEDG <= "00"; LEDR <= "10"; if ( key_3_final = '0') then State <= closed; elsif(key_2_final = '0')then State <= error; end if; when error => LEDG <= "11"; LEDR <= "11"; end case; end if; end process; end Behavioral;   --- Quote End ---   ------------------------------------------------------------------ Thanks for all the help Tricky and Kaz. Everything is working prefectly with the Finite state machine now.