uint8_t * uint8_t oddity

Forum 2005-2010 (read only) Software Syntax & Programs
1

Hi,

I've got a problem with a product of 2 uint8_t variables.

If I do this:

uint8_t val = 255;
uint8_t sat = 255;
uint16_t output = val*sat/120;

the result of output is 'strange' at best, definitely too big. It should be around 540 something, but ends up close to 0xFFFF. Using brackets around (val*sat) to reduce rounding errors is futile as well.

Changing the code to this helps, WHY ?

uint8_t val = 255;
uint8_t sat = 255;
uint16_t product = val*sat;
uint16_t output = product/120;

The actual code is this:

void
set_led_hsv (uint8_t led, uint16_t hue, uint8_t sat, uint8_t val)
{

  /* BETA */
  
  /* finally thrown out all of the float stuff and replaced with uint16_t
   *
   * hue: 0-->360 (hue, color)
   * sat: 0-->255 (saturation)
   * val: 0-->255 (value, brightness)
   *
   */

  hue = hue % 360;
  uint8_t sector = hue / 60;
  uint8_t rel_pos = hue - (sector*60);
  uint16_t const mmd = 255*255; /* maximum modulation depth */
  uint16_t top = val*255;
  uint16_t bottom = val*(255-sat); /* (val*255) - (val*255)*(sat/255) */
  uint16_t mod_depth = val*sat;
  uint16_t slope = mod_depth/120; /* dy/dx = (top-bottom)/(2*60) */
  uint16_t a = bottom + slope*rel_pos;
  uint16_t b = bottom + mod_depth/2 + slope*rel_pos;
  uint16_t c = top - slope*rel_pos;
  uint16_t d = top - mod_depth/2 - slope*rel_pos;
  
  Serial.print("hue: ");
  Serial.println(hue,DEC);
  Serial.print("sat: ");
  Serial.println(sat,DEC);
  Serial.print("val: ");
  Serial.println(val,DEC);
  Serial.print("sector: ");
  Serial.println(sector,DEC);
  Serial.print("rel_pos: ");
  Serial.println(rel_pos,DEC);
  Serial.print("top: ");
  Serial.println(top,DEC);
  Serial.print("bottom: ");
  Serial.println(bottom,DEC);
  Serial.print("slope: ");
  Serial.println(slope,DEC);
  Serial.print("a: ");
  Serial.println(a,DEC);
  Serial.print("b: ");
  Serial.println(b,DEC);
  Serial.print("c: ");
  Serial.println(c,DEC);
  Serial.print("d: ");
  Serial.println(d,DEC);

  uint16_t R, G, B;

  if (sector == 0)
    {
      R = c;
      G = a;
      B = 0;
    }
  else if (sector == 1)
    {
      R = d;
      G = b;
      B = 0;
    }
  else if (sector == 2)
    {
      R = 0;
      G = c;
      B = a;
    }
  else if (sector == 3)
    {
      R = 0;
      G = d;
      B = b;
    }
  else if (sector == 4)
    {
      R = a;
      G = 0;
      B = c;
    }
  else
    {
      R = b;
      G = 0;
      B = d;
    }

  uint16_t scale_factor = mmd / __max_brightness;

  R = (uint8_t) (R / scale_factor);
  G = (uint8_t) (G / scale_factor);
  B = (uint8_t) (B / scale_factor);

  set_led_rgb (led, R, G, B);
}

This function is used to convert HSV color space to RGB.

2

The val and sat variables are uint8_t, so a uint8_t-sized space is used to hold the result of val*sat, which results in an overflow.

You should be able to use an explicit cast, rather than an intermediate variable:

uint16_t output = (uint16_t)(val*sat)/120;

3

Just to be safe, I'd cast both terms:uint16_t output = (uint16_t)val * (uint16_t)sat / 120;When type conversions are subtle, I think it makes the program easier to understand if you are explicit about what you want to do.

Regards,

-Mike

4

So basically use the good old "never assume anything" rule.

Thanks.

Edit: the explicit cast works :wink: