Continue to Site

Welcome to EDAboard.com

Welcome to our site! EDAboard.com is an international Electronics Discussion Forum focused on EDA software, circuits, schematics, books, theory, papers, asic, pld, 8051, DSP, Network, RF, Analog Design, PCB, Service Manuals... and a whole lot more! To participate you need to register. Registration is free. Click here to register now.

Register Log in

[SOLVED] Verilog alarm clock time advancing not working correctly ..

Status
Not open for further replies.
T

techy5025

Newbie level 4
Joined
Mar 3, 2020
Messages
7
Helped
0
Reputation
0
Reaction score
0
Trophy points
1
Location
Orlando, Fla
Activity points
62
Hello. I am new to verilog and to this forum, and have been trying to get an alarm clock working on the BASYS 3 FPGA board using Vivado 2019. All the code is working correctly except for the part that advances the time. The problem is that both the seconds and minutes advance together .. so essentially the seconds display as minutes and the minutes as hours. I have included a code snippet below.

The input clock is at a .1 sec rate. The first part of the code is used to set the min and sec and works correctly. The second part causes the decimal point of the 7-segment display and an LED to blink at a one second rate and also works correctly. The third part steps the second and minutes together. I have tried using blocking and non-blocking code (as here) with the same results. The problem has been verified both on the Vivado simulator and on the BASYS board.

I'll be happy to be embarrassed by what is obviously a newly mistake!:)

Thanks for the help. Jim

Code: 
 /************* Clock operation**************/

always @(posedge first_toggle)  //toggle every .1 sec
 
begin 
       if (reset)
               begin                // reset counters
                 tmp_chour = 4'b0000;
                 tmp_cmin  = 4'b0000;
                 tmp_csec  = 4'b0000;
               end 
            
//set alarm/clock min

        if (set_min)        // set  minutes
             begin
                 if ((min_del == 0) || (min_del >= 15 ))  // first toggle on key press or 1.5 sec delay
                       begin
                          if (set_alarm)              // Set alarm ?
                              if(tmp_amin >= 59)      // Min >= 59 ?
                                  tmp_amin = 0;       // Yes - Zero alarm min
                              else                          
                                  tmp_amin = tmp_amin + 1; //No - bump alarm min
                          else if(tmp_cmin >= 59)     // Min >= 59 ?
                                  tmp_cmin = 0;       // Yes - Zero clock min
                               else                          
                                  tmp_cmin = tmp_cmin + 1; //No - bump clock min
                          if (min_del <= 14)          // 1.5 sec delay for fast speed
                              min_del <= min_del +1;  // Bump delay tenths of a sec        
                       end
                 else if (min_del <= 14)              // 1.5 sec delay for fast speed
                          min_del <= min_del +1;      // Bump delay tenths of a sec
              end
         else
              min_del = 0; // Clear min delay             
           
//set alarm/clock hours

        if (set_hour)        // set  hour
             begin
                 if ((hour_del == 0) || (hour_del >= 15 )) // first toggle on key press or 1.5 sec delay
                       begin
                          if (set_alarm && mode == 1)  // Set alarm and 24 hour ?
                              if(tmp_ahour >= 23)      // 24 hour mode?
                                  tmp_ahour = 0;       // Yes - Zero ahour
                              else                          
                                  tmp_ahour = tmp_ahour + 1; // No - bump ahour
                          else if (set_alarm && mode == 0)   // Set alarm and 12 hour ?
                              if(tmp_ahour >= 12)      // 12 hour mode?                       
                                  tmp_ahour = 0;       // Yes - Zero ahour
                              else
                                  tmp_ahour = tmp_ahour + 1; // No - bump ahour
                          else if (!set_alarm && mode == 1)  // Set clock and 24 hour ?
                              if(tmp_chour >= 23)      // 24 hour mode?
                                  tmp_chour = 0;       // Yes - Zero clock hour
                              else                          
                                  tmp_chour = tmp_chour + 1; // No - bump clock hour
                          else if (!set_alarm && mode == 0)  // Set clock and 12 hour ?
                              if(tmp_chour >= 12)      // 12 hour mode?                       
                                  tmp_chour = 0;       // Yes - Zero clock hour
                              else
                                  tmp_chour = tmp_chour + 1; // No - bump clock hour
                          if (hour_del <= 14)          // Fast delay up?
                          hour_del <= hour_del +1;     // No - Bump fast delay   
                       end
                 else if (hour_del <= 14)              // 1.5 sec delay for fast speed
                          hour_del <= hour_del +1;     // Bump delay tenths of a sec
              end
         else
              hour_del = 0; // Clear hour delay       
 
