VHDL state machine testbench - works when on board but not on simulation

Question

I have the VHDL implementation that works on board, it detects the sequence 01110 and will raise a flag for 2 clock counts. It detects overlapping sequences as well where 011101110 would raise the flag twice.

I've checked my implementation with a logic analyzer on the board and am fairly confident that it works. I am feeding in a repetition sequence of 0111 at 10 kHz, on the board, it has a clock at 100 MHz where I scale it to 10 kHz with a prescaler.

My problem is, when trying to recreate a similar scenario using a simulation, I do not get any outputs as expected

Image from logic analyzer from board

Image from Test Bench

Test Bench Code

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;

entity test_FSM_prac4 is
--  Port ( );
end test_FSM_prac4;

architecture Behavioral of test_FSM_prac4 is

component FSM_prac4 is 
    port ( 
       inputSignal : in STD_LOGIC;
       pushButton : in  STD_LOGIC;
       clk100mhz : in STD_LOGIC;
       logic_analyzer : out STD_LOGIC_VECTOR (7 downto 0);
       LEDs: out STD_LOGIC
); end component;

signal inputSignal : std_logic := '0';
signal pushButton: std_logic := '0';
signal clk100mhz: std_logic := '0';
signal logic_analyzer: std_logic_vector(7 downto 0);
signal LEDs : std_logic;

begin

uut : FSM_prac4 port map(
    inputSignal => inputSignal,
    pushButton => pushButton,
    clk100mhz => clk100mhz,
    logic_analyzer => logic_analyzer,
    LEDs => LEDs
    );

--generate clock 100mhz
clock_tic: process begin
    loop
    clk100mhz <= '0';
    wait for 5ns;
    clk100mhz <= '1';
    wait for 5ns;
    end loop;
end process;

input_changes: process begin
    loop
    inputSignal <= '0';
    wait for 100us;
    inputSignal <= '1';
    wait for 100us;
    inputSignal <= '1';
    wait for 100us;
    inputSignal <= '1';
    wait for 100us;
    end loop;
end process;   

end Behavioral;

To show the mapping for logic Analyzer

logic_analyzer(0) <= masterReset;
logic_analyzer(1) <= newClock -- 10Khz Clock;
logic_analyzer(2) <= outputZ;
--FSM States
logic_analyzer(3) <= '1' when y = A ELSE '0';
logic_analyzer(4) <= '1' when y = B ELSE '0';
logic_analyzer(5) <= '1' when y = C ELSE '0';
logic_analyzer(6) <= '1' when y = D ELSE '0';
logic_analyzer(7) <= '1' when y = E ELSE '0';

If anyone could direct to what I am doing wrong on the test bench and how to replicate to get similar results as the first image as it shows that in simulation, it always stays at state A and the new clock is not toggling meaning that clk100mhz is somehow not connected but I can't figure out why.

Any help is greatly appreciated, thanks guys

edit:

I wrote a simple program to test my scalar clock

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;

entity scaler_clk is
Port ( 
           pushButton : in std_logic;
           indicator : out std_logic;
           clk100mhz : in STD_LOGIC;
           clk10khz: out STD_LOGIC
           );
end scaler_clk;

architecture Behavioral of scaler_clk is
    signal clockScalers : std_logic_vector (12 downto 0):= (others => '0') ;
    signal prescaler: std_logic_vector(12 downto 0) := "1001110001000";
    signal newClock: std_logic := '0'; 
    signal masterReset : std_logic;     

