3

I thought the code below would toggle an LED every 500 milliseconds. But it does not work. I'm not able to catch the mistake, which must be very silly.

void setup() {
  pinMode(3, OUTPUT);
}

void loop() {
  unsigned long cnt=millis();
  if(!(cnt % 500)) {
    PORTD^= (1<<PD3);
  }
}

Anyhow the code below works. But why not the previous code?

void loop() {
  static unsigned long prev=0;
  // Note: First time case is not handled. As soon as we want to toggle,
  // we must store millis() in prev. In this case this happens after reset and may not be an issue.
  unsigned long current = millis();
  if((unsigned long)(current - prev) >= 500) {
    PORTD^= (1<<PD3);
    prev = current;
  }
}
  • A further problem with the first example, not yet mentioned, is that the if() logic is reversed. if( !(cnt % 500) ) is equivalent to if( (cnt % 500) != 0 ), which is true for 499 values and false for only one. The "!" creates the opposite effect to what was wanted. – JRobert Sep 30 '18 at 11:41
18

if (!(millis() % 500)) ...

There are two issues with this. The first, and most obvious, is that the condition will be true for a full millisecond. During that millisecond you will be toggling the LED on and off very fast. Whether you end up doing an odd or even number of toggles is anyone's guess.

The second and less obvious problem is that millis() skips some values. Roughly one integer every 42 is skipped over. This is because the millis() counter is incremented once every 1024 µs, getting 24 µs late on each increment. Once all those delays add up to a full millisecond, millis() catches up by incrementing the counter by two milliseconds instead of one. This is why you should never condition something on millis() having one specific value.

The almost correct solution to this problem is shown in Juraj's answer:

if (millis() > next) ...

This is not quite correct because it fails each time millis() rolls over back to zero. The correct solution is

static unsigned long last;
if (millis() - last >= 500) {
    last += 500;
    ...
}

Because the subtraction follows the rules of modular arithmetic, this is guaranteed to work across the rollover events.

  • the overrun is after 49 days. if the sketch has restart scheduled every day the the subtraction is unnecessary – Juraj Jan 1 '18 at 15:06
  • 11
    @Juraj: Scheduling restarts is a very contrived way of avoiding the rollover “issue”, which is a non-issue to start with if you do the arithmetic properly. – Edgar Bonet Jan 1 '18 at 16:09
  • if it has scheduled restart for other reasons – Juraj Jan 1 '18 at 16:47
  • 1
    @Juraj Then when those restarts stop... suddenly you have an unexpected, bizarre situation. – wizzwizz4 Jan 1 '18 at 19:07
  • 1
    You're in good company with scheduled reboots! theguardian.com/business/2015/may/01/… And if you forget it's just a trivial complete electric shutdown of Boeing’s 787 and [potential] “loss of control” of the aircraft. – ta.speot.is Jan 2 '18 at 4:49
5

Your timing doesn't work, because it has a low probability to read a millisecond that is an exact multiple of 500.

Do it this way:

  static unsigned long next = millis();
  if (millis() - next > 500) {
    next += 500;
  • About your recent edit: note that, because both millis() and next are unsigned long, their difference is always positive. – Edgar Bonet Jan 4 '18 at 14:30
1
void loop() {
  unsigned long cnt=millis();
  if(!(cnt % 500)) {
    PORTD^= (1<<PD3);
  }
}

At every iteration, cnt is set with the number of milliseconds elapsed since you turned on the Arduino... So this is not gonna work. At all. What you need is two variables. One that holds the last millis() count when you blinked the LED, and for the current millis() count:

unsigned long lastCount;
void setup() {
  pinMode(3, OUTPUT);
  lastCount=millis();
}

void loop() {
  unsigned long cnt=millis();
  if((cnt-lastCount)>499) {
    PORTD^= (1<<PD3);
    lastCount=cnt;
  }
}
0

I always use this

if((millis()/500)%2==0)) 
{
    PORTD |= (1<<PD3);
}
else
{
    PORTD &= ~(1<<PD3);
}

It first divides millis by 500ms. So you get the number of 500ms chucks (since the Arduino started). Then you check to see if this number is even. If so, turn the led pin on. Else turn the led pin off.

  • 1
    Division is a computationally expensive operation on most embedded micros. You should avoid it if possible. – Alnitak Jan 2 '18 at 12:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.