Search code examples
vhdlfsm

Having troubles with running FSM on Nexys2


I am trying to run a simple FSM where LEDs are scanned. I have applied this logic by shifting the bits to left, used & operator for that. It does not shift at all only the LSB glows and that is it, i slowed down the clock as well, using 1.5Hz clock. Will someone please tell me whats wrong here.

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
entity scan is
Port ( 
            clk         : in    STD_LOGIC;
            led         : out   STD_LOGIC_VECTOR (7 downto 0);
            reset       : in    STD_LOGIC
        );
end scan;
architecture Behavioral of scan is

Type state is
    (
    RESET_ST,
    S1
    );

    signal n_state : state;
    signal c_state : state;

    signal input_temp   :unsigned (7 downto 0):= "00000001";

begin
--------------------------------------------------------------------------
--------------------------CURRENT STATE ASSIGNMENT------------------------
--------------------------------------------------------------------------

STATE_ASSIGNMENT: process (clk, reset)
    begin
    if (reset = '1') then
            c_state <= RESET_ST;
    elsif  (clk'event and clk = '1') then
            c_state <= n_state;
    end if;
    end process STATE_ASSIGNMENT;


--------------------------------------------------------------------------
----------------------------- INTPUT BLOCK--------------------------------  
--------------------------------------------------------------------------

INPUT_BLOCK : process (c_state) 
    begin
case (c_state) is

when RESET_ST   =>  
input_temp  <= "00000001";
n_state         <= S1;

when S1             =>
    input_temp  <= input_temp (6 downto 0) & '0';
    n_state         <= S1;
when others => 
n_state         <= RESET_ST;

end case;
end process;
--------------------------------------------------------------------------
----------------------------- OUTPUT BLOCK--------------------------------  
--------------------------------------------------------------------------

OUTPUT_BLOCK : process (c_state, input_temp)
begin   

case (c_state) is

when RESET_ST               =>
led <= std_logic_vector (input_temp);

when S1                     =>
led <= std_logic_vector (input_temp);
when others                 =>
led                         <= (others => '1');

end case;
end process OUTPUT_BLOCK;


end Behavioral;

Solution

  • There are two immediately visible things wrong.

    First counter is not declared (comment it out, easy enough).

    Second, once in S1 n_state <= S1, in other words you go to S1 and sit there. The consequence of this is that process INPUT_BLOCK doesn't have a trigger event - the sensitivity contains only c_state and there is no further change in c_state.

    scan_tb, a single state transition

    I'd imagine Brian Drummond would be telling you about now to use one process for your FSM. Essentially input_temp should be changed to something with storage and moved into the clocked process.

    You could note there isn't anything to detect when input_temp goes static (all '0's), either.

    Addendum

    From your comment:

    Okay so if i add the next state i.e. n_state in the sensitivity list, will it work?

    No. If you look at the waveform above n_state always contains S1.

    Secondly yeah when it goes all 0's I wont see anything, but what about the shifting part?

    Eventually that one '1' will get lost once it has reached input_temp(7).

    I defined the outputs explicitly for each state, should i put a limit here?

    There are three things you could do. 1. let the outputs all go to '0's, 2. recirculate the '1' (a Johnson counter) or 3. stop with some LED displaying.

    If the states were one hot the new 8 states could drive an LED each. – David Koontz 2 hours ago

    You:

    Could you by any chance show me an example or something? It would help me more to understand

    In general this is not the correct venue for teaching basic VHDL or digital design skills, certainly not in a comment thread. It's a venue for asking and answering specific VHDL questions. See How do I ask a good question?

    You asked:

    Will someone please tell me whats wrong here.

    I answered, including a picture.

    Right about here you could note that the waveform image above conflicts with the first paragraph of your question:

    It does not shift at all only the LSB glows and that is it, i slowed down the clock as well, using 1.5Hz clock.

    If you note in the waveform it shifts exactly once, with your code unmodified (other than to remove the assignment to an undeclared counter which you edited out of your question, see your first comment below).

    What you have defined is a two state state machine either reset or shift. It doesn't work because it's not properly written. Essential it describes an intended shift register (input_temp) that currently shifts left and empties. Your state is a flip flop run off of an asynchronous reset, that when released simply toggles to the other state and supposedly enabled shifts.

    Implement and 8 bit shift register that shift's left (or is connected in reverse order), and can be implemented with a synchronous load (to "00000001") hooked up to reset. 8 clocks later it's all '0's.

    There are nine states defined (one for each of the LEDs that are illuminated and one where all LEDS are extinguished) You can add a 10th state by adding that state flip flop in. You could use 10 flip flops for a one hot state machine, 8 flip flops for just a shift register or 9 to include c_state (and the reset holdover).

    I could I suppose generate three different architectures for the above two paragraphs but I'm not going to do that.

    Here's the simplest implementation with the least amount of change to your code:

    architecture foo of scan is
    
        type state is ( RESET_ST, S1 );
        signal n_state:    state;
        signal c_state:    state;
        -- signal input_temp:  unsigned (7 downto 0):= "00000001";
        signal shft_reg:   std_logic_vector (7 downto 1);
    
    begin
    
    state_assignment: 
        process (clk, reset)
            begin
                if reset = '1' then
                    c_state <= RESET_ST;
                    -- counter <= (others => '0');
                    shft_reg <= (others => '0');
                elsif  clk'event and clk = '1' then
                    c_state <= n_state;
                    if c_state = RESET_ST then
                        shft_reg <= shft_reg (6 downto 1) & '1';
                    elsif shft_reg /= "1000000" then
                        shft_reg <= shft_reg (6 downto 1) & '0';
                    end if;
                end if;
        end process;
    
    --input_block : 
    NEXT_STATE:
        process (c_state) 
        begin
            case (c_state) is
                when RESET_ST =>  
                    -- input_temp <= "00000001";
                    n_state <= S1;
                when S1 =>
                    -- input_temp <= input_temp (6 downto 0) & '0';
                    n_state  <= S1;
                when others => 
                    n_state  <= RESET_ST;
            end case;
        end process;
    
    -- output_block:
    --     process (c_state, input_temp)
    --     begin
    --         case (c_state) is
    --             when RESET_ST =>
    --                 led <= std_logic_vector (input_temp);
    --             when S1 =>
    --                 led <= std_logic_vector (input_temp);
    --             when others  =>
    --                 led <= (others => '1');
    --             end case;
    --     end process;
    
    -- LED0_OUT:
    --     led(0) <= '1' when c_state = RESET_ST else '0';
    LEDOUT:  
        process (c_state, shft_reg)
        begin
            if c_state = RESET_ST then
                led(0) <= '1';
            else
                led(0) <= '0';
            end if;
            led (7 downto 1) <= shft_reg;  -- shft_reg(7 downto 1)
        end process;
    
    end architecture foo;
    
    library ieee;
    use ieee.std_logic_1164.all;
    
    entity scan_tb is
    end entity;
    
    architecture foo of scan_tb is
        signal clk:     std_logic := '0';
        signal reset:   std_logic := '1';
        signal led:     std_logic_vector ( 7 downto 0);
    begin
    
    DUT:
        entity work.scan 
            port map (
                clk => clk,
                led => led,
                reset => reset
            );
    
    CLOCK:
        process
        begin
            wait for 0.33 sec;  -- one half clock period, 1.5 Hz
            clk <= not clk;
            if Now > 20 sec then
                wait;
            end if;
        end process;
    
    STIMULUS:
        process
        begin
            wait until rising_edge(clk);
            wait for 0.33 sec;
            wait until rising_edge(clk);
            reset <= '0';
            wait;        
        end process;
    
    end architecture;
    

    And here's what the waveforms look like:

    scan_tb2 corrected

    Note the Radix for led has been changed in the waveform to binary.

    Also note that the first part of the two waveforms match. I also added a shft_reg state recognizer to freeze shft_reg when led(7) is set.

    You could also note there's an optimization. The first LED is driven off c_state, the remaining 7 are driven off the 7 bit shift register (shft_reg). Also of note that there are only 8 flip flops used.

    And as sonicwave notes in a comment to your question you should really simulate this stuff first, so here's a simple test bench.

    This was simulated, using your entity declaration with the use clause for package numeric_std removed (shft_reg is type std_logic_vector), a new architecture foo and the entity/architecture pair for scan_tb using ghdl-0.31:

    david_koontz@Macbook: ghdl -a scan.vhdl
    david_koontz@Macbook: ghdl -e scan_tb
    david_koontz@Macbook: ghdl -r scan_tb --wave=scan_tb.ghw

    On a Mac running OS X 10.9.3, Where scan_tb.ghw is a ghdl specific waveform dump file format highly suited for VHDL.

    Now, please no more slipping more questions in on the comments to the question you initially asked. Also you could have commented out the assignment to the undeclared signal counter in your sample code instead of editing it out. It ruins the continuity between questions and answers.

    further

    The state assignment process can be written without evaluating c_state:

    state_assignment: 
        process (clk, reset)
            begin
                if reset = '1' then
                    c_state <= RESET_ST;
                    -- counter <= (others => '0');
                    shft_reg <= (others => '0');
                elsif  clk'event and clk = '1' then
                    c_state <= n_state;
    --                if c_state = RESET_ST then
                    if shft_reg = "0000000" then
                        shft_reg <= shft_reg (6 downto 1) & '1';
                    elsif shft_reg /= "1000000" then
                        shft_reg <= shft_reg (6 downto 1) & '0';
                    end if;
                end if;
        end process;
    

    And it does the same thing.

    Now comment a bit more of it out:

    state_assignment: 
        process (clk, reset)
            begin
                if reset = '1' then
                    c_state <= RESET_ST;
                    -- counter <= (others => '0');
                    shft_reg <= (others => '0');
                elsif  clk'event and clk = '1' then
                    c_state <= n_state;
    --                if c_state = RESET_ST then
                    if shft_reg = "0000000" then
                        shft_reg <= shft_reg (6 downto 1) & '1';
    --                elsif shft_reg /= "1000000" then
                    else
                        shft_reg <= shft_reg (6 downto 1) & '0';
                    end if;
                end if;
        end process;
    

    And make the same decision change in the LEDOUT process:

    LEDOUT:  
        process (shft_reg)
        begin
            if shft_reg = "0000000" then
                led(0) <= '1';
            else
                led(0) <= '0';
            end if;
            led (7 downto 1) <= shft_reg;  -- shft_reg(7 downto 1)
        end process;
    

    And you get the scanning LEDs to keep on scanning:

    scan with continuous recirculate

    We've switch LED(0) to be dependent on no other shft_reg positions being set to '1' (not '0').