//Normal clock operation

// This code is entered once every tenth second

// Turn decimal point and LED #1 on for approx .5 sec

        if (tenth_counter >= 5)
             begin
                 dp_tmp <= 1;    // Turn decimal point off
                 LED16 <= 0;     // Turn LED off
             end
         else
             begin
                 dp_tmp <= 0;    // Turn decimal point on
                 LED16 <= 1;     // Turn LED on
             end

// Advance count on ninth of ten "tenth counts"
// tmp_csec, cmin, chour > clock second, minuter, hours

         if (tenth_counter >= 9)                  //advance time if tenth count is 9
               begin
                 tenth_counter <= 0;              // Reset tenth counter
                 
                 if(tmp_csec >= 59)               // second > 59 then minute increases
                     begin 
                         tmp_csec <= 0;            // clear second count
                         tmp_cmin <= tmp_cmin + 1; // bump the minute count
                     end
                 else 
                     begin    
                         tmp_csec <= tmp_csec + 1; //bump the second count  
                     end
                 if(tmp_cmin >= 59)                // minute > 59 then hour increases
                     begin 
                         tmp_cmin <= 0;            // clear the minute count
                         tmp_chour <= tmp_chour + 1;// bump the hour count
                     end
                  else
                     begin
                         tmp_cmin <= tmp_cmin + 1;  // bump the minute count
                     end
                         
                 if(mode == 1 && tmp_chour >= 24)   // 24 hour mode = 1 12 hour = 0
                         tmp_chour <= 0;            // 24 hours 0-23
                 else  if (mode == 0 && tmp_chour >= 13)
                         tmp_chour <= 1;            // 12 hour mode 1-12
               end
          else
               tenth_counter <= tenth_counter + 1;  // Bump tenth counter
  end
 

Hi,

I expect:
At the same time when
* tenth_counter is reset to zero
* you also need to increment the seconds counter.
Allways.

But in your code both lines are far apart and one is behind an additional "IF".

Could you show us your flow chart? If you didn't draw already, then do it now.
Did you run a simulation of your code? Or at least take a piece of paper and a pencil to do the same by hand.

Klaus
 

Don't mix blocking and non-blocking assignments in a edge triggered always block. Only use non-blocking assignments in clocked logic and blocking in combonational always blocks.

Using blocking assignments with code like this:
Code: 
                              if(tmp_chour >= 23)      // 24 hour mode?
                                  tmp_chour = 0;       // Yes - Zero clock hour
                              else                          
                                  tmp_chour = tmp_chour + 1; // No - bump clock hour
causes synthesis to generate a FF -> add_1 -> compare code increasing the combinational logic path between flip-flops. Occasionally you might want to take advantage of this, but with a Verilog newbie you want to avoid anything like this.

The structure of this code seems somewhat convoluted for the task.

Seems to me you aren't generating a 10th sec counter enable to run the sec/min/hr counters. Try looking up pipelined counters and structure the code like those types of counters, the only difference between a clock counter and a binary pipelined counter is the rollover conditions of each stage.

Typical pipelined counter (for a clock) with enable would be:

1. 1/10 sec counter, rolls over every 1 sec.
2. enable code to generate a pulse every 1 sec (can use the 1/10 sec counter rollover condition)
3. sec counter, counts on every 1 sec enable, rolls over on a count of 59
4. min counter, counts on the sec counter equal to 59 on a 1 sec enable
5. hr counter, counts on the min counter equal to 59 on a 1 sec enable
 

Thank you for the reply! I just noticed that I got a Synthesis warning message ..

"[Synth 8-6014] Unused sequential element tmp_csec_reg was removed. ["C:/Verilog/AlarmClock/AlarmClock.srcs/sources_1/imports/Alarm Clock/AlarmClockMod.v":98]"
Click to expand...

