Could be a wrong type? [solved: integer overflow]

Go To Last Post
14 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi freaks,

I am using an ATmega32 with avr-libc and AVR-Studio 4.15, and I am trying to execute the following transfer function, that convert a value readed though ADC to "lux" (I'm using in this form to avoid floating math):

lux = ((KA * adc * adc) + (KB * adc)) / SCALE

I want to print the "lux" result onto the LCD display, but the result is very strange above certain "adc" value (for example, above 300 counts, the result show values like 429464). For small values, the function works fine.
I suspect that the problem seems to be something regarding the range of the variables used (type). Please, note that I am not so good in C :oops:, but everithing is working well except this conversion.

I would like to know if my approuch is wrong (and where) and if there is a way to simulate only this part of code (in runtime mode) into AVR-Studio, for debug purpose.

This is my code (the problematic part):

Constants:

#define KA    82      // Coeficient of X^2
#define KB    1487    // Coeficient of X
#define SCALE 10000   // Scale factor

Variables

static int adcval;    // ADC reads put it value here
char sLux[7];	// String with the result of conversion

Function call and to show into display:

Calculate(adcval, KA, KB, SCALE, sLux);	// sLux string seems to have a problem
LCD_PutStr(sLux);		// This is working perfectly

Function (this is the last one I tested): Please note that I was growing the types until this.

void Calculate(int value, int a, unsigned int b, long scale, char *str)
{
  static unsigned long value2;               // value^2
  static unsigned int var;                   // result
  //
  value2 = (unsigned long)(value * value);         // Get value^2
  //
  var = (((unsigned long)(a) * value2) + ((unsigned long)(b) * value)) / scale;
  //
  ultoa(var, str, 10);        // Convert to ASCII string
}

To calculate the maximum size of the variables, I did the following:

- For value = 1023,
	lux = ((82 * 1023 * 1023) + (1487 * 1023)) / 10000
	lux = (85815378) + (1521201) / 10000
	lux = 87336579 / 10000
	lux = 8733
	So, an int output variable seems to be fine (int var)

- As (value * value) is an "int * int" operation, I need a 32 bits variable.
So an unsigned long seems to be fine (unsigned long value2).

Well, any help will be appreciated.

Thanks.

Teach is learn twice. So, what do you think regarding learn again?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It looks to me that your typecasting is making the math a long, so it won't fit in 'var', and it needs to be a long, maybe...

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you for your tip Mr. Jones11. However despite made "var" as long, the problem remains.

Oh! shit! (sorry)... My brain still burning!

Teach is learn twice. So, what do you think regarding learn again?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Maybe this needs to be bigger...

char sLux[7];

For a quick test, try 20 instead of 7. Run the simulator for values that work and don't work to see what's up. You can simulate this section, but you'd have to start a new pjt. put the relevant code , vars, etc. into it.

If I was converting a uint16_t, my char buff has to be big enough for the MAX. the var could be, if my range gets high enough, plus 1 more for the '\0' . So if a uint16_var's max. is > 10,000, I need :

char  buff[ 5 + 1 ]; // For utoa()

uint32_var, 2^ 32 ( billions ):

char   buff[ 10 + 1 ]; // For ultoa()

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you again Mr. Jones11.

The quick test doesn't work. The same simptoms.

That is incredible, for "value = 201", the LCD shows "429320", but the right result must be "361". For lowest "value" everything seems to be allright.

I think I will follow your suggestion and open a new project just to run the problematic part of code.
Seems to be, tomorrow I will have a hard long day.

Teach is learn twice. So, what do you think regarding learn again?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
(unsigned long)(value * value)

This will overflow. You need to cast to long before the multiply.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Have you tried printing your intermediate values?

I think that this cast is in the wrong place, coming "after" the actual math:

  value2 = (unsigned long)(value * value);

Try something like this instead:

  value2 = ((unsigned long)value * value);

For clarity, I think I'd be inclined to explicitly do the precision conversions, step by step:

void Calculate(int value, int a, unsigned int b, long scale, char *str)
{
  static unsigned long value2;               // value^2
  static unsigned int var;                   // result
  //
  value2 = (unsigned long)value;  // Convert
  value2 *= value2;  // square
  value2 *= (unsigned long)a; // Add a
  value2 += ((unsigned long)(b) * value));
  value2 /= scale;
  var = value2;  // back to 16bits
  ultoa(var, str, 10);        // Convert to ASCII string
} 

With appropriate added code, you can look at where things are going wrong (or not), and the compiler really ought to be good enough to come up with essentially similar code: there's not particular reason to put everything on one line...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This is the sort of thing where having a board like an Arduino can come in handy:

