So I was working with a student on reading an analog input (linear pot). The end goal was to read the 10-bit ADC value, then interpret this value as to how long (in arbitrary units) to turn on and off an LED.

Here is the code:

void main(void)
{

init(); //  Initialization code to setup ports, oscillator, etc.


while(1)
{
    ADCvalue=ADCpoll(13); // Get ADC Value
    time_Switch=(long)ADCvalue*10;//  Multiply the ADC value by scaler

    lcdwrite(time_Switch,0);//Displays time_switch on LCD

   RC1=1; //Turn off LED

    for(time_ON=0; time_ON<=time_Switch; time_ON++) //Wait time_switch
    {
        
    }
   RC1=0;  //  Turn On LED
    for(time_OFF=0; time_OFF<=time_Switch; time_OFF++)  //Wait time_switch
    {
 
    }


}
}

This code will work just as expected. However, if you change

time_Switch=(long)ADCvalue*10;

to

time_Switch=(long)ADCvalue*100;

The ADC no longer reads properly. Both the LCD and the flashing LED hint that the ADC value randomly jumps from say 512, down to 234, then back to 502 etc.

In addition, when things got shakey, the ADC value will not make it all the way to 1023 even at full voltage.

My first thought was data type issues. So we typecasted everything, removed the LCD function, and finally reverted back to the *10 multiplier, and repeated the for loop 10 times (emulating the 100x multiplier).

Nothing worked. Even hardcoding the time_switch = 102300 made the thing go crazy.

Any ideas on what to try next? It's almost like the for loop cannot handle this many iterations. But even if it can't, why would it mess up my ADC values?

Interested in any thoughts.

Thanks

 

Do you have a stabilized supply for the ADC?

I use for instance a 78L05, with 1/2 divider.

The regular supply might be too much polluted. What do you use as power supply?

 

Not much to go on. You don't spec the compiler you use, nor give a hint as to how any of your values are defined.

How big is a "long"?

Finally, I believe the type cast binds first, but you might want to try:

 time_Switch=((long)ADCvalue)*100;

 

Have you tried casting it as:

time_Switch=((long)ADCvalue)*100;

Also, your LCD write function may only work for integer-sized variables...

Edit: The_RB beat me to the cast suggestion...

Edit #2: what is the size of time_ON and time_OFF?

 

takao21203 - The board has a LT1121 linear 5V reg and is sufficiently decoupled. Also, this "error" comes from LONG delays between ADC polls, never during shorter, more frequent polling, which would seem odd for true noise.

Ernie - The variables are declared as:
unsigned int ADCvalue;
long time_Switch;
long time_ON;
long time_OFF;

Compiler is Hitech C 10-12-16 9.83

My first thought was data type as well. But consider that we rewrote the function using the *10 multiplier, then running the FOR loop 10 times (nested). Doing this should negate overrun on data types no?

Long is 32 bits and should range +/- 2,147,483,647

tshuck - My LCD function limits out when the numbers are too large (>9999999). Even if that were an issue, that should not be redefining the time_switch (IE, the LED's should not be flashing wrong just because the display is incorrect). I thought of this as well, so as a "just in case" I actually removed the LCDwrite function and still, the LED's blinked haphazardly with the 100 multiplier. Time_ON and Time_OFF have all been defined as long. When we first encountered this error, the first thing I did is define all the variables to long thinking it would fix the issue.


I look at the entire problem like this:

All the variables are declared as LONG having sufficient room for 1023*100(102300). I would say it's just the display messing up/converting to BCD wrong, but the LED's are reciprocating this issue. Then the opposite is true, if the FOR loop is not executing as I expected, why would that have anything to do with the ADC value or the time_switch which is being shown on the LCD.

Thank you for the suggestions thus far.

P.S. I will try the typecasting in parenthesis, but know that I attempted typecasting everything at one point to no avail.

 

...
Edit: The_RB beat me to the cast suggestion...
...

I'd love to take the credit for the 1 minute gazump but it was actually ErnieM.

All us smartass geeks look the same...

 

While we're all still fishin' for the root problem here, could you please post the code for the ADCpoll() function?

Might as well zip up the whole program and attach that too.

All us smartass geeks look the same.

And here I thought I was the handsome one.

 

I'd love to take the credit for the 1 minute gazump but it was actually ErnieM.

All us smartass geeks look the same...

Oh, whoops!

That's what happens when I try to recognize a person by their avatar when trying to remember which colors were there without looking again...

Sorry, about that Ernie, the credit is yours!

@OP: declare time_ON and time_OFF as volatile long... your program may be optimizing those statements out...

 

  lcdwrite(time_Switch,0);   //Displays time_switch on LCD

Normally lcdwrite(,) requires a pointer to a text string. This looks to be trying to display a numerical variable directly?

 

Looks like a possible compiler bug to me. The behavior sounds like what would happen if the value was truncated to 16 bits. Which it should not be, given the declarations and code you have shown us.

I would debug the code and check the values after assignment. Also check the actual iterations of the for loop, the bug could be there.


Bob

 

ErnieM -

unsigned int ADCpoll (unsigned char ch)  //  Use to Poll ADC port
{


    if(ch>13) return 0;  //Invalid Channel

    ADCON0=0x00;

    ADCON0=(ch<<2);   //Select ADC Channel

    ADON=1;  //switch on the adc module

    GO_nDONE=1;//Start conversion

    while(GO_nDONE); //wait for the conversion to finish

    ADON=0;  //switch off adc

    return ADRES;

}

THE_RB -
The lcdwrite() is a function I wrote for the 3 digit LCD w/2 DP. I wanted to do everything in fixed point, so this lcdwrite takes either a long or int

void lcdwrite (long output, int out)

in .0001 and .01 decimal place respectively and does all the truncation and decimal point assignments. I have a hard time blaming this function as A.) it does not modify time_switch and I have even removed the function and the code still falters in seemingly the same fashion.

