Strange behaviour from PIC16F18856 microcontroller pin...ADRES bit stuck high

[ocset]

Hello,

We are using the PIC16F18856 in 4x4mm UQFN package.
Port RA5 works when we program it as digital input…..reads it fine whether input is high or low. Port RA5 also works when we program it as digital output….gives a square wave output fine.

However, when we program the pin as an analog (ADC) input..it reads it but the 9th bit (dec 256) of the ADRES register is strangely always set high….so for example even when we have a grounded input to the pin as an ADC, it reads 0x0100 instead of zero.

Its also strange because 70% of our boards are  fine, and they work, and their ADC doesnt present this  problem.
The non-working boards show a behaviour problem which corresponds to this errrant ADC reading.
We captured  the errant ADC reading whilst using the debugger on one of the bad boards (pickit3 was the debugger)

PIC16F18856 datasheet
https://www.microchip.com/wwwproducts/en/PIC16F18856

We are worried because the errata says something about ADC that we dont fully understand
Errata:
ww1.microchip.com/downloads/en/DeviceDoc/80000706B.pdf

The below is our ADC code, its just standard ADC code so we can post it here...

Code: [Select]

//******************************************************************************
// Measure Light
//******************************************************************************
uint16_t MeasureLight(void)
{
    uint16_t  x;

    Init_Normal_ADC();
    ADC_StartConversion(0x05);          // Port A.5
    while (!ADC_IsConversionDone());
    x = ADC_GetConversionResult();
    return x;
}


//******************************************************************************
// The random.c DALI library change the ADC settings, so we need to restore them
// after a random command. Also this function initialize ADC the first time
//******************************************************************************
void Init_Normal_ADC()
{
       
    // ADGO stop; ADFM right; ADON enabled; ADCONT disabled; ADCS FOSC;
    ADCON0 = 0x84;
   
    ADCON1 = 0x00;
    ADCON2 = 0x00;
    ADCON3 = 0x00;
   
    ADSTAT = 0x00;
   
    // ADCCS FOSC/2;
    ADCLK = 0x00; 
    // ADNREF VSS; ADPREF VDD;
    ADREF = 0x00;

    // CHANNEL --> ANA0;
    ADPCH = 0x00;
   
    // PRECHARGE TIME -->  0;
    ADPRE = 0x00;
   
    // ACQUISITION TIME --> 0;
    ADACQ = 0x00;
     
    // ADDITIONAL CAPACITOR --> 0
    ADCAP = 0x00;

    // AUTO conversion disabled;
    ADACT = 0x00;
   
   
    // ADRPT 0;
    ADRPT = 0x00;
    // ADLTHL 0;
    ADLTHL = 0x00;
    // ADLTHH 0;
    ADLTHH = 0x00;
    // ADUTHL 0;
    ADUTHL = 0x00;
    // ADUTHH 0;
    ADUTHH = 0x00;
    // ADSTPTL 0;
    ADSTPTL = 0x00;
    // ADSTPTH 0;
    ADSTPTH = 0x00;
   
     
    // ADC VALUE REGISTER L
    ADRESL = 0x00;
    // ADC VALUE REGISTER H
    ADRESH = 0x00;
}



//******************************************************************************
// Start ADC conversion
//******************************************************************************
void ADC_StartConversion(uint8_t channel)
{
    // Set CHANNEL
    ADPCH = channel;
    // ADRESL 0x0;
    ADRESL = 0x00;

    // ADRESH 0x0;
    ADRESH = 0x00;

    // Acquisition time delay
    __delay_us(ACQ_US_DELAY);

 
        // Turn on the ADC module
    ADCON0bits.ADON = 1;     

    // Start the conversion
    ADCON0bits.ADGO = 1;
}


//******************************************************************************
// Conversion DONE
//******************************************************************************
uint8_t ADC_IsConversionDone()
{
    // Start the conversion
    return (!ADCON0bits.ADGO);
}

//******************************************************************************
// Get result
//******************************************************************************
uint16_t ADC_GetConversionResult(void)
{
    // Conversion finished, return the result
    return ((ADRESH << 8) + ADRESL);
}


[ocset]

By the way,  if I may add, the offending ADC reading is being taken from an SFH5711 light sensor IC which is also on the PCB. This has an 82k resistor to ground at its output which develops the requisite voltage in response to the SFH5711’s current source output. (the current  source  magnitude corresponds to the light level sensed by the SFH5711). There is also an 100pF capacitor at the SFH5711 output, as in the attached schematic.

I am just realising that the actual impedance presented to the PIC ADC channel  by the light sensor might be  too high for the PIC to get an accurate ADC reading?         

----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Please assist us, because I have just realised from page 346 of the PIC16F18856 datasheet that the ADC module requires the input impedance of the external circuit to be less than 10 kiloOhms, and this because of the leakage current that can come out of the micro pin. (page 346 of the PIC16F18856 datasheet tells us this)
The SFH5711 light sensor circuit has an impedance equal to the 82k resistor that we placed at its output.            
In other words, we have seriously violated the input impedance level required by the ADC  input of the PIC16F18856 microcontroller.           

