I’m glad you got your buttons fixed. Looks liks snap is best avoided!
Your code is possibly suffering from interrupts. I think. The data types you are holding your durations in are long but the ATmega328 is byte sized and doesn’t have 16 or 32 bit instructions as such.
Millis() turns interrupts off to update its long variables, as it is done within the ISR for Timer/counter 0’s Overflow Interrupt. That allows the update to work correctly.
Your updates to the duration generate code to update 4 bytes from a value that is 4 bytes long. Between each byte, in interrupt can occur, changing the values in the bytes not yet copied over to your variables.
Try adding noInterrupts(); before, and interrupts(); after, your assignments from millis() to your duration variables.
I think that will solve those problems.
You are aware that millis() will roll over at some point? That will affect things too. It leads to long periods of darnkess or light! (Rollover happens every 2^32 + 1 milliseconds.)
Cheers,
Norm.
1 Like
Hi @mianqi,
sorry, I was wrong! Don’t bother. While I wa correct that interrupts could be a problem, they are not because whenever you call millis() interrupts are off while the 4 bytes are transferred:
unsigned long millis()
{
unsigned long m;
uint8_t oldSREG = SREG;
37c: 2f b7 in r18, 0x3f ; 63
// disable interrupts while we read timer0_millis or we might get an
// inconsistent value (e.g. in the middle of a write to timer0_millis)
cli();
37e: f8 94 cli
m = timer0_millis;
380: 80 91 09 01 lds r24, 0x0109 ; 0x800109 <timer0_millis>
384: 90 91 0a 01 lds r25, 0x010A ; 0x80010a <timer0_millis+0x1>
388: a0 91 0b 01 lds r26, 0x010B ; 0x80010b <timer0_millis+0x2>
38c: b0 91 0c 01 lds r27, 0x010C ; 0x80010c <timer0_millis+0x3>
SREG = oldSREG;
That’s the assembly code generated for the code in the millis() function which fetches the value of timer_0_millis into a variable named m ready to be returned. You can see a cli instruction above the fetching of 4 bytes in 4 separate instructions. The SREG = oldSREG is where the interrupts are enabled again - assuming they were before they were turned off.
False alarm, sorry!
The problem looks to be in your code. There is an example of using millis() rather than delay() on the Arduino web at https://www.arduino.cc/en/tutorial/BlinkWithoutDelay which does work. Maybe you can study that code and see what’s different from yours?
You are pulling the millis() value twice, well 4 times, in loop() and each one will be very different. The value increments 1,000 times every second, so even a short time between getting the value and using it will cause it to be different, then when you assign the value to one of the duration variable, it will have moved on again.
You need to grab millis() when you enter the loop, every time. You should, I think, also be holding the previous time that you toggled the LED. You can then check that millis (now) - millis(then) > duration to determine if it’s time that the LED was toggled. That’s what the Arduino example does.
HTH
Cheers,
Norm.
1 Like
