fsm design for sequance detect (please help me)

[jango123]

hello , i try to make a fsm with a reset and one bit input a and output y to detect the sequence "1101"
i code it with vhdl and simulite it but it dont work
please help me thank you

library ieee;
use ieee.std_logic_1164.all;
entity machine is
port(
      rst,clk,a : in std_logic;
       y : out std_logic);
end machine;

architecture arc of machine is
type m_state is (s1,s2,s3,s4,s5,s6,s7,s8);
signal state : m_state :=s1;
begin
process(clk,rst) is
begin
if (rst='0') then
  state <=s1;
elsif(rising_edge(clk)) then
  case state is
 when s1 =>
                 if a = '1'then
                       state <=s2;
                   else
                       state <=s1;
                   end if;
when s2 =>
                 if a = '0'then
                       state <=s1;
                   else
                       state <=s3;
                   end if;
when s3 =>
                 if a = '0'then
                       state <=s4;
                   else
                       state <=s1;
                   end if;
when s4 =>
                 if a = '0'then
                       state <=s1;
                   else
                       state <=s5;
                   end if;
when s5 =>
                state <=s1;
when s6 =>
                state <=s1;
when s7 =>
                state <=s1;
when s8 =>
                state <=s1;
end case;
end if;
end process;

process(state)
begin
case state is
when s1 | s2 | s3 | s4 | s6 | s7 | s8 => y<='0';
when s5  => y<='0';
end case;
end process;
end arc;

 

[ElecDesigner]

Your rst signal is low so you are never changing state. Look at the beginning of your clocked process - you are always in the if rst='0'.

Not sure what your simulator is but it might help you if you can view the state machine state signal.

In my opinion it would be simpler and more effective to just cascade registers so at any time you have:
A copy of the current input value
A copy of what the input value was last clock
A copy of what the input value was 2 clocks ago
A copy of what the input value was 3 clocks ago

Then you can have another process that just compares these with your required result and does something accordingly. You don't really need a state machine for this job.

 

[Akanimo]

Hi jango123,
Cascading registers will result in an increased resource count so I would not advice you to do that.

The following corrections have the following advantages.
1) It won't miss to sample a on any clock cycle as compared to yours that missed it in "s5".
2) It will yield lesser resource count -- It results in four states compared to yours that had eight states.

Notice that you might want to consider a='1' that completes the sequence as a='1' that begins a new sequence. If so, take option A else take option B.

I only made the corrections and haven't tested this code but it should solve your problem.

[I][B]OPTION A[/B][/I]
...
[B] type m_state is (s1, s2, s3, s4);  --[I]Correction 1[/I][/B]
...
[B]when s3 =>
                 if a = '0'then
                       state <=s4;
                   else
                       state <=s3; --[I]Correction 2[/I]
                   end if;
when s4 =>    
                   state <= s1; --[I]Correction 3[/I][/B]
[B]when others =>
                state <= s1; --[I]Correction 4[/I][/B]
end case;
end if;
end process;


[B]y <= '1' when (state = s4) and (a = '1') else '0';   --[I]Correction 5[/I][/B]
end arc;

[I][B]OPTION B[/B][/I]
...
[B] type m_state is (s1, s2, s3, s4);  --[I]Correction 1[/I][/B]
...
[B]when s3 =>
                 if a = '0'then
                       state <=s4;
                   else
                       state <=s1; --[I]Correction 2[/I]
                   end if;
when s4 =>    
                 if a = '1' then  
                   state <= s2;
                 else
                   state <= s1;
                 end if;               --[I]Correction 3[/I][/B]
[B]when others =>
                state <= s1;       --[I]Correction 4[/I][/B]
end case;
end if;
end process;


[B]y <= '1' when (state = s4) and (a = '1') else '0';   --[I]Correction 5[/I][/B]
end arc;

Remember to set rst to '1' in your testbench to be able to transition from reset state.

 

[vGoodtimes]

You should have better state names.

For example:
SEEN_XXXX -- SEEN_XXXX if 0 else SEEN_1XXX
SEEN_1XXX -- SEEN_XXXX if 0 else SEEN_11XX
SEEN_11XX -- SEEN_110X if 0 else SEEN_11XX
SEEN_110X -- SEEN_XXXX if 0 else SEEN_1XXX (and match is found)

This allows 11011011111 to work correctly.

The original fsm is also incorrect.

The original code also only sets y<='0'. probably and oversight.

 