begin

    clk10khz <= newClock;
    masterReset <= pushButton;

    process (clk100mhz,masterReset) begin
        if(masterReset <= '1') then <--- error occurs here
            clockScalers <= "0000000000000";
            newClock <= '0';
            indicator <= '1';
        elsif (clk100mhz'event and clk100mhz = '1')then
            indicator <= '0';
            clockScalers <= clockScalers + 1;
            if(clockScalers > prescaler) then
                newClock <= not newClock;
                clockScalers <= (others => '0');
            end if;
        end if;
    end process;

end Behavioral;

test bench code

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;

entity test_scaler_clk is
--  Port ( );
end test_scaler_clk;

architecture Behavioral of test_scaler_clk is

    component scaler_clk Port ( 
            pushButton : in std_logic;
            indicator : out std_logic;
           --input clock
           clk100mhz : in STD_LOGIC;
           clk10khz: out STD_LOGIC
           );end component;

    signal clk100mhz: std_logic := '0';
    signal clk10khz : std_logic;
    signal pushButton: std_logic;
    signal indicator :  std_logic;

begin

uut: scaler_clk port map(
    pushButton => pushButton,
    indicator => indicator,
    clk100mhz => clk100mhz,
    clk10khz => clk10khz
    );    

    pushButton <= '0';
    clock_tic: process begin
        loop
        clk100mhz <= '0';
        wait for 5ns;
        clk100mhz <= '1';
        wait for 5ns;
        end loop;
    end process;


end Behavioral;

Even though I set pushButton to '0', it is still triggering masterReset, anyone knows why, that's why the 10 kHz clock isn't working

Answer 1

There are several things that you could (should) improve in your code. As Brian already explained, in your Behavioral architecture of scaler_clk, you should have:

if(masterReset = '1') then

instead of:

if(masterReset <= '1') then

Now, let's start with the most likely cause of your initial problem: unbound components. Your test benches instantiate the design to validate as components. VHDL components are kind of prototypes of actual entities. Prototypes are enough to compile because the compiler can perform all necessary syntax and type checking. But they are not enough to simulate because the simulator also needs the implementation behind the prototype. Some tools have a default binding strategy for unbound components: if they find an entity with the same name and if it has only one architecture, they use that. Your simulator apparently does not use such strategy (at least not by default, there is maybe an option for that but it is disabled). Note that most simulators I know issue warnings when they find unbound components. You probably missed these warnings.

Anyway, your component instances are unbound (they have no associated entity/architecture) and the simulator considers them as black boxes. Their outputs are not driven, except by the initial values you declared (1).

How to fix this? Two options:

1. Use a configuration to specify which entity/architecture pair shall be used for each component instance:

   for all: scaler_clk use entity work.scaler_clk(Behavioral);
   

2. Use entity instantiations instead of components:

   uut: entity work.scaler_clk(Behavioral) port map...
   

Now, let's go through some other aspects of your code that could be improved:

1. You are using non-standard packages, that are frequently not even compatible: IEEE.STD_LOGIC_ARITH and IEEE.STD_LOGIC_UNSIGNED. As they are not standard they should not even be in the standard IEEE library. You should use IEEE.NUMERIC_STD instead, and only that one. It declares the SIGNED and UNSIGNED types (with the same declaration as STD_LOGIC_VECTOR) and overloads the arithmetic operators on them.

2. Your test benches generate the 100MHz clock with:

   clock_tic: process begin
       loop
       clk100mhz <= '0';
       wait for 5ns;
       clk100mhz <= '1';
       wait for 5ns;
       end loop;
   end process;
   

   The infinite loop is useless: a process is already an infinite loop:

   clock_tic: process
   begin
       clk100mhz <= '0';
       wait for 5ns;
       clk100mhz <= '1';
       wait for 5ns;
   end process clock_tic;
   

   would do the same. Same remark for your input_changes process.

3. Your input_changes process uses wait for <duration> statements. This is not a good idea because you do not know when the inputSignal signal toggles, compared to the clock. Is it just before, just after or exactly at the same time as the rising edge of clk100mhz? And if it is exactly at the same time, what will happen? Of course, you can carefully chose the <durations> to avoid such ambiguities but it is error prone. You should use the wait for <duration> only in the clock generating process. Everywhere else, it is better to synchronize with the clock:

   input_changes: process
   begin
       inputSignal <= '0';
       for i in 1 to 10000 loop
           wait until rising_edge(clk100mhz);
       end loop;
       inputSignal <= '1';
       for i in 1 to 10000 loop
           wait until rising_edge(clk100mhz);
       end loop;
       inputSignal <= '1';
       for i in 1 to 10000 loop
           wait until rising_edge(clk100mhz);
       end loop;
       inputSignal <= '1';
       for i in 1 to 10000 loop
           wait until rising_edge(clk100mhz);
       end loop;
   end process input_changes;
   

   This guarantees that inputSignal changes just after the rising edge of the clock. And you could rewrite it in a bit more elegant way (and probably a bit easier to maintain):

   input_changes: process
       constant values: std_logic_vector(0 to 3) := "0111";
   begin
       for i in values'range loop
           inputSignal <= values(i);
           for i in 1 to 10000 loop
               wait until rising_edge(clk100mhz);
           end loop;
       end loop;
   end process input_changes;
   

4. You are using resolved types (STD_LOGIC and STD_LOGIC_VECTOR). These types allow multiple drive, that is, having a hardware wire (VHDL signal) that is driven by several devices (VHDL processes). Usually you do not want this. Usually you even want to avoid this like the plague because it can cause short-circuits. In most cases it is wiser to use non-resolved types (STD_ULOGIC and STD_ULOGIC_VECTOR) because the compiler and/or the simulator will raise errors if you accidentally create a short circuit in your design.

5. One last thing: if, as its name suggests, you intend to use the clk10khz signal as a real clock, you should reconsider this decision. It is a signal that you generate with your custom logic. Clocks have very specific electrical and timing constraints that cannot really be fulfilled by regular signals. Before using clk10khz as a clock you must deal with clock skew, clock buffering... Not impossible but tricky. If you did use it as a clock your synthesizer probably issued critical warnings that you also missed (have a look maybe at the timing report). Moreover, this is probably useless in your case: an enable signal generated from clk100mhz could probably be used instead, avoiding all these problems. Instead of:

   process (clk100mhz,masterReset) begin
       if(masterReset = '1') then
           clockScalers <= "0000000000000";
           newClock <= '0';
           indicator <= '1';
       elsif (clk100mhz'event and clk100mhz = '1')then
           indicator <= '0';
           clockScalers <= clockScalers + 1;
           if(clockScalers > prescaler) then
               newClock <= not newClock;
               clockScalers <= (others => '0');
           end if;
       end if;
   end process;
   

   use:

   signal tick10khz: std_ulogic;
   ...
   process(clk100mhz, masterReset) begin
       if masterReset = '1') then
           clockScalers <= "0000000000000";    
           tick10khz <= '0';
       elsif rising_edge(clk100mhz) then
           clockScalers <= clockScalers + 1;
           tick10khz <= '0'
           if(clockScalers > prescaler) then
               tick10khz <= '1';
               clockScalers <= (others => '0');
           end if;
       end if;
   end process;
   

   And then, instead of:

   process(clk10khz)
   begin
       if rising_edge(clk10khz) then
           register <= register_input;
       end if;
   end process;
   

   use:

   process(clk100mhz)
   begin
       if rising_edge(clk100mhz) then
           if tick10khz = '1' then
               register <= register_input;
           end if;
       end if;
   end process;
   

   The result will be the same but with only one single 100MHz clock, which avoids clock skew, clock buffering and clock domain crossing problems.

(1) This illustrates why declaring variables and signals with initial values is usually not a good idea: it hides potential problems. Without this your signals would have been stuck at 'U' (uninitialized) and it would maybe have helped understanding where the problem comes from.