[SOLVED] Why this timer with millis() function fails?

I’m driving a transistor gate with a pin of an Arduino Pro Mini to turn off/on a led powered with +12V.
It works fine.

After, I introduced a configurable “timer” to turn off the led after a predefined number of seconds. In this way I avoid the led to dry the battery.
Furthermore, the max number of seconds that the led can stay ON depends on the value of a “timer” variable stored in a struct called “settings”:

{
....
unsigned char LIGHT_TIMER;  // max is 255 seconds
...
}

The fragment of the code turning off the led is what follows:

// check timer for led light
if (LEDLIGHT_ON && ((millis() - LEDLIGHT_ONTIME) > config.settings.LIGHT_TIMER * 1000))
{
    digitalWrite(LEDLIGHT_POWER,LOW); // light off
    LEDLIGHT_ON = false;
}

Quite curiosly, I have obtained the following results:

config.settings.LIGHT_TIMER == 60 ----> FAIL. The led stays ON even after 60 seconds
config.settings.LIGHT_TIMER == 45 ----> FAIL. The led stays ON even after 45 seconds
config.settings.LIGHT_TIMER == 35 ----> FAIL. The led stays ON even after 35 seconds
config.settings.LIGHT_TIMER == 30 ----> SUCCESS. The led turns OFF after 30 seconds
config.settings.LIGHT_TIMER == 25 ----> SUCCESS. The led turns OFF after 25 seconds
.
.
.

where

LEDLIGHT_ON is declared as a bool,
LEDLIGHT_ONTIME is declared as an unsigned long (it is the time when the led is turned on).

I strongly suspect that the right-expression config.settings.LIGHT_TIMER * 1000 is automatically typed as an int, so values greater than 32767 cannot be represented.
Or what?

(deleted)

The full code is very long and complicated, involving many C++ objects. Posting it would be useless.

My question is restricted to the test condition of the if instruction above only. I would to know if this is the normal behaviour for the compiler.

gimpo:
The full code is very long and complicated, involving many C++ objects. Posting it would be useless.

Then post a short complete program that illustrates the problem.

I would to know if this is the normal behaviour for the compiler.

The compiler has been very extensively tested. Whatever it does can be assumed to be normal behaviour. The chances that you have found a compiler bug are vanishingly small.

...R

gimpo:
I strongly suspect that the right-expression config.settings.LIGHT_TIMER * 1000 is automatically typed as an int, so values greater than 32767 cannot be represented.
Or what?

Yeah, that would be the problem. Define one of the operands to the multiplication as an unsigned long so that the math is done using unsigned long instead of int, eg:

config.settings.LIGHT_TIMER * 1000UL

Thanks for the replies.

@Robin2
I didn’t mean that what above is a bug of the compiler, I have not enough experience about avr-gcc to state that.
I’m just blocked in debugging my program and the code above is the only place where I check for the “timer” status.

@DrAzzy
In this moment I’m writing a super-simplified version of the timer to illustrate the problem. I will try your solution and will post the results here in few (!) minutes.

Here we are again.

Below the super-simplified version that illustrate the problem:

unsigned long LEDLIGHT_ONTIME;
unsigned char LIGHT_TIMER;
bool LEDLIGHT_ON;
int counter_seconds; // a simple counter to roughly represent the time running...


void setup() {
  // initialize serial communication at 9600 bits per second:
  Serial.begin(9600);
  LEDLIGHT_ON = true;
  LIGHT_TIMER = 30;
  Serial.println("LED light turned on..."); 
  counter_seconds = 0;
  LEDLIGHT_ONTIME = millis();
}

// the loop routine runs over and over again forever:
void loop() 
{ 
  // check timer for led light
  if (LEDLIGHT_ON && ((millis() - LEDLIGHT_ONTIME) > LIGHT_TIMER * 1000))
  {
      Serial.println("Turn off the LED, please!");
      LEDLIGHT_ON = false;
  }
  
  delay(1000);
  counter_seconds++;
  Serial.println(counter_seconds);
}

The LIGHT_TIMER is set to 30 seconds and it works as expected :

LED light turned on...
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
Turn off the LED, please!
31
32
33
34
35
36
37
.........

You are not paying attention.

LIGHT_TIMER * 1000

has maximum positive value of 32000.

BUT, if the variable LIGHT_TIMER is set even to just 35 then it loops forever printing seconds. No message is printed. :o

On the other hand, by substituting the instruction

if (LEDLIGHT_ON && ((millis() - LEDLIGHT_ONTIME) > LIGHT_TIMER * 1000))

with

if (LEDLIGHT_ON && ((millis() - LEDLIGHT_ONTIME) > LIGHT_TIMER * 1000UL))

then it works like a charm!
The message "Turn off the LED, please!" is printed after 35 seconds. :slight_smile:

Thanks DrAzzy, you're a genius!

jremington:
You are not paying attention.

LIGHT_TIMER * 1000

has maximum positive value of 32000.

Hi, you're not paying attention. I have supposed that in my first post (check it above), and I was looking for a confirmation about that by who know the compiler better than me... :-[

Anyway, that was the problem, as you pointed. I will mark this post as solved.
I think it will be useful to somebody else, since I've seen only examples/tutorials with few seconds as timer-value.

"unsigned char LIGHT_TIMER;"

Study number 'type'

https://learn.sparkfun.com/tutorials/data-types-in-arduino

BTW, you can add UL to a number tell the compiler to use 'unsigned long' in multiplication.

1000UL

Thanks karryd,

your suggestion was yet proposed (see some posts before), and it is the right one!