# Digital clock by atmega8 with timer1 and LCD display

I have written code to make a digital clock using timer1 and tested with Proteus (an Arduino simulator). That program has been downloaded into an Atmega8 and interfaced to an LCD display. Seconds are being counted perfectly in 1 second duration. But when second, minutes and hours are incremented to greater than 60 I see problems. To limit those value, I have written these instruction:

if (second==60)
{
second=0;
}


But when the seconds reach 59, the LCD shows the next second's value randomly. For example: 59,09,19,29,39,49,59.....99,10,11,12,13,14.

My code:

unsigned int second=0;

unsigned int minute=0;

void init_timer1(void);

int main(void)
{
init_timer1();
sei();
Init_LCD();

while(1)
{
goto_position_X_Y(1,0);    //first line first position
sprintf(lcd,"second=%d",second);
Send_A_String(lcd);

goto_position_X_Y(2,0);   // second line first position
sprintf(lcd,"minute=%d",minute);
Send_A_String(lcd);
}
}

void init_timer1(void)
{
TCCR1B |=(1<< CS12);      // prescaler set 256;new freq=31250;
TCCR1B |=(1<< WGM12);
TCNT1 =0;
OCR1A= 3107; // 16 bit max count value 65535,
TIMSK |=(1<< OCIE1A) ;
}

ISR(TIMER1_COMPA_vect)
{
second++;

if(second==60)
{
second=0;
}

minute=minute+(second/60);

if(minute==60)
{
minute=0;
}
}


can you suggest me that can problem be occurred in lcd's program? Code:

Send_A_String(lcd); function of this instruction.

void  Send_A_String(unsigned char*String_data)
{
while (*String _data>0) {
send_A_data(*String_data++) ;
}
}

• please add more structure to your question. Add an empty line where you make a "mental" paragraph. Like this, it's very hard to read your question, and that makes it unlikely to get good answers. Also, the last sentence "Please...thank you" is superfluous (we're all here to help), distracts from the question and it's therefore generally discouraged to have such sentences. – Marcus Müller Aug 17 '17 at 12:19
• If you don't show us your code, how can we tell you what's wrong with it? – Dave Tweed Aug 17 '17 at 12:19
• Are you using timer1 interrupts? Or are you polling timer1? And do edit your question as suggested. Remember, the goal is for you to make a question that other people can find when faced with a similar problem. – st2000 Aug 17 '17 at 12:23
• I have cleaned up your question. As this appears to be an Arduino question, it may get migrated to the arduino.stackexchange.com forum. – st2000 Aug 17 '17 at 12:35
• minute=minute+(second/60); when your seconds will be 60 you reset the counter, thus minutes will always be zero as second can be max 59 when you divide it. You should put this line minute=minute+(second/60); before this if statement if(second==60). – Bence Kaulics Aug 17 '17 at 13:48

the LCD shows the next second's value randomly

Actually, it isn't shown "randomly", as you explained yourself later. Noticing that there is a pattern is important to solving this problem. Always look for patterns in your data - they are telling you something.

Based on your description, this is nothing to do with the values held in the actual variable second but it is, as you suggested later, purely a display problem. Although you've mentioned the displayed characters in a few places, I'll quote this recent comment of yours:

i have noticed that after 59 display shows 09,19,29,39,.....89,99,10,11.....the first digit is accurate but second digit appears 9 instead of delete.

Well done! You found a pattern in the "faulty" displayed data, and that pattern contains the clue to solve this problem.

This is a classic display bug, when:

• you display some characters in a fixed position e.g. on a character LCD;
• but you don't always send the same number of characters to the display.

The result is that, depending on the exact behaviour of the code being used to format the data to send to the display, there can still be some previous output characters left visible on the display. In short - one character can't replace two characters on the display.

At the start of each minute, your output data for the variable second is: 59, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 etc.

Therefore for the first 10 seconds of each minute, you are sending only a single character to that fixed location on your LCD display (you describe it in the code as "first line first position"). That single character is being written to the location where the previous "tens" unit was displayed i.e. where the 5 of 59 used to be.

