Dividing the ADC value with an integer or a float

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

Hi all,

 

I have an issue with dividing integers, and it's driving me insane :)

 

In this function everything works fine every time it is executed:

int measure_voltage() {
    int voltage = (ADCH << 8) + ADCL;

    return voltage;
}

But when I try to divide the value with an integer the first time the function is called, it goes through, the next time ATMEGA8 gets stuck:

int measure_voltage(){
	int voltage = (ADCH << 8) + ADCL;
	voltage /= 20;

	return voltage;
}

 My actual goal was to divide the voltage value with a float after it is read from the ADC. Since I failed, I wanted to try to use an integer for the division, but that, as you can see, fails too.

 

Any ideas?

 

Thanks in advance,

Sasa

This topic has a solution.
Last Edited: Thu. Apr 21, 2016 - 06:39 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Forgot to mention.. If I divide with 1, the function keeps working. I assume the reason why it breaks has something to do with a result not being the whole number. I have tried using round() and tried casting to int etc..

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

The code shown compiles OK. 

What do you mean by "the next time ATMEGA8 gets stuck:"?

How are you initializing the ADC?  

Are you doing another ADC conversion before calling  measure_voltage()? 

Are you testing to see that the ADC conversion has completed after being started?

Are you using the ADC left-justify feature ADLAR?

What is your voltage range?  

Is there some other code that is causing the ADCH and ADCL registers to have the values 0x00, or ADCL be less than 20?

That is the only reason that could cause the division to fail, as far as I can tell.

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

I have set up the ADC to work continuously. This function is called in an infinite loop, and the value passed to the LED display. When I don't have any division, the value from the ADC register is read properly and the MC manages to write out the binary value on the display. If ad a division (with int 20 or float 20.48 as should be used to achieve proper value conversion) the MC gets stuck after first successful cycle (reads the value, converts and displays). Sometimes more than 1 cycle pass (I assume this is when the division value doesn't have a remainder).

 

The ADC result is right adjusted (ADLAR=0). Voltage range is up to Vcc (~5V). None of the code makes ADCH or ADCL any other values than they provide (values turn out correct). Value of the ADC result can be less than 20 (I-m using this as a voltmeter). The problem persists with any value converted by the ADC (< or >20).

 

The code also compiles ok for me.

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

How are you so sure it's freezing within that function? I have a feeling you don't fully understand integer division so you're blaming the failure on the part that seems the spookiest to you. There's nothing special about remainders with integer division that will cause your code to fail, but it's possible certain values of voltage are causing errors elsewhere in your program. Try posting all your source code.

Edit: just so you're clear, an integer divided by an integer is always an integer in C, there's never any float or any other kind of decimal involved. Any decimal part you might expect just disappears into the void.

Last Edited: Wed. Apr 20, 2016 - 12:14 AM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Also, there's no reason you couldn't do:

 

int measure_voltage(){
	int voltage = ADC;
	return voltage/20;
}

But let's see the code where you set up your ADC. I bet it's getting stuck there.

If you don't know my whole story, keep your mouth shut.

If you know my whole story, you're an accomplice. Keep your mouth shut. 

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

I have used same ADC setup for another project, so I ruled that option out. Anyway, here is the code for setting up:

	DDRC &= 0xFE;
	ADMUX &=0x10;
	ADMUX |=0x00;
	ADCSRA &=0xF3;
	ADCSRA |=0xE3;

I agree about the part.

return voltage/20;

I also like the optimized code :) But that comes at the end when everything works :D

 

Anyway, here is the whole code translated, if anyone wants to read it all out:

#include <avr/io.h>
#define F_CPU 8000000UL
#include <util/delay.h>

unsigned char LCD_numbers[10] = {
	0x3F, //0
	0x06, //1
	0x5B, //2
	0x4F, //3
	0x66, //4
	0x6D, //5
	0x7D, //6
	0x07, //7
	0x7F, //8
	0x6F, //9
};

unsigned char measure_voltage(){
	int voltage = ((ADCH << 8) + ADCL) / 20;

	return voltage;
}

unsigned char pull_the_first_digit(unsigned char number){
	return number/10;
}

unsigned char pull_the_second_digit(unsigned char number){
	while (number >= 10) {
		number -=10;
	}
	return number;
}

int main(void)
{
	DDRB = 0xFF;
	DDRD = 0xFF;

	PORTB = 0x00;
	PORTD = 0x00;

	DDRC &= 0xFE;
	ADMUX &=0x10;
	ADMUX |=0x00;
	ADCSRA &=0xF3;
	ADCSRA |=0xE3;

	while(1) {
		unsigned char voltage = measure_voltage();

		PORTB = LCD_numbers[pull_the_second_digit(voltage)];
		PORTD = LCD_numbers[pull_the_first_digit(voltage)] | 0x80;
	}
}

So to recap.. The only issue is when I divide with a number (in this case with 20). Even if I return another value from measure_voltage() and leave the division in the back. I have tried to eliminate all possibilities, and it all comes back to the division with a number :)

 