This refers to the line marked with <<< below. Maybe this has something to do with my problem? This is how the reg is identified.

code snip...

Code: 
 reg [24:0] first_counter;          // .1 sec counter for rapid time set
 reg [3:0] tenth_counter;           // 1 sec counter
 reg [5:0] tmp_chour, tmp_cmin, tmp_csec;                 <<<<<<<<<<<<<<<<
 reg [5:0] tmp_ahour, tmp_amin, tmp_asec;



Code: 
/************* Clock operation**************/

always @(posedge first_toggle)  //toggle every .1 sec
 
begin 
       if (reset)
               begin                // reset counters
                 tmp_chour <= 4'b0000;
                 tmp_cmin  <= 4'b0000;
                 tmp_csec  <= 4'b0000;                 <<<<<<<<< line 98
               end 
            
//set alarm/clock min

        if (set_min)        // set  minutes
             begin
                 if ((min_del == 0) || (min_del >= 15 ))  // first toggle on key press or 1.5 sec delay

Thanks. Jim
 

ads-ee Thank you! I have changed all the code to non_blocking. The tenth counter is a 4 bit counter initialized to 0.
All this code is executes every .1 sec.

Code: 
    first_counter = 0;
    tenth_counter = 0;
    first_toggle = 0;

.....and is tested for a "nine" count before the time code and incremented after the time code.

Code: 
         if (tenth_counter >= 9)                  //advance time if tenth count is 9  <<<<<<<<<
               begin
                 tenth_counter <= 0;              // Reset tenth counter                           <<<<<<<<<<
                 
                 if(tmp_csec >= 59)               // second > 59 then minute increases
                     begin 
                         tmp_csec <= 0;            // clear second count
                         tmp_cmin <= tmp_cmin + 1; // bump the minute count
                     end
                 else 
                     begin    
                         tmp_csec <= tmp_csec + 1; //bump the second count  
                     end
                 if(tmp_cmin >= 59)                // minute > 59 then hour increases
                     begin 
                         tmp_cmin <= 0;            // clear the minute count
                         tmp_chour <= tmp_chour + 1;// bump the hour count
                     end
                  else
                     begin
                         tmp_cmin <= tmp_cmin + 1;  // bump the minute count
                     end
                         
                 if(mode == 1 && tmp_chour >= 24)   // 24 hour mode = 1 12 hour = 0
                         tmp_chour <= 0;            // 24 hours 0-23
                 else  if (mode == 0 && tmp_chour >= 13)
                         tmp_chour <= 1;            // 12 hour mode 1-12
               end
          else
               tenth_counter <= tenth_counter + 1;  // Bump tenth counter          <<<<<<<<<<<<

Thanks. Jim

- - - Updated - - -

Well, I seem to have found the problem.

Code: 
        if (tenth_counter >= 9)                  //advance time if tenth count is 9
               begin
                 tenth_counter <= 0;              // Reset tenth counter
                 
                 if(tmp_csec >= 59)               // second > 59 then minute increases
                     begin 
                         tmp_csec <= 0;            // clear second count
                         tmp_cmin <= tmp_cmin + 1; // bump the minute count    <<<<<<<<<<<<<<<<<<<
                     end
                 else 
                     begin    
                         tmp_csec <= tmp_csec + 1; //bump the second count  
                     end
                 if(tmp_cmin >= 59)                // minute > 59 then hour increases
                     begin 
                         tmp_cmin <= 0;            // clear the minute count
                         tmp_chour <= tmp_chour + 1;// bump the hour count
                     end
                  else
                     begin
                         tmp_cmin <= tmp_cmin + 1;  // bump the minute count           <<<<<<<<<<<<<<<<<<
                     end

The Synthesis was not happy with two "tmp_cmin <= tmp_cmin + 1" statements ... which also caused the min to advance incorrectly. Why it caused the "[Synth 8-6014] Unused sequential element tmp_csec_reg was removed. ["C:/Verilog/AlarmClock/AlarmClock.srcs/sources_1/imports/Alarm Clock/AlarmClockMod.v":98]" error of the prior post, I don't know.

Anyway, I have revised this part of the code to fix a glitch .. "min unit digit reseting to zero before the tens digit" .. to this...

