Broken math with integers...

Hi, my math is broken. I’m writing a program to use a BlinkM MaxM as a light alarm clock. It slowly raises the brightness over 10 minutes as a pleasing way to wake up :slight_smile:

I’m having trouble getting it to delay correctly. I raise the brightness over 256 steps 0->255, with a delay during each step.

I’d like the total time to be 5 minutes, so each step should be 5 mins/256 = 5 * 60*1000 / 256 milliseconds

long mins = 60 * 1000; // 1 min in microseconds
long steps = 256;

delay(5 * mins/steps); // when run, hangs over a minute
delay(5 * 60*1000/256); // when run, also hangs over a minute
delay(1171); // works as delay for 1 second, but hacky!

What’s broken with my math?

#include <Wire.h>
#include "BlinkM_funcs.h"

// Configuration
long mins = 60 * 1000; // 1 min in microseconds
long steps = 256;

// hardware addresses
byte addr = 0x09;
int ledPin = 13;

void setup() {
    // debugging
    //mins = 1000;
    //steps = 16;

    pinMode(ledPin, OUTPUT);
    Serial.begin(9600);
    BlinkM_beginWithPower();
    BlinkM_setRGB(addr, 0,0,0); 
    BlinkM_off(addr); 
}

void loop() {
    if (Serial.available() > 0) {
        while (Serial.available()) Serial.read(); // clear buffer
        doAlarm();
    }
}

void doAlarm() {
    
    blink(1000);
    // fade to red - 5 mins
    for (int i=1;i<=steps;i++) {
      BlinkM_setRGB(addr, (i*256/steps)-1,0,0);
      blink(50); 
      //delay(5 * mins/steps);
      delay(1172);
    }
    
    blink(1000);
    // fade to white - 5 mins
    for (int i=1;i<=steps;i++) {
      BlinkM_setRGB(addr, 255,(i*256/steps)-1,(i*256/steps)-1); 
      blink(50); 
      //delay(5 * mins/steps);
      delay(1172);
    }
    
    blink(1000);
    // hold 50 mins
    delay(50 * mins);
    
    blink(1000);
    // go to black
    BlinkM_setRGB(addr, 0,0,0); 
    BlinkM_off(addr); 
}

void blink(int x) {
    // blink the on-board LED for debugging
    digitalWrite(ledPin, HIGH);
    delay(x);
    digitalWrite(ledPin, LOW);
    delay(x);
}

For any constant that needs to be involved in (long) maths, put an “L” after the digits.

So, “60*1000” needs to be “60L * 1000L”.

Remember that integer math is 16-bit signed integer, meaning the range of values is -32768 to 32767. It doesn’t take a lot of milliseconds before you’re out of bits.

Hi, I suggest to pay attention at the operators precedence, for example in the following line:

delay(5 * 60*1000/256); // when run, also hangs over a minute

if (5 * 60*1000/256) is associated like this: (5*60)*(1000/256) then, considering the integer division the result is 300*3 = 900 if is associated like this: (5*60*1000)/256 the result is 300000/256 = 1171

ea123: Hi, I suggest to pay attention at the operators precedence, for example in the following line:

delay(5 * 60*1000/256); // when run, also hangs over a minute

if (5 * 60*1000/256) is associated like this: (5*60)*(1000/256) then, considering the integer division the result is 300*3 = 900 if is associated like this: (5*60*1000)/256 the result is 300000/256 = 1171

They have equal precedence, so the calculation should be

(((5 * 60) * 1000) / 256)

The result in HEX is 0xFFA4. Because it is a signed number, sign extension yields 0xFFFFFFA4. Which is a TON of milliseconds.

Moral of the story -- don't do math without a programmer's calculator. Overflow and sign extension will bite you in the butt if you use a non-geek calculator (or pencil and paper ...)

Overflow and sign extension will bite you in the butt if you use a non-geek calculator (or pencil and paper ...)

At least if you use them without understanding how the Arduino will perform the same calculation.

Moral is use L with long constants.... Since there is "contagion" of longness, you only usually need to put one L, as in:

5L * 60 * 1000 / 256

A common catch is that the result type of an assignment is ignored by the compiler when computing the expression on the right hand side until its time to do the actual assignment. Thus these two assignments calculate the same 16 bit value, one then sign-extends to get a long (32 bit) result:

int a = 34 * 56 * 20 ;
long b = 34 * 56 * 20 ;

Thus a == b == -27456 == -27456L

So the advantage is that expressions always mean the same thing whatever the context, but the cost is that the expression for b loses information. Personally I'd prefer they both mean +38080 but the assignment for a gives a compile-time error without a cast.