State machine; why only last state is working?

Question

I have a state machine with 6 states(3 main states). Only the last state is working but the first 2 doesn't(out of 3).Only the last state is working. I found out the problem, when I remove the debounce circuit it works, but I need the debouncing circuit. I got the debouncing circuit from the internet. I would be glad if someone can help.

 type SM_STATES is (state_column_1, scan_col_1, state_column_2, scan_col_2,
                          state_column_3, scan_col_3);
 signal my_state     : SM_STATES                   := state_column_1;

Below is the state machine:

scanner_sm : process (clk)
begin  -- process key_scanner
  if clk'event and clk = '1' then

  if state_inc = '1' then -- clock divider finished counting down 100000

    -- reset scan_complete
    scan_complete <= '0';

case my_state is

  when state_column_1 =>
    scanned_val  <= (others => '0');
    original_col   <= "110";
    my_state <= scan_col_1;

  when scan_col_1 =>
    case bcd_val is
      when "1110" => scanned_val <= "1100100";  -- 1 wrong
      when "1101" => scanned_val <= "1100010";  -- 2 wrong
      when others => scanned_val <= "0010000";
    end case;
    my_state <= state_column_2;

  when state_column_2 =>
    original_col   <= "011";
    my_state <= scan_col_2;

  when scan_col_2 =>
    case bcd_val is
      when "1110" => scanned_val <= "1011011";  -- 5 wrong
      when "1101" => scanned_val <= "1011111";  -- 6 wrong
      when others => scanned_val <= "0000000";
    end case;
    my_state <= state_column_3;

  when state_column_3 =>
    original_col   <= "101";
    my_state <= scan_col_3;

  when scan_col_3 => -- Reading S1 // The only working state
    case bcd_val is
      when "1110" => scanned_val <= "1100000";  -- 9/ 1
      when "1101" => scanned_val <= "0111110";  -- X/ 2
      when others => scanned_val <= "0000000";
    end case;
    my_state <= state_column_1; -- ************ Error might be here
    scan_complete <= '1'; -- ********** Error might be here

  when others => scanned_val <= "0000000";
end case;

      end if;
  end if;
end process scanner_sm;