[Akanimo]

Hi jango123,

...
Notice that you might want to consider a='1' that completes the sequence as a='1' that begins a new sequence. If so, take option A else take option B.
...

This statement that I made in post #3 should be the other way around, disregard it.

Below is the correct statement.

Notice that you might want to consider a='1' that completes the sequence as a='1' that begins a new sequence. If so, take option B else take option A.

pardon my mistake. Thanks.

 

[vGoodtimes]

The OP has multiple FSM errors. With 11101 and 1101101 specifically.

 

[Akanimo]

Hi vGoodtimes,

The OP has multiple FSM errors. With 11101 and 1101101 specifically.

These errors have been taken care of in the corrections I posted in post #3 except that Correction 2 of option B should be:

when s3 =>
                 if a = '0'then
                       state <=s4;
                   else
                       state <=s3; --Correction 2
                   end if;

 

[jango123]

Hi vGoodtimes,

These errors have been taken care of in the corrections I posted.

thank you , i will try it now ,
but bro i have a question , consider the input a is switch , so if the switch is closed , maybe the fsm can read 11111 when the switch is not changing fast lik the clock
any help ?

- - - Updated - - -

Akanimo i have used your correction , it work in simulation , but when i try to simulate it using a switch it dont work
i have used a tina software to simulate the fsm with the input as switch and clk with 1khz

 

[Akanimo]

...
consider the input a is switch , so if the switch is closed , maybe the fsm can read 11111 when the switch is not changing fast lik the clock
any help ?

This question is not very clear. If the switch, 'a' in your port list is held closed ('1') over a period of consecutive clock cycles, then yes, the FSM will see consecutive '1's. As the rising-edge of 'clk' events, the FSM will take the value of 'a' at that time.

 

[jango123]

thank you for those great information , but how to fixe it

 

[Akanimo]

Akanimo i have used your correction , it work in simulation , but when i try to simulate it using a switch it dont work
i have used a tina software to simulate the fsm with the input as switch and clk with 1khz
View attachment 135872

You have SW-HL2 connected to 'L' in your schematic but it should be connected to 'H' for the simulation to work.

 

[jango123]

yes but in the vhdl code i change the reset to be actived on only if is high

 

[jango123]

that is my new code

library ieee;
use ieee.std_logic_1164.all;
entity machine is
port(
      rst,clk,a : in std_logic;
       y : out std_logic);
end machine;

architecture arc of machine is
type m_state is (s1,s2,s3,s4,s5);
signal state : m_state :=s1;
begin
process(clk,rst) is
begin
if (rst='1') then
  state <=s1;
elsif(rising_edge(clk)) then
  case state is
 when s1 =>
                 if a = '1'then
                       state <=s2;
                   else
                       state <=s1;
                   end if;
when s2 =>
                 if a = '0'then
                       state <=s1;
                   else
                       state <=s3;
                   end if;
when s3 =>
                 if a = '0'then
                       state <=s4;
                   else
                       state <=s1; --Correction 2
                   end if;
when s4 =>    
                 if a = '1' then  
                   state <= s5;
                 else
                   state <= s1;
                 end if;               --Correction 3
when s5 =>   if rst='1' then 
                  state <= s1;
               else
                  state <= s5;
                 end if; 
when others =>
                state <= s1;       --Correction 4
end case;
end if;
end process;

y <= '1' when (state = s5)  else '0';   --Correction 5
end arc;

 

[Akanimo]

If 'a' is a switch with states that event at some random time, then your approach of detecting the 1101 sequence is not going to work and this FSM is not going to be able to do the job.

- - - Updated - - -

yes but in the vhdl code i change the reset to be actived on only if is high

That's not right. The former designations (i.e. active-low reset) was okay. All you needed to do was to assign '1' to 'rst' in your testbench. Doing this puts 'rst' in an inactive mode for the purpose of testing the FSM.

It will help if you post your new code here.

- - - Updated - - -

are their any solution to do

Yes there is a solution. You just have to consider only the present state of the switch (i.e. whether it is 'open' or 'closed') or a state change (i.e. whether it closes from an 'open' state or opens from a 'closed' state), whichever does your job.

Better still, if you tell us what you intend to achieve, then we would help you with a solution.

 

[jango123]

thank you bro ,i whant achieve :
when the sequence "1101" by using the input a switch is detected then y=1 until rset =1 (when rset =1 than fsm come to state 1)

 

