| View previous topic :: View next topic |
| Author |
Message |
KTrenholm
Joined: 19 Dec 2012
Posts: 43
Location: Connecticut, USA
|
| PIC24F ADC Done function getting stuck? |
 Posted: Wed Jan 29, 2014 10:09 am |
 |
|
Hi all,
I'm finding a strange issue with some code I wrote to take consecutive reads of the ADC on a PIC24FV32KA302. In this specific case I'm looping through this code:
| Code: |
unsigned int8 index = 0;
unsigned int16 reading = 0;
int1 done = 0;
for (index=0; index < 100; index++){
restart_wdt();
read_adc(ADC_START_ONLY);
while (!done){
done = adc_done();
}
reading = read_adc(ADC_READ_ONLY);
VBrt[index] = reading;
}
|
What ends up happening with this code is every couple minutes it gets hung up inside the adc_done() function and causes a processor reset due to WDT timeout. I tried kicking the WDT within the while(!done) loop to see if it was just getting stuck in the loop rather than the adc_done() function but it still resulted in an eventual WDT reset. In the time between starting up and when it gets stuck (usually 3-4 minutes into execution) the ADC conversions come back perfectly fine.
I have a workaround which is simply a 50uS delay between when the ADC starts and when I read it (it should be ample time to do a conversion):
| Code: |
for (index = 0; index < NUM_ADC_INDEX; index++){
restart_wdt();
read_adc(ADC_START_ONLY);
delay_us(50);
reading = read_adc(ADC_READ_ONLY);
VBrt[index] = reading;
}
|
This seems to work without any problems but I would much rather have it trigger the read immediately after the ADC raises the done bit if possible so I'm not having to guess when the ADC is finished.
Has anyone come across anything like this problem that might have a suggestion? I am on PCWHD 4.132. |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Wed Jan 29, 2014 3:05 pm |
|
|
What happens if you get rid of all that stuff and just do this ?
| Code: | | reading = read_adc(); |
This should be the equivalent of your multi-line code.
Also, you didn't post your ADC setup lines, which could have some error
that causes the problem. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19541
|
|
| Posted: Thu Jan 30, 2014 1:40 am |
|
|
Several of the other PIC's in this family have an errata on the ADC, where the 'done' bit cannot reliably be used to detect the end of the conversion.
The solution is to define the interrupt bit (with a #bit statement), and test this instead.
You are not alone in finding that other chips which do not list this as a problem, intermittently do this. Search the Microchip forums, and you will find quite a few threads which end up coming down to this. Slowly more chips are having this added to their errata list....
It is affected by the clock rate of the CPU, the clock selected for the ADC, and whether any acquisition time is programmed in the ADC setup.
If I remember correctly, if you program some acquisition time, the problem disappears.
However as PCM_programmer says, keep it simple.
The internal function, has a fix for this that can be enabled, if you have the IDE. Select the device editor, and in the 'Errata' box, tick the square marked 'ADC sticks'. This then adds code to avoid the problem, if you use the standard function.
You should simplify the code to a 'bare minimum', and then report it to MicroChip, since it looks as if you have found another chip with the problem...
Best Wishes |
|
|
KTrenholm
Joined: 19 Dec 2012
Posts: 43
Location: Connecticut, USA
|
|
| Posted: Thu Jan 30, 2014 8:29 am |
|
|
| PCM programmer wrote: | What happens if you get rid of all that stuff and just do this ?
| Code: | | reading = read_adc(); |
This should be the equivalent of your multi-line code.
Also, you didn't post your ADC setup lines, which could have some error
that causes the problem. |
That was actually the first thing I tried, I should have mentioned that.
As far as ADC setup it's not terribly out of the ordinary.
| Code: |
/*In header file*/
#device ADC=10
#define ADC_POT_CHANNEL 1 /*ADC Channel for Vbr*/
/*On Startup*/
setup_adc(ADC_CLOCK_INTERNAL);
setup_adc_ports(sAN1); //AN1 and AN5 analog. VSS - VREF+ range (0-5v)
set_adc_channel(ADC_POT_CHANNEL);
|
| Ttelmah wrote: | Several of the other PIC's in this family have an errata on the ADC, where the 'done' bit cannot reliably be used to detect the end of the conversion.
The solution is to define the interrupt bit (with a #bit statement), and test this instead.
You are not alone in finding that other chips which do not list this as a problem, intermittently do this. Search the Microchip forums, and you will find quite a few threads which end up coming down to this. Slowly more chips are having this added to their errata list....
It is affected by the clock rate of the CPU, the clock selected for the ADC, and whether any acquisition time is programmed in the ADC setup.
If I remember correctly, if you program some acquisition time, the problem disappears.
However as PCM_programmer says, keep it simple.
The internal function, has a fix for this that can be enabled, if you have the IDE. Select the device editor, and in the 'Errata' box, tick the square marked 'ADC sticks'. This then adds code to avoid the problem, if you use the standard function.
You should simplify the code to a 'bare minimum', and then report it to MicroChip, since it looks as if you have found another chip with the problem...
|
I'll give these a shot and see if it does anything. Thanks for the help! |
|
|
alan
Joined: 12 Nov 2012
Posts: 357
Location: South Africa
|
|
| Posted: Thu Jan 30, 2014 9:34 am |
|
|
Also maybe try resetting the done bit. You reset it when entering the loop but then the test does not evaluate for the other 99 readings. Go straight from START to READ.
Regards |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19541
|
|
| Posted: Thu Jan 30, 2014 9:36 am |
|
|
Simplest one to try is to add ADC_TAD_MUL_4 and see if anything improves.
Best Wishes |
|
|
KTrenholm
Joined: 19 Dec 2012
Posts: 43
Location: Connecticut, USA
|
|
| Posted: Thu Jan 30, 2014 10:07 am |
|
|
Just an update to note what I've tried.
The first thing I tried was setting the ADC Stuck in the errata. I also added ADC_TAD_MUL_4 into the arguments of the setup_adc function. This didn't seem to do too much. Maybe caused it to be a bit less frequent but that could just be me.
Right now I'm using the following code and it seems to be stable. Looks like triggering off the ISR is the way to go:
*Note that ADC_Done is a enumerated type BOOLEAN I made 0=FALSE 1=TRUE
| Code: |
/*ADC DONE ISR*/
#int_ADC1
void ADC_ISR(void){
ADC_Done=TRUE;
}
/*ADC READ FUNCTION*/
unsigned int8 index = 0;
unsigned int16 reading = 0;
for (index = 0; index < NUM_ADC_INDEX; index++){
restart_wdt();
read_adc(ADC_START_ONLY);
while (!ADC_Done){
restart_wdt();
}
reading = read_adc(ADC_READ_ONLY);
ADC_Done = FALSE;
VBrt[index] = reading;
}
|
| alan wrote: |
Also maybe try resetting the done bit. You reset it when entering the loop but then the test does not evaluate for the other 99 readings. Go straight from START to READ.
Regards
|
you're totally right I can't believe I didn't catch that. I'll give that a shot next build. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19541
|
|
| Posted: Thu Jan 30, 2014 3:58 pm |
|
|
Whoa a lot.....
The ADC interrupt triggers when the conversion is complete. You don't want multiple starts and reads inside the interrupt handler. One read only.
You don't even want an interrupt handler!....
| Code: |
#bit ADIF=getenv("BIT:AD1IF")
ADIF=FALSE;
read_adc(ADC_START_ONLY);
while (!ADIF) ;
value=read_adc(ADC_READ_ONLY);
|
Use this as the entire read_adc replacement. Do _not_ enable INT_ADC.
Best Wishes |
|
|
KTrenholm
Joined: 19 Dec 2012
Posts: 43
Location: Connecticut, USA
|
|
| Posted: Thu Jan 30, 2014 4:11 pm |
|
|
| Ttelmah wrote: | Whoa a lot.....
The ADC interrupt triggers when the conversion is complete. You don't want multiple starts and reads inside the interrupt handler. One read only.
You don't even want an interrupt handler!....
| Code: |
#bit ADIF=getenv("BIT:AD1IF")
ADIF=FALSE;
read_adc(ADC_START_ONLY);
while (!ADIF) ;
value=read_adc(ADC_READ_ONLY);
|
Use this as the entire read_adc replacement. Do _not_ enable INT_ADC.
Best Wishes |
You misunderstand. That code for the read isn't inside the interrupt (gee give me SOME credit lol)
The only line of code in the ADC_ISR is ADC_Done = TRUE.
The actual application code starts the conversion and holds in the loop until the ISR fires and sets the flag high. The loop will then break and perform the read. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19541
|
|
| Posted: Thu Jan 30, 2014 4:15 pm |
|
|
Fair enough, but you are then wasting an age. The interrupt gets called when the interrupt flag gets set. You currently have:
Flag gets set.
Handler gets called (lots of overhead).
You set your indicator.
Handler exits
code sees your indicator.
You can just test the interrupt flag _directly_. You never need call an interrupt handler at all. You might as well add back your time delay with the current approach....
In fact some of the ADC's, recommend using a delay, and reading at a very specific time after triggering the conversion or they will hang (some of the PIC18's have this one....).
Best Wishes |
|
|
KTrenholm
Joined: 19 Dec 2012
Posts: 43
Location: Connecticut, USA
|
|
| Posted: Thu Jan 30, 2014 4:20 pm |
|
|
Overhead isn't really a concern but yeah I'll end up going with your method just for good practice. One less interrupt.
I'll also try this on another PIC18 based design that exhibited the same issue. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19541
|
|
| Posted: Fri Jan 31, 2014 2:11 am |
|
|
As a question/comment, what PIC18's are you having ADC problems with?.
This particular problem with the 'done' flag, is specific to the PIC24. There are other problems with some PIC18's, but they require a more complex solution (in fact some are so bad, that you really need to pick another chip....).
In particular there are a few PIC18's, where the ADC itself gets into problems unless it is read at a totally specific time, the cycle before the DONE/ADIF bits set. The 'fix' in the erratum, is to set the GO/DONE bit, wait an exact number of machine cycles (which means interrupts have to be off), then read the registers. A real 'mess'....
Best Wishes |
|
|
KTrenholm
Joined: 19 Dec 2012
Posts: 43
Location: Connecticut, USA
|
|
| Posted: Fri Jan 31, 2014 8:29 am |
|
|
| Ttelmah wrote: | As a question/comment, what PIC18's are you having ADC problems with?.
This particular problem with the 'done' flag, is specific to the PIC24. There are other problems with some PIC18's, but they require a more complex solution (in fact some are so bad, that you really need to pick another chip....).
In particular there are a few PIC18's, where the ADC itself gets into problems unless it is read at a totally specific time, the cycle before the DONE/ADIF bits set. The 'fix' in the erratum, is to set the GO/DONE bit, wait an exact number of machine cycles (which means interrupts have to be off), then read the registers. A real 'mess'....
Best Wishes |
Oh wow that's brutal. Some of these microchip errata are unreal.
I looked up the code I was thinking of and it turns out it's another PIC24F, not a PIC18. Same series at the one I created this thread about actually (a PIC24FV32KA304). Next time I have to test one of them I'll try this fix. |
|
|
KTrenholm
Joined: 19 Dec 2012
Posts: 43
Location: Connecticut, USA
|
|
| Posted: Fri Jan 31, 2014 9:58 am |
|
|
| Ttelmah wrote: | Whoa a lot.....
The ADC interrupt triggers when the conversion is complete. You don't want multiple starts and reads inside the interrupt handler. One read only.
You don't even want an interrupt handler!....
| Code: |
#bit ADIF=getenv("BIT:AD1IF")
ADIF=FALSE;
read_adc(ADC_START_ONLY);
while (!ADIF) ;
value=read_adc(ADC_READ_ONLY);
|
Use this as the entire read_adc replacement. Do _not_ enable INT_ADC.
Best Wishes |
So I went ahead and tested this and I get an issue with the PIC restarting every few minutes with an "unknown" cause. I tried throwing a restart_wdt() into the while(!ADIF) loop just for laughs to see if anything would change and it now it just locks up after a while rather than rebooting. Thoughts/Ideas? It's almost like it's getting a WDT timeout but it's not showing up when I check the reset cause. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1355
|
|
| Posted: Fri Jan 31, 2014 3:19 pm |
|
|
What is the frequency of your oscillator/crystal or if not an oscillator/crystal, what clock settings are you using? Ideally, I would like to see your full list of FUSES and your #use delay() statement.
I know it seems like a weird question, but I work with the F version of these chips a lot, so I am curious if it is a similar problem I ran into. |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