There are some changes in returned types than my previous posts. In this case I use unsigned char for everything. I have also tried using int for completely everything in the project, it behaves the same way. I have also used various ways of writing the measure_voltage() function, tried casting values etc.. This is the most 'stable' version :)

Last Edited: Wed. Apr 20, 2016 - 06:19 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Rewrite your magic numbers with bitmasks using the official BIT_NAMEs.
This will involve you reading the data sheet.
In the process you will understand what is happening.
.
If you still have a problem, post the re-written code. We will be pleased to explain.

Last Edited: Wed. Apr 20, 2016 - 06:24 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'll do my best if I get time. I am a bit rusty, haven't worked with MCs for a long time. Thanks anyway for all posts :)

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

Did you test your LCD (say, sending arbitray digits?)

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

Yes, I work step by step, and this is the first thing I made before working with ADC to make sure I could display values. :)

Also when using division in measure_voltage(), during the first loop of the program (or some more if they pass before it gets stuck), the value in volts is displayed correctly on the display with a DP as a floating point.

 

The problem is literally when I try to divide two integers and the result of division has a remainder.

I will try later by removing completely all functionality from the program and just have division in the main() along with a blinking diode or some other indicator and paste the code here again.

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

It seems to me that your division by 10 works even with a remainder.  Therefore the problem is not in your division.

Is is possible that you've not declared 'voltage' as 'volatile' and so the compiler decides you never need to read it again and so sits and spins on the same value?

 

Not a C expert,

S.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
unsigned char measure_voltage(){
	int voltage = ((ADCH << 8) + ADCL) / 20;

	return voltage;
}

The return type is unsigned char, but you try to return type int

 

Also, instead 

int voltage = ((ADCH << 8) + ADCL) / 20;

use 

int voltage = ADC / 20;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

To tell you the truth, I had an issue similar to this, but in the case where I used a global variable (Clawson pointed this out to me at one point in time). So I figure in this case, you are referring to the voltage level within the while loop. Now that you mentioned it, I will play around with this idea later during the day and see if it works out.

 

One thing though, I did try to place the calculation directly into the while loop, without calling a helper function and storing it into any variable, and didn't work... But who knows, at that point I was worn out for the day.. maybe I missed something.

 

Thanks :)

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

As I mentioned, I did try declaring the return type as int, also tried casting on return, also tried casting into an unsigned char and then return, also tried everything in the program with int :D

 

Thanks for the hint about reading directly from ADC, I do like more optimized code :)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1
int voltage = (ADCH << 8) + ADCL;

Just to emphasise a point others have been making....

 

If you pick up any AVR datasheet and read the description of ADCL/ADCH you will find words to the effect:

 

Your line of C gives no such guarantee. In fact it's most likely that ADCH is read before ADCL there.

 

If you insist on reading it as two halves then combining then use:

int voltage = ADCL;
voltage += (ADCH << 8);

But as others have pointed out the compiler defines a 16 bit symbol called "ADC" as well as the two 8bit symbols ADCH/ADCL so you can just use:

int voltage = ADC;

In fact you don't really need a variable called "voltage" here at all. This would work just as well:

uint16_t measure_voltage(void) {
	return ADC / 20;
}

 

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

Thanks for the detailed comment :)

 

I'll try with using ADC directly. I cannot remember for sure, but I think had ADCH and ADCL read separately into chars and then combining them into an int. I used the same configuration in another project which was a sound visualizer. The result there was left aligned and only used ADCH (if I remember correctly), so I see based on your comment where I might have been wrong. The awkward thing is that this part works ok:
 

int voltage = (ADCH << 8) + ADCL;

And even if you divide this result with a 1, it continues working properly.. I change the measured voltage, and the value changes on the LED. But if I ever ever use division by any other number, that's when it gets stuck.

 

I agree with the optimized measure_voltage helper function, but as mentioned.. I have tried it as well, and left optimization for the end when I make it work :D

 

I'll post back a reply when I try what everyone suggested.

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

If speed matter don't use / even with a const.

 

If you can live with >>4 or >>5 do that or mul with 13 and use only the high byte. (1/20 is about 13/256)

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

Thanks for pointing it out :) I am a bit rusty with low level programming.. I'll do some test calculations to see how and if I could benefit from this in my case.

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

You're using Aref as your voltage reference, what do you have Aref connected to?

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

I'm still a little fuzzy on what OP means by "gets stuck".

Supposing division really is the problem,

my guess is a timing issue.

To speed things up: return div5[ADC>>2].

((ADC>>2)*(0x100/5))>>8 might also be good,

but is not as accurate.

Iluvatar is the better part of Valar.

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

Connected to Vcc

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

By getting stuck I meant the MC freezes in a state so I gotta reset it.

At some point I have had a situation when writing out to the display was in great intervals (1-2sec) even though the MC is set up to 8MHz and division factor on the ADC is 8.

I haven't had time to test shifting, but if it works, I'll probably go with it, since I don't need a laboratory-precise voltmeter, I just need one to measure voltage to one decimal and push the value to the Raspberry Pi.

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

How are you verifying that it's "freezing in a state"?  Maybe the division by 20 clears any kind of entropy from the lower bits so you just keep getting the same reading over and over, but the microcontroller is still executing the program?  BTW without the divide by 20 you're practically guaranteed to violate the bounds of your LCD array since a char divided by 10 gives a range of 0-25.