[Akanimo]

thank you bro ,i whant achieve :
when the sequence "1101" by using the input a switch is detected then y=1 until rset =1 (when rset =1 than fsm come to state 1)

The approach of monitoring a switch for "1101" is not likely going to do your job. The only way this approach can be effective is if the process controlling the switch is synchronous enough to always change the switch from state '1' to state '0' and then back to '1' without two consecutive events of the rising edge of clock. If this is not the case, then you need a different approach.

If this is the case, then post the new code and your testbench with the corrections here so we can look at it to determine what is keeping it from doing the job.

- - - Updated - - -

that is my new code

library ieee;
use ieee.std_logic_1164.all;
entity machine is
port(
      rst,clk,a : in std_logic;
       y : out std_logic);
end machine;

architecture arc of machine is
type m_state is (s1,s2,s3,s4,s5);
signal state : m_state :=s1;
begin
process(clk,rst) is
begin
if (rst='1') then
  state <=s1;
elsif(rising_edge(clk)) then
  case state is
 when s1 =>
                 if a = '1'then
                       state <=s2;
                   else
                       state <=s1;
                   end if;
when s2 =>
                 if a = '0'then
                       state <=s1;
                   else
                       state <=s3;
                   end if;
when s3 =>
                 if a = '0'then
                       state <=s4;
                   else
                       state <=s1; --Correction 2
                   end if;
when s4 =>    
                 if a = '1' then  
                   state <= s5;
                 else
                   state <= s1;
                 end if;               --Correction 3
when s5 =>   if rst='1' then 
                  state <= s1;
               else
                  state <= s5;
                 end if; 
when others =>
                state <= s1;       --Correction 4
end case;
end if;
end process;

y <= '1' when (state = s5)  else '0';   --Correction 5
end arc;

View attachment 135874

It is not working. I didn't include state s5. State s5 is the reason why output y is latched up at '1'. Just take all the corrections I gave you and you should be fine.

Sorry, I didn't see this post until now.

- - - Updated - - -

If you want to latch y once it is activated until rst=1 then what you have to do is change the statement:

y <= '1' when (state = s4) and (a = '1') else '0';

to

y <= '1' when (state = s4) and (a = '1');
But ensure to take all the previous corrections I made.

 

[jango123]

ok bro , but the problem will be in the switching input

 

[Akanimo]

ok bro , but the problem will be in the switching input

With electronics design, implementation is secondary, the idea of how to solve a problem at hand is what drives the approach to implementing the solution. This is why we say tell us your problem so we would help you with a proper approach to solving it.

You're making it difficult. Now you have an effective implementation of a wrong idea. The problem remains.

 

[Akanimo]

Hi jango123,

We didn't hear from you how the FSM behaves in your process.

you said you simulated the code as I corrected and it was okay?
I simulated it myself today and realised I had not considered a condition to reset the latching "y" output. That has been taken care of below.

library ieee;
use ieee.std_logic_1164.all;
entity sequence_detect is
port(
      rst,clk,a : in std_logic;
       y : buffer std_logic := '0');
end sequence_detect;

architecture func of sequence_detect is
type m_state is (s1,s2,s3,s4);
signal state : m_state :=s1;
begin
process(clk,rst) is
begin
if (rst='1') then
  state <=s1;
elsif(rising_edge(clk)) then
  case state is
 when s1 =>
                 if a = '1'then
                       state <=s2;
                   else
                       state <=s1;
                   end if;
when s2 =>
                 if a = '0'then
                       state <=s1;
                   else
                       state <=s3;
                   end if;
when s3 =>
                 if a = '0'then
                       state <=s4;
                   else
                       state <=s3;
                   end if;
when s4 =>    
                 if a = '1' then  
                   state <= s2;
                 else
                   state <= s1;
                 end if;
when others =>
                state <= s1;
end case;
end if;
end process;

y <= '1' when (state = s4) and (a = '1') else '0' when rst = '1';

end func;

Should you not need "y" to be latching, then substitute y <= '1' when (state = s4) and (a = '1') else '0' when rst = '1'; with y := '1' when (state = s4) and (a = '1');

 

[TrickyDicky]

Yes - I recommend you avoid latches - using them is poor design practice. And using buffer is generally frowned on, especally when y is not even used as a buffer.
Also because you have a clocked process, using else to hold the state machine in the same state is not required, as this will happen automatically.