-1

Currently i faced some problem on my lcd screen. I'm try to do a countdown timer but when i set Hour = 0, Min = 1, the sec hit 0 and my hour turn to some unknown character and min = 59, sec = 59. I'm i missing out something?

void Timer1(void) interrupt 3
{
    TF1 = 0;
    TH1 = 0xB1;
    TL1 = 0XE0;
    cd_msec--;

    if(cd_msec == 0)
    {
        cd_msec = 99;
        cd_sec--;
    }
    if(cd_sec == 0)
    {
        cd_sec = 59;
        cd_min--;
    }
    if(cd_min == 0) 
    {
        cd_min = 59;
        cd_hour--;
    }
    if(cd_hour == 0)
    {
        cd_hour = 0;
    }
    if(cd_hour == 0 && cd_min == 0)
    {
        cd_hour = 0;
        cd_min = 0;
    }
    if(cd_hour == 0 && cd_min == 0 && cd_sec == 0)
    {
        cd_hour = 0;
        cd_min = 0;
        cd_sec = 0;
        cd_msec = 0;
    }     

}
Fufu Alex
  • 37
  • 1
  • 1
  • 9

3 Answers3

1

Your logic is pretty wrong. Assume the pre-condition:

cd_msec=1; cd_sec=1; cd_min=5;

When the code executes, you'll get:

cd_msec=99; cd_sec=59; cd_min=4;

So a single tick changed the countdown more than 1 sec.

Remember that zero is a valid value! I'll suggest that you rewrite the code so that you check for zero before decrementing.

Something like:

if (cd_msec > 0) {
  cd_msec--;
}
else
{
  if (cd_sec > 0) {
    cd_sec--;
    cd_msec = 99; // Assummes 10ms ticks
  }
  else
  {
     // Handle case with both cd_msec and cd_sec being zero
     // ...
     // ....
  }
}
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
1

I agree with @nielsen that the logic is wrong. You may consider following approach to update all the variables properly at every millisecond tick.

Also, I have assigned milli_sec to 999 considering that you will manage to get a 16 bit variable for it.

if (milli_sec > 0)
{
    milli_sec--
} else {
    if (second > 0) {
        milli_sec = 999;
        second--;
    } else {
        if (minute > 0) {
            milli_sec = 999;
            second = 59
            minute--
        } else {
            if (hour > 0) {
                milli_sec = 999;
                second = 59;
                minute = 59;
                hour--
            }
            else {
                //COUNTDOWN REACHED TO 0
                //hour,minute,second,milli_second is zero
            }

        }
    }
}
Sunil Shahu
  • 946
  • 1
  • 9
  • 24
  • @Sunil, @FuFu Alex: Notice that the example here is a little bit wrong. The last `else` is wrong. That point will be reached 1 tick too late! The check for "COUNTDOWN REACHED" must be _after_ the decrement has been done, i.e. after the big if-statement. – Support Ukraine Feb 06 '15 at 10:44
  • @nielsen, Thanks for pointing that out. So, on every tick it will be like 3->2->1->0->"COUNTDOWN REACHED TO 0". In fact there should be a check outside and after topmost if..else for "COUNTDOWN REACHED TO 0". for example if (milli_sec == 0 && second == 0 && minute == 0 && hour == 0 {//COUNTDOWN REACHED TO 0} – Sunil Shahu Feb 06 '15 at 11:05
  • Thanks both of you for the explanation. Very appreciated . – Fufu Alex Feb 06 '15 at 12:24
0

You have 0H : 1M : 0S.

You check seconds, seconds is zero. You set minutes = 0. You check minutes, it is now 0. So you subtract 1 from hours. Hours is already zero. So it wraps around to maybe ~65k.

IMHO it would be better to have only msecs and convert to hours:minutes:seconds only when you update the display (if you need).

RedX
  • 14,749
  • 1
  • 53
  • 76