Code: 
// Advance count on ninth of ten "tenth counts"
// tmp_csec, cmin, chour > clock second, minuter, hours

         if (tenth_counter >= 9)                         // advance time if tenth count is 9
               begin
                 tenth_counter <= 0;                     // Reset tenth counter
                 
                  if(tmp_csec >= 59)                     // second > 59 then minute increases
                     begin
                        tmp_csec <= 0;                   // clear second count
                        if(tmp_cmin >= 59)
                            begin                        // minute > 59 then hour increases
                               tmp_cmin <= 0;            // clear the minute count
                               if(mode == 1 && tmp_chour >= 23)   // 24 hour mode = 1 hours 0-23
                                   tmp_chour <= 0;       // set the hour count to 0
                               else
                                   tmp_chour <= tmp_chour +1;// bump the hour count 
                               if (mode == 0 && tmp_chour >= 12) // 12 hour mode = 0 hours 1-12
                                   tmp_chour <= 1;       // set the hour count to 1
                               else
                                   tmp_chour <= tmp_chour +1;// bump the hour count 
                            end
                        else                            
                            tmp_cmin <= tmp_cmin + 1; // bump the minute count
                     end
                  else
                        tmp_csec <= tmp_csec + 1;     // bump the second count    
              
             end
          else
               tenth_counter <= tenth_counter + 1;    // Bump tenth counter
  end

....and now all works correctly.

Thanks guys for your time!

Jim
 

Your code is difficult to read with all the extra indents and there are too many nested if's. IMO coding in HDLs you should limit both the nesting and the indenting along with not combining a bunch of different signals into a single always block. e.g.

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90

`timescale 1ns/1ps
 
module pipe_cntr (
  input clk,
  input rst,
  output [8:0] cntr
);
 
  reg  [2:0]  cnt_hi;
  reg  [2:0]  cnt_mi;
  reg  [2:0]  cnt_lo;
  wire        tc_lo;
  wire        tc_mi;
 
 
  assign cntr = {cnt_hi, cnt_mi, cnt_lo};
 
  // terminal count of the lower two stages
  assign tc_lo = cnt_lo == 3'h 7;
  assign tc_mi = cnt_mi == 3'h 7;
 
 
  // lower counter, increments continuously
  always @(posedge clk, posedge rst) begin
    if (rst) begin
      cnt_lo <= 0;
    end else if (cnt_lo < 3'h 7) begin
      cnt_lo <= cnt_lo + 1;
    end else begin
      cnt_lo <= 0;
    end
  end
 
  // middle counter only counts if we are at the terminal count of the
  // lower counter
  always @(posedge clk, posedge rst) begin
    if (rst) begin
      cnt_mi <= 0;
    end else if (tc_lo) begin
      if (cnt_mi < 3'h 7) begin
        cnt_mi <= cnt_mi + 1;
      end else begin
        cnt_mi <= 0;
      end
    end
  end
 
  // high counter only counts if we are at the terminal count of both
  // the lower and middle counter
  always @(posedge clk, posedge rst) begin
    if (rst) begin
      cnt_hi <= 0;
    end else if (tc_lo && tc_mi) begin
      if (cnt_hi < 3'h 7) begin
        cnt_hi <= cnt_hi + 1;
      end else begin
        cnt_hi <= 0;
      end
    end
  end
 
 
endmodule
 
 
module tb;
 
  reg clk, rst;
  wire [8:0] cntr;
 
  initial clk = 0;
  always #10 clk = ~clk;
 
  pipe_cntr  uut (
    .clk    (clk),
    .rst    (rst),
    .cntr   (cntr)
  );
 
 
  initial begin
    rst = 1;
    #100;
    rst = 0;
    #10000;
    $stop;
  end
 
 
endmodule


Is an example of how I would write a pipelined counter with different counters for each stage (3 stages), being broken up into each stage makes it easy to see how the counters interact.

You can run the simulation and see how the counters work with the terminal counts letting the next stage know when to count.
 
Last edited:
ads-ee. Thank you much! I will check out the pipelined counter technique. Phew..a lot to learn!

Jim
 

Status
Not open for further replies.

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Back
Top