# 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      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;
error = 1'b0;
end
else begin
next_state = s_reset;
unlock = 1'b0;
error = 1'b0;
end

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

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

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

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

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

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

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

default : begin
next_state = s_reset;
unlock = 1'b0;
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        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 ),
.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. – Tom Carpenter Apr 1 '16 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 '16 at 20:01
• In the second always block x should be included in the sensitivity list. How do you test your module? – andrsmllr Apr 1 '16 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 '16 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 '16 at 21:03

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;
unlock <= 1'b0;
error  <= 1'b0;
end
else begin
state  <= next_state;
unlock <= next_unlock;
error  <= next_error;
end
end

always @* begin
// default assignment
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;
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.