The states in this FSM machine are changing too quickly due to an issue with the clock updating the present state

Question

I'm in the process of implementing a finite state machine in verilog, and I've encountered an issue. However, I know what the problem is, but I'm not sure how to fix it.

This is my current code:

module moore_SM(clk, rstn, btn, z, rstLED, state);

  //Port Assignments
  input clk, rstn;
  input [2:0] btn;
  output  z;
  output reg rstLED;
  output reg [5:0] state;

  //Internal Port Assignments
  reg [1:0] w, x; //NOTE: This is typically the input in FSM, 
                  //but it is internal because there is a conversion from btn to w.
  reg [2:0] y, Y; //Present state and next state respectively
  reg [2:0] pstBtn; 
  reg [31:0] cnt;

  //Define some parameters
      //Input Type (i.e. A, B or C)
      parameter [1:0] A = 2'b00, B = 2'b01, C = 2'b11, DC = 2'b10; //DC => don't care - shouldn't affect FSM

      //State (i.e. S1, S2, S3, S4, S5 or S6)
      parameter [2:0] S1 = 3'b000, S2 = 3'b001, S3 = 3'b010, S4 = 3'b011, S5 = 3'b100, S6 = 3'b101;

  initial begin 
      state = 0;
  end

  //Determine which button is active
  always @(*)
      begin
            case(btn)
                3'b110: begin w = A; end
                3'b101: begin w = B; end
                3'b011: begin w = C; end
            //  default: w = DC;
            endcase
      end

  //Define the next state and output combinational circuits
  always @(w,y)
    begin
        case(y)

            S1: begin
                    state = 6'b000001;
                    if(w == A) begin Y = S2; end
                    else begin Y = S1; end
                 end

            S2: begin
                    state = 6'b000010;
                    if(w == B) begin Y = S3; end
                    else begin if(w == A) begin Y = S2; end
                    else Y = S1; end
                 end

            S3: begin
                    state = 6'b000100;
                    if(w == A) begin Y = S2; end
                    else begin if(w == B) begin Y = S4; end
                    else Y = S1; end 
                 end

            S4: begin
                    state = 6'b001000;
                    if(w == A) begin Y = S5; end
                    else begin Y = S1; end
                 end

            S5: begin
                    state = 6'b010000;
                    if(w == A) begin Y = S2; end
                    else begin if(w == B) begin Y = S3; end
                    else Y = S6; end
                 end

            S6: begin 
                    state = 6'b100000;
                    if(w == A) begin Y = S2; end
                    else begin Y = S1; end
                 end

            //default: Y = 3'b111;

        endcase

      end

    //assign z = (Y == S2);

  //Define the sequential block
  always @(posedge clk)
    begin
        y <= Y;
    end

endmodule

This code is supposed to identify the pattern ABBAC. I believe the logic accomplishes this purpose.

However, I've encountered a problem. When I press one of the three buttons, the first always block - always @(*) - executes and evaluates the change. The state of the button is encoded and saved into w. At the same time, the next always block - always @(w,y) - senses a change and determines in which state to reside. Like I expected, if I send input A, the machine moves from S1 into S2.

Now if I send input B, the machine moves back into state S1. After I changed a few things around, I identified what I believe to be the problem. Note the last always block: always @(posedge clk). This always block continuously updates the present state, y. When I press a button, I assume what is happening is that the second always block, always @(w,y), is receiving an updated y at 50mHz (in my case), which is obviously much faster than I can press and release a button. Assuming I'm pressing the B button while in state S2, the state quickly changes to S3 to S4 and then to S1 because of the continuous update.

That said, I'm having some trouble coming up with a way to fix this issue. I basically need some way to change only one state per button press.

Note that I'm primarily testing this code on an experimental board. I did write a testbench, but the simulation does not seem to match what I'm seeing on the design board. In case anyone wants to see the testbench code, I posted it below:

`timescale 1ns/1ns
module moore_SM_TB();
    reg clk;
    reg [2:0] btn;
    wire [5:0] state;
    wire z;
    wire rstLED;

  initial begin 
    clk = 1;
    #0 btn = 3'b111;
    #50 btn = 3'b110;
    #50 btn = 3'b111;
    #50 btn = 3'b101;
    #50 btn = 3'b111;
    #50 btn = 3'b011;
    #50 btn = 3'b111;
 end

always begin
  #20 clk = ~clk;    
end

  //MUT
  moore_SM MUT (clk, 0, btn, z, rstLED, state);

endmodule

NOTE: I believe the solution will ultimately involve modifying how the buttons are handled. When I release a button, this also registers as a change. Hence, I had to comment out the default case because it would also mess up which state the machine is in.

UPDATE: I wrote a reduced version of the FSM I posted above to try and debug the code. It tries to detect the sequence AB.

module moore_SM(clk, btn, rstN, state, rstLED);
//Port assignments
input clk;
input rstN;
input [2:0] btn;
output reg rstLED;
output reg [5:0] state;

//Internal Port Assignments
reg [1:0] w;
reg [2:0] y, Y;
reg [2:0] pstBtn;

//Define some parameters
parameter [1:0] A = 2'b00, B = 2'b01, C = 2'b11, DC = 2'b10;
parameter [2:0] S1 = 3'b000, S2 = 3'b001, S3 = 3'b010, S4 = 3'b011, S5 = 3'b100, S6 = 3'b101;

//Initialize some values to prevent Quartus from doing
initial 
    begin
        y = S1;
        Y = y;
        state = 0;
        rstLED = 0;
    end

always @(*)
    begin

        if(btn == pstBtn) begin w = DC; end

        else begin
            case(btn)
                3'b110: w = A;
                3'b101: w = B;
                3'b011: w = C;
                default: w = DC;
            endcase
        end

    end

always @(*)
    begin
        case(y)
            S1: begin
                    state = 6'b000001;
                    if(w == A) begin Y = S2; end
                    else begin Y = S1; end
                 end

            S2: begin
                    state = 6'b000010;
                    if(w == B) begin Y = S3; end
                    if(w == DC) begin Y = S2; end
                    else begin Y = S1; end
                 end

            S3: begin
                    state = 6'b000100;
                    if(w == DC) begin Y = S3; end
                    else begin Y = S1; end
                 end
            default: begin state = 6'b100000; Y = S1; end
        endcase
    end



//Update state and check for reset signal
always @(posedge clk, negedge rstN)
    begin
        pstBtn <= btn;

        if(rstN == 0) begin y <= S1; rstLED <= 1; end
        else begin y <= Y; rstLED <= 0; end
    end

endmodule

Note that I'm trying to "sample" the button state in the last always block. I've used this method in the past, but it does not seem to work here. Whenever I press a button, there is no change in state. I'm wondering if this is a timing issue now.

Answer 1

If the button input is coming a switch rather than test stimulus it will be asynchronous and so should be put through 2 meta-stability flip-flops. To change one state per button press create an edge detection.

Edge detection will create a 1 clock period wide pulse, resulting in 1 transition per press.

reg [2:0] btn0_meta;
always @(posedge clk or negedge rstN) begin
  if (~rstN) begin
    btn0_meta <= 'b0;
  end
  else begin
    btn0_meta <= {btn0_meta[1:0], btn[0]};
  end
end

reg btn0_posedge;
always @* begin
  btn0_posedge = btn0_meta[1] & ~btn_meta[2]; 
end

Do this for all button inputs and use the btnX_posedge in your state machine.