To clear the air, I need to retry simply having the LCD print ADCvalue without the scaler multiplication and see how it reacts. I believe I did this for most of my testing, but want to make sure. I will also try the typecasting in parenthesis.

 

re the ADCpoll() function:

1) For an invalid ch input the function returns the perfectly legitimate value of zero.

B) Each time you go thru you turn the A2D on and off again. Just turn it on and leave it on, it only uses a small amount of current.

III) After selecting (and connecting) the channel to read there needs be a small delay to allow the internal cap to charge. This isn't the problem with your code as you never change the input, but one day it will be the problem if you reuse this function.

I still don't see what may be causing the problem, but you only provide very small snapshots of your code which makes it very hard to see. If you zip up the entire project someone may be able to put a finger on it.

 

Ernie - Thank you for the feedback. The ADC module turn-on/off is just a creature of habit. I never knew if there were more serious repercussions for leaving it on, now I know. I will add this delay. I take it this is necessary after turning on the ADC module OR switching channels?

So I tried the code changes I spoke of last e-mail and truncated the code to a more compact version that still causes issue:

unsigned int ADCvalue;
long time_Switch;
long time_ON;
long time_OFF;

void main(void) ///  Where main code starts
{
init(); //  Initialization code to setup ports, oscillator, etc.

while(1)
{

    ADCvalue=ADCpoll(13);
    lcdwrite(0,ADCvalue);
    (long)time_Switch=((long)ADCvalue)*100;

   
    RC1=1;
    for(time_ON=0; time_ON<=time_Switch; time_ON++)
    {
    }
  /*  RC1=0;
    for(time_OFF=0; time_OFF<=time_Switch; time_OFF++)
    {
    }
*/

}

}

Note, even with the second FOR loop commented out, the error is present.

If I turn the potentiometer up to 5V (AN13), and use a the *10 multiplier (instead of the 100), I get a reasonable readout on the LCD (~1010). By changing the multiplier to 100, the ADC value (as shown on LCD) changes to around 800 and oscillates down to near the 600 level.

This code is quite long and contains many custom function I have implemented on this board. I could certainly trim it down and package if that would help.

 

Sorry for joining the thread late.

You completely missed the acquisition step in your code. Therefore, the sampling capacitor never gets charged to the full voltage.

Read the "15.4 A/D Acquisition Requirements" in the datasheet.

 

Ernie - Thank you for the feedback. The ADC module turn-on/off is just a creature of habit. I never knew if there were more serious repercussions for leaving it on, now I know. I will add this delay. I take it this is necessary after turning on the ADC module OR switching channels?

Switching channels is the important place to add the delay. You have to allow the internal cap of the A2D to fully charge to the input voltage. (See the data sheet for an overly complicated explanation (where 1/2 LSB means the largest change that doesn't affect the output state) or just go with their recommend minimum.

I ran the code you posted thru the MPLAB simulator using the XC8 compiler, which I believe internally is the same as Hitech C. I made a dummy A2D routine to return the max 1023 value, and commented out the LCD thing.

Since there were no runs, no hits, and no errors I would suggest you make sure the delay inside your A2D read routine is adequate, and STOP TURNING THE DANG THING ON AND OFF.