I presume the reason that 70% of the boards worked was because  the microcontrollers on those boards just happened to have lower  leakage current at their ADC pins?   
We wonder what is the max/min leakage current levels that are likely in the pic16f18856?
Is it up to 100uA?

Also, i presume we can solve this mess by changing the resistor at the output of the SFH5711 light sensor to 10k (instead of the 82k that it already is)........and then just settling for a lower resolution light reading?
We don't have any room on the PCB to add an opamp buffer.

SFH5711 light sensor..
https://dammedia.osram.info/media/resource/hires/osram-dam-2496447/SFH%205711.pdf

PIC16F18856 datasheet:
https://www.microchip.com/wwwproducts/en/PIC16F18856
 

[floobydust]

First I would add the fix for the silicon errata, 1.1 says the A/D conversion complete flag has a one cycle delay.
"This can lead to a false conversion complete scenario (i.e.,  ADGO being cleared), depending if the user code has a bit clear test (BTFSC) instruction on the ADGO bit, immediately after setting the ADGO bit."

Also, I would not leave x unassigned, if the while loop never executes you return garbage.

[cv007]

In addition to any other problems you seem to have-
it appears you are running the conversion clock at fosc/2 (ADCS=0, ADCLK=0)- that's as fast as possible. Which is fine if you are running fosc at something like 1MHz.
Table 23-1.

ADC 101

[ocset]

Thanks, the clock frequency of the micro is set to 16MHz.

The above two lines of code are near the top of the main function

Code: [Select]

#define _XTAL_FREQ  16000000                        // 16Mhz
#define ACQ_US_DELAY (500)                          // ADC time acquisition

[cv007]

'ADC time acquisition' is not conversion time

you need to stay out of the shaded boxes in table 23-1

[SiliconWizard]

Note that I'd be wary of this:

Code: [Select]

return ((ADRESH << 8) + ADRESL);
What compiler do you use?
Some will automatically promote ADRESH (which is 8-bit) to a larger integer before doing the left shift, some won't. I think the "standard" behavior is not to promote it.
That means that 'ADRESH << 8' would actually always evaluate as 0 (8 times left shift of an 8-bit number).

To be safe, replace it with:

Code: [Select]

