# Synchronuous Combination Lock

I'm trying to implement a synchronous combination lock that will unlock once it receives "101011" using verilog. It has one input: x, and three outputs: unlock, ready, and error. Following these rules:

• In initial state ready=1. Remains in initial while x=0
• Upon receiving 101011 on x, unlock=1. Remains in unlock while x=1
• If in unlock state and x=0, will go to initial state
• In all states other than initial and unlock, if input on x doesn't advance the sequence (101011), error=1.
• Remain in error state while x=1. If x=0 in error state, go to initial state

From these rules I created the state diagram below (I labeled the initial state s_reset, since initial is 000000): I've treated it like a sequence detector and just created a bunch of states. Here's my code below:

module lock
(
input  wire      clock,
input  wire      reset,
input  wire      x,
output reg      ready,
output reg      unlock,
output reg      error
);

reg [2:0] state;
reg [2:0] next_state;

localparam s_reset=3'b000,
s1=3'b001,
s2=3'b010,
s3=3'b011,
s4=3'b100,
s5=3'b101,
open=3'b110,
s_error=3'b111;
s_error=3'b111;

always @ (posedge clock, posedge reset) begin
if(reset)
state <= s_reset;
else
state <= next_state;
end

always @ * begin
case(state)
s_reset : if(x==1'b1) begin
next_state = s1;
unlock = 1'b0;
ready = 1'b0;
error = 1'b0;
end
else begin
next_state = s_reset;
unlock = 1'b0;
ready = 1'b1;
error = 1'b0;
end

s1      : if(x==1'b0) begin
next_state = s2;
unlock = 1'b0;
ready = 1'b0;
error = 1'b0;
end
else begin
next_state = s_error;
unlock = 1'b0;
ready = 1'b0;
error = 1'b1;
end

s2      : if(x==1'b1) begin
next_state = s3;
unlock = 1'b0;
ready = 1'b0;
error = 1'b0;
end
else begin
next_state = s_error;
unlock = 1'b0;
ready = 1'b0;
error = 1'b1;
end

s3      : if(x==1'b0) begin
next_state = s4;
unlock = 1'b0;
ready = 1'b0;
error = 1'b0;
end
else begin
next_state = s_error;
unlock = 1'b0;
ready = 1'b0;
error = 1'b1;
end

s4      : if(x==1'b1) begin
next_state = s5;
unlock = 1'b0;
ready = 1'b0;
error = 1'b0;
end
else begin
next_state = s_error;
unlock = 1'b0;
ready = 1'b0;
error = 1'b1;
end

s5      : if(x==1'b1) begin
next_state = open;
unlock = 1'b0;
ready = 1'b0;
error = 1'b0;
end
else begin
next_state = s_error;
unlock = 1'b0;
ready = 1'b0;
error = 1'b1;
end

open    : if(x==1'b0) begin
next_state = s_reset;
unlock = 1'b1;
ready = 1'b0;
error = 1'b0;
end
else begin
next_state = open;
unlock = 1'b1;
ready = 1'b0;
error = 1'b0;
end

s_error  : if(x==1'b0) begin
next_state = s_reset;
unlock = 1'b0;
ready = 1'b1;
error = 1'b0;
end
else begin
next_state = s_error;
unlock = 1'b0;
ready = 1'b0;
error = 1'b1;
end

default : begin
next_state = s_reset;
unlock = 1'b0;
ready = 1'b1;
error = 1'b0;
end
endcase
end

endmodule


Testbench below

module testbench;
reg         clock;      // Free running clock
reg         reset;      // Reset active high
reg         x;          // Input - X
wire        ready;      // Output - ready for combination
wire        unlock;     // Output - unlocked
wire        error;      // Output - error in combination

initial begin
$dumpfile( "dump.vcd" );$dumpvars;

clock = 0;          // Set initial values for inputs
reset = 0;
x = 0;

#1 reset = 1;
#9 reset = 0;

#30                 // Wait to make sure system is idle

// Test good sequence
x = 1;
#10 x = 0;
#10 x = 1;
#10 x = 0;
#10 x = 1;
#10 x = 1;

// Test to see if stays unlocked and then returns to ready
#30 x = 0;

// Test 1011 which should go into error on the fourth digit
#30 x = 1;
#10 x = 0;
#10 x = 1;
#10 x = 1;

// Stay in error until x -> 0
#30 x = 0;

// Wait in ready and test error on a zero
#30 x = 1;
#10 x = 0;
#10 x = 1;
#10 x = 0;

#30 \$finish;
end

always
#5 clock = ~clock;

lock u1
(
.clock( clock ),
.reset( reset ),
.x( x ),
.ready( ready ),
.unlock( unlock ),
.error( error )
);
endmodule


EDIT: Second always block changed to always@* now unlock and ready seem to be working, but error will equal one even if the sequence is correct. Waveform output shown below:

• Not your issue, but for future reference, the [2:0] in the parameter declaration is pointless. Also for state machines you should be using localparam not parameter as they are not parameters that should be changed externally. Apr 1, 2016 at 19:54
• I thought the [2:0] wasn't doing anything, but saw it on another's code with a similar goal and thought that might've been my problem. Obviously not. Edited code: Removed [2:0] and switched parameter to localparam.
– tlb
Apr 1, 2016 at 20:01
• In the second always block x should be included in the sensitivity list. How do you test your module? Apr 1, 2016 at 20:33
• @tlb always @ (state, next_state) should be always @* (auto-sensitivity list). always @ (state, next_state, x) does work, but it requires you to remember to add all input to your sensitivity list for RTL and synthesized gates to be behaviorally equivalent.
– Greg
Apr 1, 2016 at 20:57
• @Greg changed it to be always @* but I'm still having problems with the error output... the waveform I'm getting has been added
– tlb
Apr 1, 2016 at 21:03

## 3 Answers

Your seeing glitchy behavior on the signals like error because they are pure combinational logic and the input and state-machine are out of phase. You should flop your outputs.

You can do something like this (as an example):

reg  next_ready;
reg  next_unlock;
reg  next_error;
always @ (posedge clock, posedge reset) begin
if(reset) begin
state  <= S_RESET;
ready  <= 1'b0;
unlock <= 1'b0;
error  <= 1'b0;
end
else begin
state  <= next_state;
ready  <= next_ready;
unlock <= next_unlock;
error  <= next_error;
end
end

always @* begin
// default assignment
next_ready  = 1'b0;
next_unlock = 1'b0;
next_error  = error; // <-- keeps previous values unless needed to be changed
//
case(state)
S_RESET : begin
if(x==1'b1) begin
next_state = S1;
end
else begin
next_state = S_RESET;
next_ready = 1'b1;
end
end

S1 : begin
if(x==1'b0) begin
next_state = s2;
end
else begin
next_state = S_ERROR;
next_error = 1'b1;
end
end

....

endcase
end


I uppercased the parameters in my example to match with most coding guild-lines. This helps prevent confusing them with nets/variables.

Perhaps I am missing something, but it seems that a state machine is too complicated for this task. Use a shift register with parallel out (verilog borrowed):

module shift (C, SI, PO);
input  C,SI;
output [7:0] PO;
reg [7:0] tmp;

always @(posedge C)
begin
tmp = {tmp[6:0], SI};
end
assign PO = tmp;
endmodule


Take the parallel out and compare it to your combination. If you really want to add some state memory, you can add a reset input and some external logic to this, but a shift register seems like the clear starting point for solving this problem. Simpler to debug as well.

Due to comb logic output you have faced issues in error signal.. If you try to model your FSM using standard 3 always or 4-always block you wont observe this issue, of course with this it leads to utilization of Flip-Flops in FPGA which can be ignored. FSM coding style might be helpful to debug and readability.

  module lock (
input  wire     clock,reset,x,
output reg      ready,unlock,error
);

reg [2:0] state,next_state;

localparam S_RESET = 3'd0,
S1      = 3'd1,
S2      = 3'd2,
S3      = 3'd3,
S4      = 3'd4,
S5      = 3'd5,
OPEN    = 3'd6,
S_ERROR = 3'd7;

always @ (posedge clock, posedge reset)
if(reset)  state <= #1 S_RESET;
else        state <= #1 next_state;

always@* begin
next_state = 'bx;
case(state)
S_RESET : next_state =  x ? S1      : S_RESET ;
S1      : next_state = ~x ? S2      : S_ERROR ;
S2      : next_state =  x ? S3      : S_ERROR ;
S3      : next_state = ~x ? S4      : S_ERROR ;
S4      : next_state =  x ? S5      : S_ERROR ;
S5      : next_state =  x ? OPEN    : S_ERROR ;
OPEN    : next_state = ~x ? S_RESET : OPEN    ;
S_ERROR : next_state = ~x ? S_RESET : S_ERROR ;
endcase
end

always @ (posedge clock, posedge reset)
if(reset)  begin
unlock <= #1 1'b0;
ready  <= #1 1'b0;
error  <= #1 1'b0;
end else begin
unlock <= #1 1'b0;
ready  <= #1 1'b0;
error  <= #1 1'b0;
case(next_state)
S_RESET : ready  <= #1 1'b1;
OPEN    : unlock <= #1 1'b1;
S_ERROR : error  <= #1 1'b1;
endcase
end
endmodule


output in vivado simulation