user41348: 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;
  }
}


user4628: 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;
  }
}


user37622: 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;


user7508: 
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.

41348: <p>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.</p>

<pre><code>void setup() {
  pinMode(3, OUTPUT);
}

void loop() {
  unsigned long cnt=millis();
  if(!(cnt % 500)) {
    PORTD^= (1&lt;&lt;PD3);
  }
}
</code></pre>

<p>Anyhow the code below works. But why not the previous code?</p>

<pre><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) &gt;= 500) {
    PORTD^= (1&lt;&lt;PD3);
    prev = current;
  }
}
</code></pre>
 4628: <pre><code>void loop() {
  unsigned long cnt=millis();
  if(!(cnt % 500)) {
    PORTD^= (1&lt;&lt;PD3);
  }
}
</code></pre>

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

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

void loop() {
  unsigned long cnt=millis();
  if((cnt-lastCount)&gt;499) {
    PORTD^= (1&lt;&lt;PD3);
    lastCount=cnt;
  }
}
</code></pre>
 37622: <p>Your timing doesn't work, because it has a low probability to read a millisecond that is an exact multiple of 500.</p>

<p>Do it this way:</p>

<pre><code>  static unsigned long next = millis();
  if (millis() - next &gt; 500) {
    next += 500;
</code></pre>
 7508: <blockquote>
  <p><code>if (!(millis() % 500)) ...</code></p>
</blockquote>

<p>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.</p>

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

<p>The almost correct solution to this problem is shown in Juraj's answer:</p>

<blockquote>
  <p><code>if (millis() &gt; next) ...</code></p>
</blockquote>

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

<pre class="lang-c++ prettyprint-override"><code>static unsigned long last;
if (millis() - last &gt;= 500) {
    last += 500;
    ...
}
</code></pre>

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