0
\$\begingroup\$

I'm trying to write some vhdl that detects a given pattern in a string of bits. The circuit should output 1 when it finds the pattern "110" in the input stream. My input is "X" and my output is "Z".

For some reason when I simulate the results, I'm not getting any output for "Z". It just stays low. This is what I have so far:

library IEEE;
 use IEEE.STD_LOGIC_1164.ALL;

 entity checker is
     Port ( clk : in  STD_LOGIC;
            x : in  STD_LOGIC;
            z : out  STD_LOGIC);
 end checker;

 architecture Behavioral of checker is

 type state_type is (S0, S1, S2);
 signal pr_state: state_type := S0;
 signal nx_state: state_type := S0;

 begin

 process(clk) begin
     if (rising_edge(clk)) then
         pr_state <= nx_state;
     end if;
 end process;

 process(pr_state, nx_state) begin

case (pr_state) is 

    when S0 => z <= '0';
        if (x = '1') then 
            nx_state <= S1;
        else 
            nx_state <= S0;
        end if;

    when S1 => z <= '0';
        if (x = '1') then
            nx_state <= S2;
        else 
            nx_state <= S1;
        end if;

    when S2 => z <= '1';
        if (x = '0') then
            nx_state <= S0;
        else
            nx_state <= S2;
        end if;

    when others => z <= '0';

end case;

 end process;

 end Behavioral;

Any thoughts? Appreciate your feedback.

Test bench Code:

LIBRARY ieee;
USE ieee.std_logic_1164.ALL;

ENTITY checker_tb IS
END checker_tb;

ARCHITECTURE behavior OF checker_tb IS 

-- Component Declaration for the Unit Under Test (UUT)

COMPONENT checker
PORT(
     clk : IN  std_logic;
     x : IN  std_logic;
     z : OUT  std_logic
    );
END COMPONENT;


    --Inputs
   signal clk : std_logic := '0';
   signal x : std_logic := '0';

--Outputs
   signal z : std_logic;

   -- Clock period definitions
   constant clk_period : time := 10 ns;

   BEGIN

-- Instantiate the Unit Under Test (UUT)
   uut: checker PORT MAP (
          clk => clk,
          x => x,
          z => z
        );

   -- Clock process definitions
   clk_process :process
   begin
    clk <= '0';
    wait for clk_period/2;
    clk <= '1';
    wait for clk_period/2;
   end process;

   x_process :process
   begin
    x <= '1';
    wait for 100ns;
    x <= '1';
    wait for 100ns;
    x <= '0';
    wait for 100ns;
   end process;

   stim_proc: process begin     
  wait for 100 ns;  
  wait for clk_period*10;
  wait;
   end process;

END;
\$\endgroup\$
  • 1
    \$\begingroup\$ Can we see the testbench code you used to simulate this? \$\endgroup\$ – Dave Tweed Dec 11 '12 at 2:34
  • \$\begingroup\$ sure. coming right up in a sec. \$\endgroup\$ – codedude Dec 11 '12 at 2:35
  • \$\begingroup\$ Your x_process should probably be clocked by the clk signal; as it is, your stimulus bits are 10 times the length of your clock period. \$\endgroup\$ – Dave Tweed Dec 11 '12 at 2:51
3
\$\begingroup\$

There are several issues with your code. My rewritten version is below.

The main problem is that your code was that the assignment of Z was incorrect. The next problem was that the state machine itself was incorrect. As it was written, Z should have gone high after a pattern of "11", and not "110". It also would have gotten stuck in state S2 and not recovered.

I should also mention that there were several "stylistic" issues with your code too. Having two processes instead of one was a major one. I cleaned up that as well. This allowed for having only a single state signal, which makes the whole thing more readable as well.

 library IEEE;
 use IEEE.STD_LOGIC_1164.ALL;

 entity checker is
   Port ( clk : in  STD_LOGIC;
            x : in  STD_LOGIC;
            z : out STD_LOGIC);
 end checker;    

 architecture Behavioral of checker is    
   type state_type is (S0, S1, S2);
   signal state: state_type := S0;
 begin

     process(clk)
     begin
       if rising_edge(clk) then
         case state is 
             when S0 => 
                z <= '0';
                if x='1' then 
                  state <= S1;
                else 
                  state <= S0;
                end if;

             when S1 => 
                z <= '0';
                if x = '1' then 
                  state <= S2;
                else 
                  state <= S1;  -- S1, not S0 because we also want to detect a "111...1110" 
                end if;

             when S2 => 
                if x = '0' then
                  state <= S0;
                  z <= '1';  -- Z='1' only when a match is made
                else
                  state <= S0;  -- Goes back to S0 to detect the next pattern
                  z <= '0';
                end if;

             when others => 
                z <= '0';
                state <= S0;  -- In case the state machine is in an invalid state 
         end case;
       end if;    
     end process;

   end Behavioral;

Update: Here's a rewritten testbench:

library ieee;
use ieee.std_logic_1164.all;

entity testbench is
end testbench;

architecture arch_testbench of testbench is
  component checker
    port (clk   :in  std_logic;
          x     :in  std_logic;
          z     :out std_logic);
  end component;

  signal clk    :std_logic := '1';
  signal x      :std_logic := '0';
  signal z      :std_logic := '0';

  signal sr     :std_logic_vector (15 downto 0) := "0001100011111110";
begin

  process (clk)
  begin
    if clk='1' then
      clk <= '0' after 5.0 ns, '1' after 10 ns;
    end if;
  end process;


  process (clk)
  begin
    if rising_edge(clk) then
      sr <= sr(sr'high-1 downto 0) & "0";
    end if;
  end process;

  x <= sr(sr'high);

  UUT:  checker
      port map (clk, x, z);

end arch_testbench;
\$\endgroup\$
  • \$\begingroup\$ Really appreciate your help here! \$\endgroup\$ – codedude Dec 11 '12 at 4:11
  • \$\begingroup\$ Hmm. For some reason my prof always used two processes in his FSMs. Not sure why now that I see your method. Definitely a lot simpler. \$\endgroup\$ – codedude Dec 11 '12 at 4:18
  • 2
    \$\begingroup\$ +1 for single process SM. For further simplification, assign state <= S0 and z <= '0' just before the case statement and clean up the redundant copies (and the "when others" clause!). Use the testbench to verify the behaviour is unchanged... \$\endgroup\$ – Brian Drummond Dec 11 '12 at 11:28
  • \$\begingroup\$ If I wanted to generate excitation equations for this, how would I go about that? \$\endgroup\$ – codedude Dec 11 '12 at 15:34
  • 1
    \$\begingroup\$ @David - I disagree that default assignments reduce readability (However that may be familiarity, and some style guidelines may prohibit them) In their favour, as well as the DRY principle, they concentrate the "default options" in a single place instead of scattering them widely where they can easily be missed when making changes. Your point about the "safe SM" behaviour of some synth tools is worth making; however you usually also need attributes or compile options to turn off synthesis optimisations before you actually get the recovery logic! \$\endgroup\$ – Brian Drummond Dec 11 '12 at 16:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.