possible problem in BlinkWithoutDelay regarding constants

Community Suggestions for the Arduino Project
1

In BlinkWithoutDelay, the constants are declared as long like this:

long interval = 1000;

In arduino 1.0.1, it seems like interval is still treated as an int, so the program does not behave as expected for large values of interval. It works better if 'L' is appended to the value, like this:

interval = 1000L;

In this case, the program works as expected for larger values.

I am a beginner at programming, so I usually assume I'm misunderstanding something, but this sure looks like a mistake in the example. I would be very grateful for any help understanding what's happening here!

Thanks,

2

It doesn't really make any difference for 1000. The compiler can easily fit 1000 into an int.

In any case, the compiler is smart enough to handle larger figures. Try this test:

void setup ()
{
unsigned long foo = 1000;

 Serial.begin (115200);
 Serial.println (foo);
 foo = 12345678;  
 Serial.println (foo);
} // end of setup

void loop () {}

Output:

1000
12345678

There are times when you are right about the suffix, this just isn't one of them.

I think the "blink without delay" example sketch is wrong here though:

long previousMillis = 0;        // will store last time LED was updated
...
long interval = 1000;           // interval at which to blink (milliseconds)

They should both be unsigned long, not long.

3

Okay, I explored this some more tonight, and I think I understand what's happening.

The Arduino compiler is pretty smart about automatically using an appropriate variable type, which made it harder to see what was going on.

I used Serial.print(interval) to see what was happening and I kept getting the right values.

When I set long interval = (1000); I saw 1000 on the serial monitor, just as I expected.
long interval = (100); gave 100
long interval = (5000000); gave 5000000
long interval = (100*100); gave 10000

So far so good, but here's where it got weird:

long interval = (100*1000); gave -31072

It seems that the compiler did the math with ints, since both of the operands are ints, and the result rolled over. In the earlier case of setting interval to 5000000, the program worked as expected because the compiler saw that it needed to use a long.

Appending an 'L' to one of the operands causes the compiler to do the math with longs, and then things make more sense again:

long interval = (1000*100L); gave 100000

This caused me to do a lot of head scratching, and I suspect I'm not the only beginner who has been tripped up by this.

Can you think of a way to clarify this so that other beginners won't have to take as much time as I did to figure out what's happening here?

4

I make any time related variables all unsigned long.

// pre-void setup declarations
unsigned long currentMillis;
unsigned long previousMillis;
unsigned long duration = 1000;
void setup(){
}
void loop(){
currentMillis = millis(); 
if ( (currentMillis - previousMillis) >= duration){ // keep all time elements of same type
previousMillis = previousMillis + duration;
// your other code
} // end time check
} // end loop
5

chopperdave:
It seems that the compiler did the math with ints, since both of the operands are ints, and the result rolled over.

Yes, it would do that. In this case:

long interval = (5000000); gave 5000000

Clearly the compiler recognizes that 5000000 can't be an int so it promotes it.

But:

long interval = (100*1000); gave -31072

The expression (the RH side) involves ints, so the arithmetic is done at int level. Then it happens to be assigned to a long, but too late.

Personally I would promote the first one:

long interval = 100L * 1000;

(I don't know what the brackets achieve in this case).

And as CrossRoads said, intervals are usually unsigned long, not just long (long being signed long).

6

Thank you for clarifying this for me.

I'm pretty clear on what's happening, but it seems like a major pitfall for beginners.

In my opinion it would be an improvement for the compiler to say "Hey, the result of your arithmetic overflowed. Is that what you meant to do?"

Again, thank you for helping me understand.

7

chopperdave:
In my opinion it would be an improvement for the compiler to say "Hey, the result of your arithmetic overflowed. Is that what you meant to do?"

Turn on verbose compilation and you get exactly that:

sketch_aug03a.cpp:4: warning: integer overflow in expression

Code to reproduce:

long interval = (100*1000);
void setup () {} 
void loop () {}
8

IMHO a warning should be displayed without Verbose compilation output.

With verbose set so many messages are output that the final status of the output window is filled with
...avr-ar rcs ...core.a ...*.c.o
"
"
...avr-gcc...
...avr-objcopy... elf ... epp
...avr-objcopy... elf ... hex
The final
Binary sketch size: 466 bytes (of a 32,256 byte maximum)
looks good.

The warning is scrolled off

Not user friendly.

9

I agree, personally. However I believe that some libraries spew out warnings which may be confusing to beginners, even if there is nothing wrong with their code.