Keypad Scanner Verilog code problem with state machine and column input

cyclonekeypadmatrixstate-machinesverilog

I am developing a Keypad both in hardware and Verilog using a DE2 Cyclone II board. I made a keypad using buttons (switches) that follows this schematic:
Keypad Design
Final Result

The scanner works by setting the Column inputs all to HIGH (1) in the initial state, and when a keypress has been detected, it scans each column individually and when it detects that one of the rows has a HIGH logic value, it will proceed to decode which key has been pressed. I tested the physical circuit using Logic probes and it is working correctly.

I created a Verilog program that would detect the key pressed, and also account for the debouncing effect of the buttons.
The problem is that when I hook up the wires to the correct GPIO pins of the Expansion header of the DE2 board and test my program, it seems that it does the following behaviour: No matter which key in which column I press, the State Machine starts in initial state S1 (C[2:0] = 111), proceeds to S2 when a key is pressed (C[2:0] = 001), continues to pick up a high signal in K (result of |R[3:0]) and proceeds to S5, the final state where the key is decoded.

For instance, any key pressed generates the following output:

Any Key pressed in Row0: KeyValue=1.

Any Key pressed in Row1: KeyValue=4.

Any Key pressed in Row2: KeyValue=7.

Any Key pressed in Row2: KeyValue=A.

The expected behaviour is that it will continue to scan Column 2 (C[2:0] = 010) or Column 3 (C[2:0] = 100) if necessary since keys in these columns could be pressed. Unfortunately, this does not happen.

I am not sure what the problem is. I went through the code multiple times, and don't see the cause. I feel like it is a timing issue, could someone suggest some way to improve my code (especially the finite state machine in module keyscan)?

    /* KEYPAD MATRIX Scanner, Debouncer, and Decoder. */
    //DE2 Board Expansion pinout: http://www.johnloomis.org/altera/DE2/expansion.html
    //Source:                                                                                                                                                 https://www.allsyllabus.com/aj/note/ECE/Digital_System_Design_Using_VHDL/Unit3/Design%20of%20a%20keypad%20scanner.php#.VQYExI7F-So
    //Similar: http://www.uccs.edu/~gtumbush/4200/lab5.pdf
module keypad (CLOCK_50, GPIO_0, LEDG, HEX0);
    input CLOCK_50; 
    inout [23:11] GPIO_0; // //changing R input values
    output [1:0] LEDG;
    output [0:6] HEX0;

    wire KeyPress; //valid key press detected
    wire [3:0] KeyValue;
    /*
    output KeyPress; //valid key press detected
    output [3:0] KeyValue;
    */

    wire K, Kd; //for debouncer circuit
    wire Q1; //identifies whether current state is S5 (idle).
    wire [2:0] C; //Column output pin (for scanner circuit)

    wire [3:0] R;
    assign R[3] = GPIO_0[17];
    assign R[2] = GPIO_0[15];
    assign R[1] = GPIO_0[13];
    assign R[0] = GPIO_0[11];

    debounce(R, Q1, Kd, K, CLOCK_50);
    keyscan(Kd, K, CLOCK_50, KeyPress, C, Q1);
    decoder(R, C, KeyValue, KeyPress);

    //assign C to output pins...
    assign GPIO_0[19] = C[0];
    assign GPIO_0[21] = C[1];
    assign GPIO_0[23] = C[2];

    assign LEDG[0] = KeyPress;
    seven_seg_decoder(KeyValue, HEX0);

endmodule