debounce: process (CLK) is 
begin 
 if (CLK'event and CLK = '1') then  
  Q0 <= scannel_val; 

  Q1 <= Q0; 

  Q2 <= Q1; 

 end if; 

end process; 

Final_val <= Q0 and Q1 and (not Q2);

end Behavioral; 

Answer 1

Your code so far is incomplete - you directly assign within your case statement to the signal my_state which is evaluated by the state machine. To understand the problem with this we need to know how the simulator works:

In comparison to actual hardware the simulator has to process the the code with a sequential CPU in a sequential matter. This works by running through the code over and over again in infinitesimal time distances - the so called Delta delay - until all dependencies have resolved, i.e. nothing changes anymore.

Note that no actual time has passed during this iterations. In a correctly written design the simulator now waits until the next event occurs - usually caused by the tick of a clock which again restarts the sequential iterations.

Your example basically resembles an infinite loop: a change of my_state always causes the next change of my_state and thus the simulator never settles to a value - until it reaches a hardcoded iteration limit which, by chance in your case is the third state.

So, how to fix this? We need to introduce a clock and need to make the state transition depending on the actual simulation time, usually by waiting for a clock event. Best practice is to separate the combinatoric and sequential parts into two different processes as shown in this minimal example of your state machine:

library ieee;
use ieee.std_logic_1164.all;

entity foo is

end entity foo;

architecture bar of foo is
  type SM_STATES is (state_column_1, scan_col_1, state_column_2, scan_col_2, state_column_3, scan_col_3);
  signal my_state, my_state_next : SM_STATES;
  signal bcd_val                 : std_logic_vector(3 downto 0) := "1110";
  signal clk                     : std_logic                    := '1';
  signal nRST                    : std_logic                    := '0';
  signal scanned_val             : std_logic_vector(6 downto 0);
  signal original_col            : std_logic_vector(2 downto 0);
  signal scan_complete           : std_logic;
begin  -- architecture bar

  comb : process (my_state, bcd_val) is
  begin  -- process baz
    case my_state is

      when state_column_1 =>
        scanned_val   <= (others => '0');
        original_col  <= "110";
        my_state_next <= scan_col_1;

      when scan_col_1 =>
        case bcd_val is
          when "1110" => scanned_val <= "1100100";  -- 1 wrong
          when "1101" => scanned_val <= "1100010";  -- 2 wrong
          when others => scanned_val <= "0010000";
        end case;
        my_state_next <= state_column_2;

      when state_column_2 =>
        original_col  <= "011";
        my_state_next <= scan_col_2;

      when scan_col_2 =>
        case bcd_val is
          when "1110" => scanned_val <= "1011011";  -- 5 wrong
          when "1101" => scanned_val <= "1011111";  -- 6 wrong
          when others => scanned_val <= "0000000";
        end case;
        my_state_next <= state_column_3;

      when state_column_3 =>
        original_col  <= "101";
        my_state_next <= scan_col_3;

      when scan_col_3 =>                -- Reading S1 // The only working state
        case bcd_val is
          when "1110" => scanned_val <= "1100000";  -- 9/ 1
          when "1101" => scanned_val <= "0111110";  -- X/ 2
          when others => scanned_val <= "0000000";
        end case;
        my_state_next <= state_column_1;  -- ************ Error might be here
        scan_complete <= '1';           -- ********** Error might be here

      when others => scanned_val <= "0000000";
    end case;
  end process comb;

  process (clk, nRST) is
  begin  -- process
    if nRST = '0' then                  -- asynchronous reset (active low)
      my_state <= state_column_1;
    elsif clk'event and clk = '1' then  -- rising clock edge
      my_state <= my_state_next;
    end if;
  end process;

  -- this clock and reset signal would usually be supplied from the outside
  clk  <= not clk after 10 ns;
  nRST <= '1'     after 101 ns;
end architecture bar;

If we run this file now within a simulator we can see the states switching with each clock cycle:

Answer 2

You can think of this as building on iterationbound's answer.

I implemented your complete state machine process:

library ieee;
use ieee.std_logic_1164.all;

entity foo is

end entity foo;

architecture fum of foo is

    type SM_STATES is ( 
        state_column_1, scan_col_1, 
        state_column_2, scan_col_2, 
        state_column_3, scan_col_3
    );

    signal my_state:        SM_STATES;  -- defaults to SM_STATES'LEFT

    signal state_inc:       std_logic := '0';

    signal bcd_val:         std_logic_vector(3 downto 0) := "1110";
    signal clk:             std_logic := '0';

    signal scanned_val:     std_logic_vector(6 downto 0);
    signal original_col:    std_logic_vector(2 downto 0);
    signal scan_complete:   std_logic;

begin

scanner_sm: 
    process (clk)
    begin
        if clk'event and clk = '1' then

            if state_inc = '1' then

                scan_complete <= '0';

                case my_state is

                    when state_column_1 =>
                        scanned_val  <= (others => '0');
                        original_col   <= "110";
                        my_state <= scan_col_1;

                    when scan_col_1 =>
                        case bcd_val is
                            when "1110" => scanned_val <= "1100100";  -- 1 wrong
                            when "1101" => scanned_val <= "1100010";  -- 2 wrong
                            when others => scanned_val <= "0010000";
                        end case;
                        my_state <= state_column_2;

                    when state_column_2 =>
                        original_col   <= "011";
                        my_state <= scan_col_2;

                    when scan_col_2 =>
                        case bcd_val is
                            when "1110" => scanned_val <= "1011011";  -- 5 wrong
                            when "1101" => scanned_val <= "1011111";  -- 6 wrong
                            when others => scanned_val <= "0000000";
                        end case;
                        my_state <= state_column_3;

                    when state_column_3 =>
                        original_col   <= "101";
                        my_state <= scan_col_3;

                    when scan_col_3 => -- Reading S1 // The only working state
                        case bcd_val is
                            when "1110" => scanned_val <= "1100000";  -- 9/ 1
                            when "1101" => scanned_val <= "0111110";  -- X/ 2
                            when others => scanned_val <= "0000000";
                        end case;
                        my_state <= state_column_1; -- ************ Error might be here
                        scan_complete <= '1'; -- ********** Error might be here

                    when others => scanned_val <= "0000000";
                end case;

          end if;
      end if;
    end process scanner_sm;

CLOCK:
    process 
    begin
       wait for 10 ns;
       clk <= not clk;
       if Now > 200 ns then
           wait;
       end if;
    end process;

STIMULUS:
    process
    begin
        wait for 30 ns;
        state_inc <= '1';
        wait;
    end process;

end architecture;

You can see there is no next state. Guess what it works.

I delayed state_inc by one clock before asserting to show why you should reset:

You can see the state of scan_complete, scanned_val and original_col are originally unknown until they are written. You might as well also reset my_state.

You can see from both the above simulation and iterationbound's that you're state machine traverses all six states. If it get's stuck somewhere you'd expect state_inc isn't going valid (and it's also not apparent what makes it go invalid from your process, either).

Other than not having resets there's nothing apparently wrong with your state machine, the problem appears to lay elsewhere or relates to state boundary operation stopping an occurrence of state_inc.

I would have though you might need an additional state for signalling scan complete and starting the next scan, otherwise any state_inc still valid would cause the next round of state transitions. (Between scan_col3 and state_column_1, in keeping with your comments "************ Error might be here").

Adding resets and an additional state (name - complete?) that only lasts one clock could be done by:

 if state_inc = '1' or my_state = complete then

where the present

  if state_inc = '1' then

is.

That would give whatever the chance to react to scan_complete before going to state_column_1. You might one to check if complete should be named start_or_complete, and be the first state.

I the only thing I did to your process was change the indentation to make it easier to see where in the if statements everything fell.

(And we did warn you we'd want to see more of your model)