Erroneous output in multiplying number in ATmega32

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

I was a making a project in which the intensity of  LED glow increase with increase the intensity of light falling on a LDR.

I am using an ATmega32 , configured at 8MHz internal clock ,i have connected the LDR to pin 4 of port A of ATmega it is giving correct ADC values .Now for PWM to increase/decrease the LED light intensity i am using 16 bit timer 1 and pin OC1A  is connected to with LED via a resistor of 330 ohm . The PWM on the output pin is 4KHz ,fast PWM mode with ICR1 as top and OCR1A to set the high time,also it is configured to clear OC1A pin on compare match. To check the values of ADC result and OCR1A which is calculated form ADC value using the equation 

OCR1A = (1999*ADC_result)/1023;

i am using UART and checking it on my laptop.

but the value of  OCR is coming out to be a very large number  , or sometimes some random number (i have added screenshots for reference).

My USART, PWM  and ADC all libraries/functions are working correctly independently.

i have copied the main function of the code below for reference.  

   

 

int main(void)

{

InitADC();

USART_Init(51);//9600 baud rate

configure_PWM();

int adc_result;

long OCR_value;

long intermediate;

 

    while (1) 

    {

USART_Transmitchar(0x0D);                        // newline

adc_result = ReadADC(4);                         //reading ADC value from pin 4

USART_TransmitString("ADC result :- "); //sending string using UART

USART_TransmitNumber(adc_result);                // sending 10 bit ADC number in 

USART_Transmitchar(0x0D); //newline

intermediate = adc_result*1999; //ERROR OCCURS HERE

USART_TransmitString("intermediate :- ");

USART_TransmitNumber(intermediate);

USART_Transmitchar(0x0D);

OCR_value = intermediate/1023; //ERROR HERE AS WELL

USART_TransmitString("OCR result :- ");

USART_TransmitNumber(OCR_value);

USART_Transmitchar(0x0D);

set_pwm(OCR_value);

     _delay_ms(260);

    }

}

 

 

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

 

How to properly post source code: http://www.avrfreaks.net/comment...

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

there is one bug here:

intermediate = adc_result*1999; //ERROR OCCURS HERE

adc_result is 16-bit integer and the multiplication is done with 16-bit precision. For example, 878*1999=1795102 and the result can't be fit in 16 bits, so overflow occurs. You should change this line to:

intermediate = (long)adc_result*1999; //no error here anymore

And I suggest you put the complete code, really.

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

Also, rather than "int" / "long" I'd recommend use of stdint.h types to make widths more obvious but remember that 1999 (and other literals) are "signed int" which for AVR means -32768 .. +32767. Feel free to use 1999UL if you mean an unsigned constant in 32 bits

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

Thank you very much for the help ,

intermediate = (long)adc_result*1999;

and

intermediate = adc_result*1999UL;

both worked for me it does not shows any errors.

also please excuse me for not posting the complete code,this is the first time i am asking for help online.

thank you very much again .

Last Edited: Thu. Oct 12, 2017 - 12:49 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Not that this is standard 'C' behaviour - nothing specifically to do with AVR

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

Yeah but the thing that makes it AVR specific is sizeof(int)==2 rather than the "usual" expectation of sizeof(int)==4

 

That catches a lot of folks moving from "big iron" to AVR.

 

stdint.h helps a lot but I'm not sure there is a solution to know the width of numeric literals?

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

clawson wrote:
the thing that makes it AVR specific is sizeof(int)==2

But that would apply to any 8- or 16-bit processor - not just AVR.

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

clawson wrote:

Yeah but the thing that makes it AVR specific is sizeof(int)==2 rather than the "usual" expectation of sizeof(int)==4

 

That catches a lot of folks moving from "big iron" to AVR.

Right.  Furthermore, for the OP, it can really pay to think about less obvious ways to do this kind of scaling math on a <32-bit processor (keeping in mind that your source data, ADC_result  is only precise to 10 bits in this case).  32-bit multiplies on an 8-bit device like the AVR are rather cycle-intensive.  32-bit divides even moreso.  Sometimes they are necessary but often they are not.

I did a quick spreadsheet looking for integer ratios of 1999/1023 that can keep the math within 16 bits.  I found that 43/22 is only off by 1 part in 3998 - that is much less error than the 1-in-1024 ADC resolution.  So you can use 16-bit math and write

OCR1A = (43U*ADC_result)/22U;

You need to use unsigned math, first to prevent the intermediate 43*ADC_result result from overflowing negative, and second because OCR1A is an unsigned register (as is ADC_result).

Another approach, which uses a 32-bit multiply but eliminates a division, is to scale your factor (1999/1023 in this case) so that the divisor is a power of 2.  In this case, you can replace the divide by a faster right shift.  If the right shift can be >>8, >>16 or >>24, the compiler may be smart enough to recognize that it can just chop off entire low-end bytes.  So for example

1999/1023 * 65536 (2^16) = 128,061, to within less than 1 part in 100,000.  Then you can write
 

OCR1A = (128061U*ADC_result) >> 16;

This calls for a 32-bit multiply, with the desired value being found in the high 2 bytes of the result without any division necessary.

Last Edited: Thu. Oct 12, 2017 - 03:56 PM