return ((((uint16_t) ADRESH) << 8) + ADRESL);
(The particular compiler you use may do this for you, but note that most compilers won't. So the kind of code you posted is unsafe/not portable.)

[ocset]

Thanks, ill check with the softy which compiler, but i think its the free XC8 one.
At least, he sends us the c code, and we compile it with the free one from microchip, called XC8.

i  think to be safe we will just get the adres left justified, and then take adresh as our value as an 8 bit number.

[rstofer]

Your device has an output current varying around about 30 uA and you need a source impedance of 10k so, at most, all you get is 0.3V.  That doesn't use up much of the ADC range.

Maybe you don't care about the actual value and can get by using the analog comparator module.  The light level is above some threshold or it's below.

[ocset]

Thanks, As you say,  putting in a 10k resistor at the output of the light sensor is what we have to do. –Even though it gives us a lousy small range of measurement. However, if the leakage current out of the adc pin is low, then we would just leave in our errant 82k resistor.
 Page 610  of the PIC16F18856 datasheet appears to state that it’s a maximum of 1uA leakage. This gives an inaccuracy of  +/-82mV to our measurement setup.
That’s not that bad. But we cant be sure what page 610 is referring to?

[mikeselectricstuff]

Note that I'd be wary of this:

Code: [Select]

return ((ADRESH << 8) + ADRESL);
What compiler do you use?
Some will automatically promote ADRESH (which is 8-bit) to a larger integer before doing the left shift, some won't. I think the "standard" behavior is not to promote it.
That means that 'ADRESH << 8' would actually always evaluate as 0 (8 times left shift of an 8-bit number).

To be safe, replace it with:

Code: [Select]

return ((((uint16_t) ADRESH) << 8) + ADRESL);
(The particular compiler you use may do this for you, but note that most compilers won't. So the kind of code you posted is unsafe/not portable.)

With XC8, most register pairs are also declared as 16-bit symbols without the L/H suffix, so
return(ADRES);
will also work

[ocset]

Please could you advise us…..we have two ADC readings, one is for temperature from a MCP9700 temp sensor IC (on pin RA4) …..the other is light reading from a SFH5711 light sensor IC (on pin RA5). The light sensor has a current source output, but the MCP9700 has a buffered output………I am just wondering if the code has been written such that the S&H capacitor has not had time to discharge from the temp  sensor reading before the light reading is taken?….this could explain why we always get this residual high value in the ADRES  register?
……. I did not realise that if you have two separate ADC inputs to the PIC16F18856, then ultimately both of them end up having to feed into the same S&H capacitor in the ADC of PIC16F18856
The temperature sensor IC has a buffered output, so it can drive the S&H capacitor to the required value in very quick time.

What I now suspect, is that in our software our softy has inadvertently left the temperature channel connected to the S&H capacitor for too long, and then when we take the light reading, it simply reads what the temperature monitor had left in there. This would make kind of sense because the  dreaded residual reading that we are getting in the ADRES register is 0x0100…this is what we would expect to see from the temperature sensor at 25degC….room temperature…hmmmmmm….much head-scratching to do now for us.

Another point is that dry/intermittent joints on the micro ADC input pins could  also make the  ADC readings look higher than they are, because the leakage current from the pin would charge up the 10pF pin capacitor , which would then charge up the S&H capacitor.

MCP9700 datasheet:
http://www.microchip.com/wwwproducts/en/en022289

[ovnr]

It's not like the S&H cap is very large.

There is a spec - or used to be a spec - saying you should wait a period of time (100 µs?) when switching ADC channels. I also noticed you're initializing the ADC every time you measure the light sensor because some library is fucking with it? Brilliant design in that case.

Anyway:

* Add delays to everything. Add a delay at the end of Init_Normal_ADC() and ADC_IsConversionDone(), and increase ACQ_US_DELAY used in ADC_StartConversion() to 200 µs at least.
* Configure the ADC module correctly. You're running at 16 MHz; ADCLK should be 0x0F, not 0x00.


As for the light sensor: You've managed to pick a sensor that's got a very feeble output. I'd try whacking a small cap across it to prevent dips during ADC cycles. Also make sure you've disabled the weak pullup for the pin; not sure if it's done automatically.

[ocset]

Thanks yes, the osc is set to 16MHz, so one thing we can definetely say is that the software engineer has set the conversion clock wrongly. He has set it to FOSC/2, and page 340 of the datasheet clearly says that this is not acceptable for a micro set to 16MHz.
Also, the softy has set ADACQ  to 0x00...which means no acquisition time, so thats not good for us either.

Also, our softy has put in an acquisition delay, but he does this before the ADC module is actually turned ON...so this is absolutely useless.

Also, our softy is setting the  ADLTHL, ADLTHH, ADUTHL, ADUTHH, ADSTPTL and ADSTPTH registers. These all pertain to the ADC module. There is no need for this. This might be having unwanted effects on our ADC reading?

Another thing we would like to do, is to discharge C(PIN) and C(HOLD) capacitors   of page 347 down to ground before starting the acquisition time prior to a ADC conversion…but we are not sure how to set the ADPRE register in order to achieve this?
---------------------------------------------------------------------
Sorry but does anyone know what is it that turns the sampling switch ON so that the C(HOLD) can be charged up?....does the sampling switch get turned on when the analog channel is enabled?
..or is it after the go/done bit is set?
Because our software engineer has set a variable and called it "acquisition delay".......but how does he make this happen at the right time..surely the acquisition delay is what happens after the go/done bit is set?...you cannot start this yourself.....it can surely only be started by setting the go/done bit?...and the actual value of acquisition delay can only be set in the ADACQ register, and it is up to 255* TAD?
So why has our softy made a variable called "acquisition delay"?...surely this is of no use whatsoever?

[ovnr]

Also, the softy has set ADACQ  to 0x00...which means no acquisition time, so thats not good for us either.

ADACQ is for sharing charge between external and internal caps; it's not related to the actual acquisition, despite the name. Leaving it at 0 will just operate the ADC in the old-style PIC ADC mode.

Also, our softy has put in an acquisition delay, but he does this before the ADC module is actually turned ON...so this is absolutely useless.

You mean "__delay_us(ACQ_US_DELAY);" in ADC_StartConversion? It's useful to have there (channel settling), but setting ADON=1 should've happened in init_adc() or whatever it's called.

Also, our softy is setting the  ADLTHL, ADLTHH, ADUTHL, ADUTHH, ADSTPTL and ADSTPTH registers.

Setting or clearing these has no effect on anything; they're just for interrupt flags and shit on the new ADC module.

Another thing we would like to do, is to discharge C(PIN) and C(HOLD) capacitors   of page 347 down to ground before starting the acquisition time prior to a ADC conversion…but we are not sure how to set the ADPRE register in order to achieve this?

This will just make your results worse. Add a substantial amount of external capacitance on the pin, cross your fingers and pray that the input leakage is acceptably low, and do a normal ADC sampling. Cpin and Chold is a total of 15 pF; they're not your problem, and Chold is not something you can even touch directly.

[JPortici]

With XC8, most register pairs are also declared as 16-bit symbols without the L/H suffix, so
return(ADRES);
will also work

You don't say?
But you still have macros to ensure correct read/wrote sequences to timer registers..
Also last time i used them in the debugger they weren't showing any value so i decided to avoid using them..