0
\$\begingroup\$

The goal of this project to design a computational lock alarm in Verilog to be run on an Altera board (note on the board 0 is actually ON and 1 is OFF).

Based on the design criteria I created a state diagram of all possible states and contained them into a "cases" statement in my verilog code.

However, I'm having trouble comparing the inputted combination code and the registered password and ALSO rewriting a new password to the registered password when the design is in a particular state.

Note that I've included many of my troubles in comment blocks/lines.

/* NOTES: FIX THE PASSWORD REGISTER AND INPUTS*/

module combinationalLock (clock, INPUTS, enter, change, reset, door, newPass, alarm);

    input enter, change, reset;
    input [4:1] INPUTS; // I don't know if I've defined this correctly

    output door, newPass, alarm;

    reg[3:1] y, Y;

    wire[4:1] passwordREG; // rewritable and comparable to INPUTS (I don't know if I've defined this correctly)
    parameter[3:1]

    // Binary state values

    START = 3'b000, 
    AGAIN = 3'b001, 
    ALARM = 3'b010, 
    CHANGE_OLD = 3'b011, 
    OPEN_DOOR = 3'b100, 
    CHANGE_OLD_AGAIN = 3'b101, 
    CHANGE_NEW = 3'b110;

    // Default password combo

   passwordREG = 4'b0110; // IM TRYING TO SET THE DEFAULT PASSWORD HERE

    // State conditions and changes

    always @(INPUTS, y, reset, change, enter)

        case(y)

            START: if((enter == 0) && (/*INPUTS ARE EQUAL TO REGISTRED PASSWORD*/)) Y = OPEN_DOOR;
                     else if ((enter == 0) && (/*INPUTS ARE NOT EQUAL TO REGISTRED PASSWORD*/)) Y = AGAIN;
                     else Y = START;

            AGAIN: if((enter == 0) && (/*INPUTS ARE EQUAL TO REGISTRED PASSWORD*/)) Y = OPEN_DOOR;
                     else if ((enter == 0) && (INPUTS != passwordREG)) Y = ALARM;
                     else Y = AGAIN;

            ALARM: if(reset == 0) Y = CHANGE;
                     else Y = ALARM;

            CHANGE_OLD: passwordREG = 4'b0110;
                            if((change == 0) && (/*INPUTS ARE EQUAL TO REGISTRED PASSWORD*/)) Y = CHANGE_NEW;
                          else if ((change == 0) && (/*INPUTS ARE NOT EQUAL TO REGISTRED PASSWORD*/)) Y = CHANGE_OLD_AGAIN;
                          else Y = CHANGE_OLD;

            OPEN_DOOR: if(enter == 0) Y = START;
                         else Y = OPEN_DOOR;

            CHANGE_OLD_AGAIN: if((change == 0) && (/*INPUTS ARE EQUAL TO REGISTRED PASSWORD*/)) Y = CHANGE_NEW;
                                else if ((change == 0) && (/*INPUTS ARE NOT EQUAL TO REGISTRED PASSWORD*/)) Y = ALARM;
                                else Y = CHANGE_OLD_AGAIN;

            CHANGE_NEW: passwordREG = INPUTS; // NOTE IN THIS LINE IM TRYING TO REWRITE THE REGISTERED PASSWORD
                            if((CHANGE == 0)||(ENTER == 0)) Y = START; 
                          else Y = CHANGE_NEW;

            default: Y = 3'bxxx;

        endcase

    // Clock pulse

    always @(posedge clock)
        y<=Y;

    // Output

    assign door = (y == OPEN_DOOR);
    assign newPass = (y == CHANGE_NEW);
    assign alarm = (y == ALARM);


endmodule 

Note: I'm also getting some odd syntax errors at line 27, 47, and 59 where they expect an "end case".

\$\endgroup\$
1
\$\begingroup\$

A few points to make.

Firstly for Verilog you should be using zero-indexing. e.g. wire [3:0] imAFourBitWire.


Second, if the body of a case statement takes up more than one line, you need to wrap it in begin-end statements (in fact liberal use of this is a good habit to get into). E.g.

case(y)
   SOMETHING: begin
      aThing = bThing;
      cThing = anotherThing;
   end
   ...
endcase

Notice how multi-line is wrapped in begin-end.

The same goes for if and if-else statements. Yes you can omit the begin-end, but it doesn't mean you should, especially if you are starting out with the language. It doesn't take long to add 8 characters, but it makes the code structure far easier to follow.


Thirdly, this line:

passwordREG = 4'b0110; // IM TRYING TO SET THE DEFAULT PASSWORD HERE

You need to use an assign statement if you want to assign a value to a wire.


This then leads on to:

passwordREG = INPUTS; // NOTE IN THIS LINE IM TRYING TO REWRITE THE REGISTERED PASSWORD

But you declared passwordREG as a wire, not a reg. You are violating two rules here:

  1. You cannot assign a value to a wire with a procedural assignment (e.g. = in an always block), only using continuous assignment (assign statements)
  2. You can only assign a wire with more than one assignment. You've already got passwordREG = 4'b0110; further up.

Next, using parameter[3:1] is no different from parameter. Setting the width of a parameter like this has no effect. Instead the width is the width of whatever value you are setting - e.g. parameter something=3'b110 would create a 3-bit parameter.


Finally, again from a stylistic standpoint, while you can declare multiple parameters like you've done, i.e.

parameter[3:1]

// Binary state values

START = 3'b000, 
AGAIN = 3'b001, 
ALARM = 3'b010, 
CHANGE_OLD = 3'b011, 
OPEN_DOOR = 3'b100, 
CHANGE_OLD_AGAIN = 3'b101, 
CHANGE_NEW = 3'b110;

I would advise not doing so, it makes the code harder to follow and mistakes can start to creep in. Instead use:

// Binary state values

parameter START = 3'b000;
parameter AGAIN = 3'b001; 
parameter ALARM = 3'b010; 
parameter CHANGE_OLD = 3'b011;
parameter OPEN_DOOR = 3'b100; 
parameter CHANGE_OLD_AGAIN = 3'b101;
parameter CHANGE_NEW = 3'b110;

It doesn't add much more in the way of effort, but it will some day save you a load of time debugging the code.

\$\endgroup\$

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.