different behavior in "if" using long integers

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

Dear "freaks" - friends,
I am experiencing a weird problem with code inside an if block.
Here is the code that works

unsigned char autogain = FALSE;
signed int uplimit = 15750;
signed int downlimit = 32000;
signed int ADC1=0;
signed int ADC2=0;
		if (autogain == TRUE)
		{
			if ((ADC2>downlimit)||(ADC2<-downlimit))
			{
				if (gain>=2)
				{
					gain--;
				}
			}
			if ((ADC2>-uplimit)&&(ADC2

ADC1 and ADC2 are the readings from a 16-bit ADC chip
with bipolar input.
I am trying to make a very simple auto-gain control and in this way it works

If I use abs() the gain only increases, really I don't know and I can't understand why

		if (autogain == TRUE)
		{
			if ((abs(ADC2) > downlimit)  &&  (gain>1))
				gain--;
			if ((abs(ADC2) < uplimit)  &&  (gain<13))
				gain++;
		}

Anyway here is a third option where I am using some averaging for better accuracy

unsigned char autogain = FALSE;
signed int uplimit = 15750;
signed int downlimit = 32000;
signed long ADC1=0;
signed long ADC2=0;
for (int i=0;i<7;i++)
{
    ADC2 += (signed long)internal_spi_ReadAI(0x00);
}
			
ADC2 /= (signed long)8;

if (autogain == TRUE)
{
    if (((signed int)ADC2>(signed int)downlimit)||(signed int)ADC2<-(signed int)downlimit))		//ADC2
    {
	    if (gain>=2)
	    {
		    gain--;
	    }
    }
    if (((signed int)ADC2>-(signed int)uplimit)&&((signed int)ADC2<(signed int)uplimit))			//ADC2
    {
	    if (gain<12)
	    {
		    gain++;
	    }
    }
}

This last code also doesn't work. The gain only increases. I've even tried casting...
Any suggestions why?
Thanks in advance.

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

You make life hard for yourself when you remove spaces!
Cluttering up code with extraneous casts does not help.

If you have a differential ADC, you will get a signed result. Looking at your last snippet, ADC2 should give you the average of 8 int16_t readings. And this will fit in an int16_t.

if (autogain == TRUE)
{
    if (ADC2 > 32000 || ADC2 < -32000)      //ADC2
    {
       if (gain >= 2)
       {
          gain--;
       }
    }
    if (ADC2 > -15750 && ADC2 < 15750)         //ADC2
    {
       if (gain < 12)
       {
          gain++;
       }
    }
} 

If you hand-trace this logic you will see that the gain will go up to 12. It will only decrease if you satisfy the first condition. And if you think about it, a differential ADC is unlikely to ever reach +32000 or even -32000.

I suggest that you log the actual readings from your ADC. I am surprised that you would ever need a 16-bit ADC. An ATmega324P would give you a differential ADC with plenty enough resolution. And you don't need int32_t arithmetic.

David.

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

Quote:

If you have a differential ADC, you will get a signed result. Looking at your last snippet, ADC2 should give you the average of 8 int16_t readings. And this will fit in an int16_t.

I am using a bipolar single ended analog to digital converter, not differential.
The result is 16-bit with sign.
In the for loop I sum 8 readings which won't fit in a
int16_t if let's say adc readings are around 20000
giving approximately 160000. Then I average.

Quote:
If you hand-trace this logic you will see that the gain will go up to 12

Correct. Thanks
Quote:
I suggest that you log the actual readings from your ADC

The ADC is fully functional after lots of tests and calibrations. I have access to oscilloscope and other crucial equipment.
If I use the first snippet of code everything works fine
the other two are problematic.
Quote:
And if you think about it, a differential ADC is unlikely to ever reach +32000 or even -32000.

Why?
Saturation does not exist for them? Please explain.

Perhaps I didn't explain you correctly.
I want to use the third snippet of code, but using
"signed long" and casting the if block only alternates "gain" variable by increasing, so the "gain" never returns to lower values, even if voltage at the adc's input is 0 V (adc code ~ 0)

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

Quote:
I am using a bipolar single ended analog to digital converter, not differential.

Ok. So it is a differential ADC with an internal -ve input.
Otherwise, how can you get a signed result?

If your 'typical' ADC result is 20000 you will never satisfy the first test.

Your ADC can only satisfy the first test with values -32768 to -32001 or +32001 to +32767

David.

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

David,
Thanks for your answers.
The logic behind this, is that the amplifier before the ADC
must provide a large enough voltage for better dynamic performance.
So the gain of the amplifier is adjusted so that the ADC code is between -15750 ... -32000 or if positive voltage between 15750 ... 32000.
So as you say

Quote:
Your ADC can only satisfy the first test with values -32768 to -32001 or +32001 to +32767

I try to detect whether the ADC will get in trouble.
So if ADC code is greater than 32000 or lower than -32000, I suspect that the voltage tends to go near the limits of saturation so the uC must decrease the gain.
On the opposite side, if the adc code I get is too small, lower than 15750 or greater than -15750 we must increase gain.
If I try to average 8 values of ADC I need in the worst case scenario a 19-bit variable to place the sum of all values. So I use "signed long" which is 32-bit
But then the
      if (autogain == TRUE)
      {
         if ((ADC2>downlimit)||(ADC2<-downlimit))
         {
            if (gain>=2)
            {
               gain--;
            }
         }
         if ((ADC2>-uplimit)&&(ADC2

does not work as expected.
That's why I used casting, but it still behaves wrong...the gain is only increasing, meaning that the second if is always satisfied.
How can I make it work?
The use of abs() function was another observation of mine.
Thanks again.

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

Quote:

does not work as expected.

Quote:

I suggest that you log the actual readings from your ADC.

When you have the actual values fed into this code you can then hand trace oit or even stick the list of samples in an array and single step the code as they are fed into the algorithm and watch for things like intermediate calculation results to see if there's potential for overflow or whatever. I suspect you are simply going to find that your lists of samples does not include excursions into the abs(32000..32767) range.

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

Dear clawson,
In my first post there are 3 chunks of code.
The first works like a charm.
The second only increases gain (although it is supposed to do the same thing with the first code) and I finally get saturation....
The third code implements averaging but again the code only increases gain and I finally get saturation.
I can't figure out why.

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

Quote:

I can't figure out why.

David (and then I) have suggested a way to find out. Any interest in actually doing that or are you happy for this to remain a mystery?

One thing C code is, is deterministic. Fed the same inputs it will (should!) always behave identically.

In the work I do (video processing) we feed the exact same frame data into an algorithm over and over so we can (a) fix bugs in the processing that it's supposed to do and (b) find ways to make it work quicker. Having done that we can then connect it up to a live camera and it should then be able to handle any visual scenario presented to it. If we then find some scenario where it misbehaves we record that sequence and repeatedly play it into the algorithm while debugging until we resolve whatever issue that was.

Recording a sequence of input and then repeatedly playing it into an algorithm to bug fix it or improve it is a very powerful technique.

Both David and I are suggesting that to understand the phenomenon you are seeing you need to know what values are being input when it "doesn't work". As I say, if you record such a sequence you can play it over and over (from an array) while observing the operation of the code until you understand why you see the results you do.

My money is still on the fact that in reality you aren't seeing excursions into the 32000..32767 range. Once you have recorded some samples you will know whether that is true or not.

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

I agree with you (both)...
I have a recording software and the data are showing saturation.
I will attach data and/or photos later (4-5 hours later as I don't have access now) so that you can see also the different behaviors.

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

I'm not sure how that helps - it's YOU that needs to step through the data as it's fed to the algorithm and determine when/why it is/isn't doing gain-- or whatever. I doubt that pictures are going to help much.

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

I guess that the pics only give some clues, of what i am facing.
I will do step by step debugging with JTAG mkII

Attachment(s): 

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

Just do a log of your ADC values. e.g. printf() to terminal or SDCard.

It is the sort of problem that is difficult to do with JTAG. You want the hardware to run without stopping.

David.

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

Have you #included stdlib.h?
I not, I think the implicit declaration for abs is
int abs(...) .
avr-gcc puts ... on the stack instead of in the registers that would be used for
int abs(int)
if it were not inlined.

Did you get a warning?

Moderation in all things. -- ancient proverb

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

It is included.
No warning!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
for (int i=0;i<7;i++)
{
    ADC2 += (signed long)internal_spi_ReadAI(0x00);
}
         
ADC2 /= (signed long)8; 
 

First bug here is that ADC2 contains the sum of 7 readings and then divided by 8. So even +32768 by 7 divided by 8 is 28672, which says "do not change gain"
I will change the code to

  
for (int i=0;i<8;i++)
{
    ADC2 += (signed long)internal_spi_ReadAI(0x00);
}       
ADC2 /= (signed long)8; 

Still I didn't log the ADC readings, I just relaxed and rethought everything.

Yet abs() seems not clear to me...
so I'll do some tests & logs for both and I I'll be back.

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

Quote:

First bug here is that ADC2 contains the sum of 7 readings and then divided by 8. So even +32768 by 7 divided by 8 is 28672, which says "do not change gain"

No, the first bug is that ADC2 is a signed 16-bit variable that can hold +/- 32k.

How many bits does internal_spi_ReadAI() return? 12 bits signed and you are OK. If, say, 16 bits signed then you will overflow with your addition.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
Quote:

First bug here is that ADC2 contains the sum of 7 readings and then divided by 8. So even +32768 by 7 divided by 8 is 28672, which says "do not change gain"

No, the first bug is that ADC2 is a signed 16-bit variable that can hold +/- 32k.

How many bits does internal_spi_ReadAI() return? 12 bits signed and you are OK. If, say, 16 bits signed then you will overflow with your addition.

No. In that version, ADC2 is signed long.

Moderation in all things. -- ancient proverb

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

internal_spi_ReadAI() returns 16-bits
ADC chip is max1135
[url]
http://www.maximintegrated.com/d...
[/url]
How did you conclude that it returns 12-bits?

I guess the first bug is that I exist :lol:

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

Thanks for your answers and help.

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

Quote:
If I use abs() the gain only increases, really I don't know and I can't understand why

Code:

if (autogain == TRUE)
{
if ((abs(ADC2) > downlimit) && (gain>1))
gain--;
if ((abs(ADC2) < uplimit) && (gain<13))
gain++;
}

I tried this code in Studio simulator.
Strange thing.
For me the "gain" only decreases.

In Watch window the value of 'gain' stays unchanged.
But actually it increases as it is seen on PORTB value.
Only when I declare 'gain' volatile, it changes in Watch window.

// Avrstudio 4.17.666
// Winavr 20100110
// Atmega88, 8MHz
 
unsigned char autogain = 1; 
signed int uplimit = 15750; 
signed int downlimit = 32000; 
signed int ADC1= -32000; 

signed int ADC2=15000;
unsigned char gain = 3;
//volatile unsigned char gain = 3; 
 

int main()
{

  while(1)
  {
  
  if (autogain == 1) 
      { 
         if ((abs(ADC2) > downlimit)  &&  (gain>1)) 
            gain--; 
         if ((abs(ADC2) < uplimit)  &&  (gain<13)) 
            gain++; 
      }

      PORTB=gain;
   }
}

Attachment(s): 

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

Quote:

Only when I declare 'gain' volatile, it changes in Watch window.

That's fairly normal when debugging optimised code - why are you surprised?

(if you look at the generated Asm you'll presumably find that 'gain' is cached into a register and the SRAM location "gain" that AS6 is watching is not being updated)

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

I think certain Alzheimer is responsible.
The older I am the more surprises there are around me.
Thanks clawson.

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

clawson:
Now I remember what I meant with my post.

Quote:
If I use abs() the gain only increases, really I don't know and I can't understand why

Didn't OP wrongly think so because he did not see changes in watch window?

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

Quote:

Didn't OP wrongly think so because he did not see changes in watch window?

It's so long ago I haven't a clue but as you've found the first thing you do when watch windows don't work is start peppering about the "volatile"s.

For those interested in such things there's always:

Optimization and the importance of volatile in GCC