Last Edited: Wed. Apr 20, 2016 - 07:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I may have missed an important detail, but my hunch is that you may have a timing problem when the extra time for the division is added into your code.  Try removing the division and adding various delays one at a time, say 20us, 50us, 100us, 200us, 500us, 1000us.  See if there's a delay that "breaks" the code.  That would confirm that you have a timing problem, not a division problem.

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

Waitaminutehere....  He REALLY wants to divide by, get this, "20.48".

 

2048.

 

So quit it with the dividing entirely.  Multiply by 100, shift right 11, and declare yourself done.

 

There's your 'optimized' code.  Division on the AVR takes _forever_ (or it can seem like that.  I've had bad data make me try to divide ~4 billion by 3 through repetitive subtraction... Wrote myself some slightly optimized division routines after that)

 

S.

 

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

Scroungre wrote:

Waitaminutehere....  He REALLY wants to divide by, get this, "20.48".

 

2048.

 

So quit it with the dividing entirely.  Multiply by 100, shift right 11, and declare yourself done.

 

There's your 'optimized' code.  Division on the AVR takes _forever_ (or it can seem like that.  I've had bad data make me try to divide ~4 billion by 3 through repetitive subtraction... Wrote myself some slightly optimized division routines after that)

 

S.

 

 

Problem is division isn't his problem, it's just causing unexpected behavior elsewhere in the program.  I highly doubt an optimized division routine will help at all with the issue he's seeing.

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

Rezer wrote:

Problem is division isn't his problem, it's just causing unexpected behavior elsewhere in the program.  I highly doubt an optimized division routine will help at all with the issue he's seeing.

 

This is true.  As I pointed out earlier, his division by ten works fine.  I just noticed that his first attempts at floating point math were really off-the-wall (with some hints from skeeve about it).  Note that div by ten can also be accelerated (approximately, but close enough) by multiplying and shifting too.

 

For more help to the OP, why the extraordinary division of the main clock for the ADC?  Less noise, perhaps, but much longer time per acquisition.  I wondered how much patience he had - If he left the thing for an hour, or only got frustrated after a few minutes?

 

S.

 

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

Scroungre wrote:

Waitaminutehere....  He REALLY wants to divide by, get this, "20.48".

 

2048.

 

So quit it with the dividing entirely.  Multiply by 100, shift right 11, and declare yourself done.

That's a good catch.  I'm a big fan of multiply-and-shift anyway, and it's a natural here.  One little tweak, to keep from overflowing a 16-bit intermediate value when working with 10-bit ADC values, would be to multiply by 50 and >> 10, or multiply by 25 and >> 9.

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

Sty I couldn't reply sooner.. I literally replaced:

int voltage = ((ADCH << 8) + ADCL) / 20;

With

int voltage = ADC / 20;

As Visovian and Clawson pointed out. Actually, I used division by a float now (20.48) and I get the correct result. In this situation I am using the circuit only as a voltmeter, so there is not much need for efficiency over simplicity (reading seems pretty fast to the human eye). I will optimize my code on the other hand, to point it out to others who might read this thread and I would always encourage others to do so.

 

I see many have misplaced my use of an LED display with an LCD. LED (7 segment displays), the description at the beginning points this out... I guess this comes from a copy/paste name for the map of numbers in my code from another project (LCD_numbers) :) What a noob mistake :D PORTD and PORTC each have a 7segment LED display attached (LSB=a ... MSB=DP), and the value from measure_voltage() is parsed in two digits and or-ing with 0x80 adds the dot, later on outputted to ports. I will remove LEDs since I will send the result through a small network of these devices to a Raspberry Pi. Anyway, this was all pointless to the topic, but to avoid further confusion, wanted to clarify this was the case :)

 

Thanks very much for all who contributed in a polite way, I did point out I haven't worked with this for years. My actual line of work is with object oriented programming, microcontrollers and low level programming were and are just a hobby for me.

 

Cheers and thanks again!

Last Edited: Thu. Apr 21, 2016 - 06:48 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kk6gm wrote:

Scroungre wrote:

Waitaminutehere....  He REALLY wants to divide by, get this, "20.48".

 

2048.

 

So quit it with the dividing entirely.  Multiply by 100, shift right 11, and declare yourself done.

That's a good catch.  I'm a big fan of multiply-and-shift anyway, and it's a natural here.  One little tweak, to keep from overflowing a 16-bit intermediate value when working with 10-bit ADC values, would be to multiply by 50 and >> 10, or multiply by 25 and >> 9.

 

 

Good catch yourself.  For some reason I thought it wouldn't overflow at 100x (maybe because he's only displaying two digits?), but it can.  Furthermore, the AVR instruction set doesn't have an arbitrary-distance barrel shifter, so every bit shifted is another instruction (two if sixteen-bit values, possibly more).

 

Anyhow, have we stamped 'Solved!' on this one yet?  Cheers to you too!

 

S.

 

Last Edited: Thu. Apr 21, 2016 - 06:47 AM