/* Decoder is solely a combinational circuit (no flip-flops).
Its output will change as the keypad is scanned. At the time a valid key
is pressed (V=1), its output will have the correct value, and may be saved 
in a register elsewhere. */
module decoder(R, C, N, V);
    input [3:0] R; //active rows in key matrix
    input [2:0] C; //active columns in key matrix
    input V;
    output reg [3:0] N; //key number


    /*FIXED logic: Last two encoded buttons changed FROM [0000, 1011] TO [1011, 1100]*/
    always @(posedge V) begin 
        N[3] = R[3] | (R[2] & ~C[0]);
        N[2] = R[1] | (R[2] & C[0]) | (R[3] & C[2]);
        N[1] = (R[0] & ~C[0]) | (R[3] & ~C[2]) | (R[1] & C[2]) | (R[2] & C[0]);
        N[0] = (C[1] & (R[1] | R[3])) | (~C[1] & (R[0] | R[2]));
    end

    /* OLD LOGIC
    assign N[3] = (R[2] & ~C[0]) | (R[3] & ~C[1]);
    assign N[2] = R[1] | (R[2] & C[0]);
    assign N[1] = (R[0] & ~C[0]) | (~R[2] & C[2]) | (~R[1] & ~R[0] & C[0]);
    assign N[0] = (R[1] & C[1]) | (~R[1] & C[2]) | (~R[3] & ~R[1] & ~C[1]); */
endmodule

//Example: http://www.fpga4fun.com/Debouncer2.html
module debounce(R, Q1, Kd, K, Clk_50);
    input [3:0] R; //activated rows of keypad
    input Q1; //is scanner in S5 state? 1 for true, 0 for false.
    input Clk_50;
    output K; //immediate Key detector (primarily used for scan)
    output Kd; //debounced Key

    wire dClk;
    clock_50M_toX(Clk_50, dClk);

    assign K = |R; //binary OR reductor 
    wire DA;
    wire QA;
    assign DA = (QA & ~Q1) | K;

    D_flipflop(DA, dClk, QA);
    D_flipflop(QA, dClk, Kd);
endmodule



//Key scanner FSM
//Modeled on following: http://www.asic-world.com/verilog/memory_fsm3.html
module keyscan(Kd, K, Clk, V, C, Q1);
    input Kd, K;
    input Clk; //Clock - 50MHz
    output reg [2:0] C;
    output reg V;
    output reg Q1; //top state bin value (X6)
    /* Q1 used in debounce circuit to provide extra security 
    blanket for K input (in case of error or delay with K) */

/**
State assignment (One-Hot):
S1 = 00001  START state
S2 = 00010  
S3 = 00100  
S4 = 01000  
S5 = 10000  IDLE/DEST state
State transition table:
State Bin       Kd  K       NextBin Next
S1      00001       1   X       00010       S2
                    0   X       00001       S1
S2      00010       X   0       00100       S3
                    X   1       10000       S5
S3      00100       X   0       01000       S4
                    X   1       10000       S5
S4      01000       X   0       10000       S5 <-shouldn't ever happen, but JIC
                    X   1       10000       S5
S5      10000       1   X       10000       S5
                    0   X       00001       S1
ERR 00000       X   X       00000       WAIT <- Just for fun
**/
//Mealy + Moore FSM
    reg [4:0] state = 5'b00001;
    reg [4:0] next_state;
    initial begin //SET starting state
        state = 5'b00001;
    end
    /*
    assign state[0] = (state[0] & ~Kd) | (state[4] & ~Kd);
    assign state[1] = state[0] & Kd;
    assign state[2] = state[1] & ~K;
    assign state[3] = state[2] & ~K;
    assign state[4] = (state[1] & K) | (state[2] & K) | state[3] | (state[4] & Kd);
    assign Q1 = state[4]; //X6*/

    always @ (state or Kd or K) begin
        next_state[0] = (state[0] & ~Kd) | (state[4] & ~Kd);
        next_state[1] = state[0] & Kd;
        next_state[2] = state[1] & ~K;
        next_state[3] = state[2] & ~K;
        next_state[4] = (state[1] & K) | (state[2] & K) | state[3] | (state[4] & Kd);
        Q1 = next_state[4];
    end

    always @(posedge Clk) begin
        state <= next_state;
        case (state)
            5'b00001: begin //S1
                V = 1'b0;
                C <= 3'b111;
            end
            5'b00010: begin //S2
                C <= 3'b001;
            end
            5'b00100: begin //S3
                C <= 3'b010;
            end
            5'b01000: begin //S4
                C <= 3'b100;
            end
            5'b10000: begin //S5
                //***while here, don't change C.
                V <= 1'b1;
            end
        endcase
    end