That leaves the digit 9 of 59 still visible, until it is overwritten by the first output data (10) which has 2 digits!

See this table showing the characters which you send to the LCD display, what you want (intend) it to display, and what it actually displays (which matches your list):

Data sent to display    Intended display    Actual display    Correct display
59                      59                  59                Yes
0                       00                  09                No
1                       01                  19                No
2                       02                  29                No
3                       03                  39                No
...                     ...                 ...
8                       08                  89                No
9                       09                  99                No
10                      10                  10                Yes
11                      11                  11                Yes
...                     ...                 ...


I see two places where you could fix this - either:

• In your Send_A_String() function, test whether the input string is only a single character and add a leading 0 when that occurs. However, I would not do that, as it changes that routine from a general "display what I receive" function, into a function with specific behaviour for this program, which reduces its reusability for the future.

Or

• If your library sprintf() function includes the necessary functionality for "format strings" (some "minimal" sprintf() versions do not include this) then use:

sprintf(lcd,"second=%02d",second);

sprintf(lcd,"second=%d",second);

That will cause the padding to be with the character 0 instead of spaces (sprintf "flags") and will cause the output to be a minimum of 2 characters (sprintf "width") - hence the 02 in the sprintf() format string.

• Hi SamGibson, You are great .I changed sprintf(lcd,"second=%d",second); to sprintf(lcd,"second=%02d",second);.the display is showing my desired output.now i understood that where the fault was occurred .I'll always be grateful to you.thank you. – Muzahid Karim Aug 19 '17 at 0:27

You reset the second counter before making the division with 60 to count the minutes. So you always add 0.

if(second==60)
{
second=0;
}

minute=minute+(second/60);  // <---- This line always adds 0 because you reset the seconds before.


Replace the minute increment before the if statement like this:

minute=minute+(second/60);  // <---- Increment minutes if necessary, then reset the seconds

if(second==60)
{
second=0;
}


Also declare your counters as volatile to make sure that they won't be optimized.

volatile unsigned int second=0;
volatile unsigned int minute=0;


In general, you'll want to avoid doing anything "complex" in an ISR. The problem is that complexity can be hidden in rather subtle ways.

In this case, you are doing some math in your ISR, specifically, calculating second/60, which almost certainly involves a subroutine call into a runtime library. This library function may or may not be "interrupt safe" (the technical term is "reentrant"). If it isn't, this would explain your "random" results.

It would make much more sense to code your ISR more like this:

ISR(TIMER1_COMPA_vect)
{
second++;
if (second==60) {
second=0;
minute++;
if (minute==60) {
minute=0;
}
}
}


This avoids all arithmetic other than increment and compare, and minimizes the number of instructions executed in the most common case (second != 60).

• "calculating second/60, which almost certainly involves a subroutine call into a runtime library" Isn't that valid only if the variable "seconds" is a floating point type? – m.Alin Aug 17 '17 at 14:52
• @m.Alin: No, integer division frequently requires a library call, too. AVR has no hardware divide instruction. – Dave Tweed Aug 17 '17 at 14:53
• Oh, yeah, you're right. It depends if the micro has an hardware integer division instruction or not. – m.Alin Aug 17 '17 at 14:59
• Hi Dave Tweed,. The main problem isn't to increment the minutes .I have faced the main problem in counting second after 59th second.After 59 second the display shows second randomly.for example 59,09,19,29......99,10,11,12,13,.... – Muzahid Karim Aug 17 '17 at 17:09
• Does it still happen with the code that I suggested? If it does, then the problem lies elsewhere. – Dave Tweed Aug 17 '17 at 20:25

I would change your "seconds reset" code to

if(second >= 60)
{
second=0;
minute++;
}


and delete the "minute=minute+(second/60);" line - you want to increment minute every time you reset second to zero.

• Hi Peter Bennett,. The main problem isn't to increment the minutes .I have faced the main problem in counting second after 59th second.After 59 second the display shows second randomly.for example 59,09,19,29......99,10,11,12,13,.... – Muzahid Karim Aug 17 '17 at 17:07