Keypad Scanner Verilog code problem with state machine and column input

Question

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

• Dave Tweed: That's an awful lot of code for what should really be a very simple function. I'm not going to analyze it to try to determine what your specific problem might be. Take a look at this answer for some hints about how to approach this.
• Dave Tweed: BTW, if you want to allow multiple keys to be pressed simultaneously, you'll need to put a diode in series with each key. Otherwise, the results will be ambiguous for certain key combinations.

Answer 1

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.