endmodule


/* Manually adjust values here depending on bounce time */
module clock_50M_toX(Clk_50MHz, dClk);
    input Clk_50MHz;
    output dClk;
    reg pulse_XHz = 1; //start off high (w/0 registering "posedge")
    /**
    Formula for calculating bit length of counter.
    Say bounce time = Xms.
    Then num pulses of 50MgHz gone by for entire duration of Xms:
        n = (X/1000) * (50 *10^6)
    We want to simulate the rising and falling edge of the pulse,
    so we mark the halfway pulse point like so:
        n_half = n / 2
    Then the length of the counter to hold the correct pulses
    and toggle the clock is:
        c = ceil(log2(n_half)) 
        => reg [c-1:0] clock_pulses!!
        => if (clock_pulses < ((n_half) - 1)) begin
    **/
    reg [16:0] clock_pulses; //try ~3ms
    always @ (posedge Clk_50MHz) begin
            //Marking the halfway point in order to simulate
            //the rising and falling edge of the 1Hz clock pulse.
            if (clock_pulses < ((75000) - 1)) begin
                clock_pulses <= clock_pulses + 1;
            end else begin
                clock_pulses <= 0;
                pulse_XHz <= ~pulse_XHz;
            end
    end
    assign dClk = pulse_XHz;
endmodule


/* positive edge triggered D_FF without reset */
module D_flipflop(d, clk, q);
    input d, clk;
    output q;
    reg q;
    always @(posedge clk) begin
        q <= d;
    end
endmodule

//hexadecimal convertor: converts any value from 0 to 15
//hexadecimal: a consequetive group of 4 binary digits can be considered independently, and converted directly
module seven_seg_decoder(V, Display);
    input [3:0] V; //input code
    output [0:6] Display; //output 7-seg code

    //When you want to turn on a segment, set it low
    assign Display[0] = (~V[3] & ~V[1] & (V[2] ^ V[0])) | (V[3] & V[0] & (V[2] ^ V[1]));
    assign Display[1] = (~V[3] & V[2] & ~V[1] & V[0]) | (V[3] & V[2] & ~V[0]) | (V[3] & V[1] & V[0]) | (V[2] & V[1] & ~V[0]);
    assign Display[2] = (~V[0] & ((V[3] & V[2] & ~V[1]) | (~V[3] & ~V[2] & V[1]))) | (V[3] & V[2] & V[1]);
    assign Display[3] = (~V[3] & ~V[1] & (V[2] ^ V[0])) | (V[3] & ~V[2] & V[1] & ~V[0]) | (V[2] & V[1] & V[0]);
    assign Display[4] = (~V[3] & V[2] & ~V[1]) | (V[0] & ((~V[2] & ~V[1]) | ~V[3]));
    assign Display[5] = (V[3] & V[2] & ~V[1] & V[0]) | (~V[3] & ~V[2] & (V[1] | V[0])) | (~V[3] & V[1] & V[0]);
    assign Display[6] = (V[3] & V[2] & ~V[1] & ~V[0]) | (~V[3] & V[2] & V[1] & V[0]) | (~V[3] & ~V[2] & ~V[1]);
endmodule

Best Answer

This is more of an extended comment than an answer, because I can't put code examples in a comment.

Verilog gives you a lot of power to express logic in the most appropriate form; it isn't necessary to reduce everything to Boolean logic equations. For example, your 7-segment converter would be more clearly expressed as a case statement:

module seven_seg_decoder (V, Display);
  input [3:0] V; //input code
  output [0:6] Display; //output 7-seg code

  // When you want to turn on a segment, set it low
  always @(*) begin
    case (V)
    4'h0: Display <= 7'b0000001;
    4'h1: Display <= 7'b1001111;
    4'h2: Display <= 7'b0010010;
    // ... etc.
    endcase
  end
endmodule

A case statement is also a good way to express the functionality of a state machine, as well. If you want other engineers to be able to read and understand your code, you should use the highest-level constructs that make sense.

Related Topic