// lux = ((KA * adc * adc) + (KB * adc)) / SCALE 

#define KA    82      // Coeficient of X^2
#define KB    1487    // Coeficient of X
#define SCALE 10000   // Scale factor 

static int adcval;    // ADC reads put it value here
char sLux[7];   // String with the result of conversion
char sLux2[7];   // String with the result of conversion 

void setup()
{
  Serial.begin(19200);
}
void loop() {
  Serial.println("adcval  calculate   modifiedcalc");
  for (adcval=1; adcval < 1024; adcval+=50) {
    Serial.print(adcval);
    Serial.print("     ");
    Calculate(adcval, KA, KB, SCALE, sLux);   // sLux string seems to have a problem
    Serial.print(sLux); 
    Calculate2(adcval, KA, KB, SCALE, sLux2);   // sLux string seems to have a problem
    Serial.print("         ");
    Serial.println(sLux2);
  }
  delay(10000);
}


void Calculate(int value, int a, unsigned int b, long scale, char *str)
{
  static unsigned long value2;               // value^2
  static unsigned int var;                   // result
  //
  value2 = (unsigned long)(value * value);         // Get value^2
  //
  var = (((unsigned long)(a) * value2) + ((unsigned long)(b) * value)) / scale;
  //
  ultoa(var, str, 10);        // Convert to ASCII string
} 

void Calculate2(int value, int a, unsigned int b, long scale, char *str)
{
  static unsigned long value2;               // value^2
  static unsigned int var;                   // result
  //
  value2 = (unsigned long)value;  // Convert
  value2 *= value2;  // square
  value2 *= (unsigned long)a; // Add a
  value2 += ((unsigned long)(b) * value);
  value2 /= scale;
  var = value2;  // back to 16bits
  ultoa(var, str, 10);        // Convert to ASCII string
} 
adcval  calculate   modifiedcalc
1     0         0
51     28         28
101     98         98
151     209         209
201     36104         361
251     16         553
301     250         787
351     36268         1062
401     303         1378
451     122         1734
501     36263         2132
551     36165         2571
601     36107         3051
651     347         3571
701     371         4133
751     36180         4736
801     6         5380
851     153         6064
901     342         6790
951     33         7557
1001     304         8365
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Code:
lux = ((KA * adc * adc) + (KB * adc)) / SCALE

I want to print the "lux" result onto the LCD display, but the result is very strange above certain "adc" value (for example, above 300 counts, the result show values like 429464). For small values, the function works fine.


I think the simple way is to define constants as unsigned long type.
This works:

#define KA    82UL      // Coeficient of X^2
#define KB    1487UL    // Coeficient of X
#define SCALE 10000UL   // Scale factor 
int lux,adc;

int main(void) 
{                               
   lux = ((KA * adc * adc) + (KB * adc)) / SCALE ;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

:D Well, it seems now it's all right. I made the suggested changes and the results are ok.

Thanks Koshchi, you're right. As confirmed by westfw, that "cast" was in the wrong place.

Thank you westfw, your suggestions were very valuable. In fact, solve things step-by-step makes it much simpler to understand. Thanks also for running on your Arduino, which proves the results. Thanks for your time and for your teaching. You were very clear and precise.

I also thank Visovian for the suggestion. However, the change of type of that constants seems unnecessary.

I think now I understand and I could learn.
Thank you all for your help.

I think this topic can be closed. Problem solved!

Teach is learn twice. So, what do you think regarding learn again?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I also thank Visovian for the suggestion. However, the change of type of that constants seems unnecessary.

It's just another way to achieve the same thing (promotion of the calculation to a wider type). For my money it is the "cleaner" solution. YMMV.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I also do not understand why to pass arguments "a", "b", "scale" to func Calculate().
I would do this:

#define KA    82UL      // Coeficient of X^2
#define KB    1487UL    // Coeficient of X
#define SCALE 10000UL   // Scale factor
int lux,adc_value;
char tempstr[8];

//--------------------------------------------------

void Calculate(int adc, char *str)
{
   lux = ((KA * adc * adc) + (KB * adc)) / SCALE ;  
   utoa(lux, str, 10);    // Convert to ASCII string
} 

//--------------------------------------------------

int main(void) 
{                               
   Calculate(adc_value,tempstr);
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you Clawson, I did understand.

Visovian wrote:
I also do not understand why to pass arguments "a", "b", "scale" to func Calculate().

Visovian, this is because I have more than one sensor, each with it proper function transfers constants, so I can use the same function to calculate. Thank you for your tips.

Teach is learn twice. So, what do you think regarding learn again?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, now I understand it.