Unexpected output from max() function: nesting instructions

Projects Programming
1

Hi everyone!

I'm starting playing with arduino, new to electronics but with some experience on different programming languages including C. Anyway what it was surprising for me was that I was playing with the max/min functions in order to constraint the output of a testing project to read the output from a light sensor, I wanted to limit the output between 0 and 100 so I tried the following code:

void setup(){
  pinMode(9, OUTPUT);
  Serial.begin(9600);
}

void loop(){
  int light = max(0, min(255, map(analogRead(A0), 100, 750, 0, 255)));
      
  analogWrite(9, light);
  Serial.println(light);
  delay(100);
}

The pin 9 was used to light up a led with the output at the same time (I guess this is not important). Values of 100 and 750 in the map function were empirically calculated by looking at the sensor output when covering it or using an additional light source to directly illuminate the sensor.

My surprise was that when observing the console serial output sometimes light values below zero were produced (I cover the sensor with my finger to simulate total absence of external light). However, when using the following code instead, the output was as expected between 0 and 100 (i.e. not showing values below zero):

  int light = min(255, map(analogRead(A0), 100, 750, 0, 255));
  light = max(0, light);
  if (light < 0)
  {
    light = 0;
  }

Both codes should be equivalent. Why in the first case values below zero are stored in light variable? I don't think a bug is possible on something so basic. Perhaps I should review my old programming notes from university :slight_smile: Anybody knows?
Many thanks!

2

Max and Min aren't actually functions. They're macros defined in Arduino.h:

#define max(a,b) ((a)>(b)?(a):(b))
#define min(a,b) ((a)<(b)?(a):(b))

So your code would be expanded by the preprocessor to:

((0)>(((255)<(map(analogRead(A0), 100, 750, 0, 255))?(255):(map(analogRead(A0), 100, 750, 0, 255)))?(0):((255)<(map(analogRead(A0), 100, 750, 0, 255))?(255):(map(analogRead(A0), 100, 750, 0, 255)))

(I think - that was manual expansion - it wasn't easy).

So you see, what was 1 analogRead() is now 4.

At the very least you should do an analogRead() into a variable and then use that variable instead in the max(), min() and map() functions/macros.

3

The problem very likely stems from the min, max, and map macros. They are coded in a way that can cause undesirable side-effects (like your example). There is a discussion here... http://arduino.cc/forum/index.php/topic,84364.0.html ...including the best solution to the problem here... min, max, abs.. are macroses? - #14 by bperrybap - Suggestions for the Arduino Project - Arduino Forum

Please create an issue here... Google Code Archive - Long-term storage for Google Code Project Hosting.

This at the top of your sketch should get you past the bug...

#ifdef max
#undef max
#endif

#define max(a,b) \
       ({ typeof (a) _a = (a); \
           typeof (b) _b = (b); \
         _a > _b ? _a : _b; })

#ifdef min
#undef min
#endif

#define min(a,b) \
       ({ typeof (a) _a = (a); \
           typeof (b) _b = (b); \
         _a < _b ? _a : _b; })
4

Arduino offers a solution in theory with constrain()

int light = constrain( map(analogRead(A0), 100, 750, 0, 255)), 0,255));

but in practice this is a macro too :frowning:

So the final solution is to use a temporary variable.

int raw = map(analogRead(A0), 100, 750, 0, 255);
int light = constrain( raw, 0, 255));