Verilog code involving always blocks

Question

I am trying to write a verilog code to do the following 3 tasks:

If the right button is pressed once, the led will shift to the right continuously
If the left button is pressed once, the led will shift to the left continuously
At any time I want to change the direction of the led(for example from right to left), I only need to press the left button once

My code is as follows:

module moveled(input clk, input buttonright, input buttonleft, output [15:0]led);

reg [15:0]led=16'b0000100000000000;  
wire slowclk;
wire r;
wire qr;
wire l;
wire ql;


slowclock clk(clk, slowclk);

pulse spR(clk,buttonright,r);
pulse spL(clk,buttonleft,l);
jkff jkR(r,r,clk,qr);
jkff jkL(l,l,clk,ql);

always @(posedge slowclk)
begin 

  if(qr==1 && ql==0)
  begin
    led<=led>>1;
  end

  else if(qr==0 && ql==1)
  begin
    led<=led<<1;
  end

  else 
    led<=led;
end    

endmodule

The slowclock module is making the led moving to the right/left slower so that it is more observable. The pulse module is there to remove debouncing. The 2 jkff module are t flip flops to toggle qr and ql for the led to move to the right/left continuously. The main problem in my code is in the always block, the code fails when qr=1 and ql=1. For example, if qr=1, my led is moving right and I press the left button so that qr=1 and ql=1, the led stops. When I try to press the left button again to move led to the left, the led will instead move to the right because ql=0 and qr=1. The sensitivity list in if() must be qr==1 && ql==0 or else the led will move to the right non stop.

As such, this code fails task 3 as stated above. Any help would be greatly appreciated.

How do you want it to switch from stopped (11) to move to a 01 state ie de-asssert ql and assert qr on a qr press. At present they do not effect each other. — Morgan
If possible, I want to avoid a (11) state, it should be a (01) state when it moves to the right and (10) state when it moves to the left — David

Morgan Morgan · Accepted Answer · 2016-03-07T12:59:15

Assuming that you pulse turns a level of a button press into a single 1 clock wide pulse (edge detection) then you could just have:

NB: need to add a reset to set state to S_IDLE either through initial or a active low reset.

localparam S_IDLE  = 2'b00;
localparam S_RIGHT = 2'b10;
localparam S_LEFT  = 2'b01;
localparam BOTH  = 2'b11;

reg  [1:0] state;
wire [1:0] next_state = {r,l};//bypass toggle flops

always @(posedge slowclk) begin 
  case(next_state) 
    S_IDLE  : state <= S_IDLE ;
    S_RIGHT : state <= S_RIGHT;
    S_LEFT  : state <= S_LEFT ;
    S_BOTH  : state <= S_IDLE ;
  endcase
end

//Outputs
always @(posedge slowclk) begin 
  case(state) 
    S_IDLE   : led <= led;
    S_RIGHT  : led <= led>>1;
    S_LEFT   : led <= led<<1;
    default  : led <= led;
  endcase
end

Verilog code involving always blocks

1 Answers