 |
 |
| View previous topic :: View next topic |
| Author |
Message |
xieliwei
Joined: 13 Nov 2011
Posts: 5
|
| ADC, floating point and interrupts |
 Posted: Sun Nov 13, 2011 1:36 am |
 |
|
I'm facing a problem trying to get a piece of code working. After some testing, it appears that my floating point operations and printing gets interrupted by the ADC interrupt.
Here's a condensed version of the code running on a DSPIC30F4013 at 120/4=30MHz using FRC_PLL16
| Code: |
#define NUMADC 12
volatile unsigned int16 adcval[NUMADC] = {0};
volatile int curradc = 0;
#int_ADC1
void ADC1_isr(void)
{
int oldadc = curradc;
unsigned int16 tempadc;
tempadc = read_adc(ADC_READ_ONLY);
adcval[sno[curradc]] = tempadc;
curradc = (curradc+1)%NUMADC;
set_adc_channel(curradc);
read_adc(ADC_START_ONLY);
return;
}
void main(){
float meep=0.351245242;
setup_adc_ports(sAN0 | sAN1 | sAN2 | sAN3 | sAN4 | sAN5 | sAN6 | sAN7 | sAN8 | sAN9 | sAN10 | sAN11);
setup_adc(ADC_CLOCK_DIV_64 | ADC_TAD_MUL_31);
enable_interrupts(INT_ADC1);
enable_interrupts(INTR_GLOBAL);
fprintf(debug,"Moop!\r\n");
fprintf(debug,"%5f\r\n", meep);
set_adc_channel(curradc);
read_adc(ADC_START_ONLY);
fprintf(debug,"Meep!\r\n");
fprintf(debug,"%5f\r\n", meep);
}
|
Output
| Code: | Moop!
0.351245
MeeMeeMeeMeeMeeMee
|
I understand that that's how interrupts work, but I require the ADC values to be as fresh as possible while my code in the main body processes it. The ADC acquire rate is already as slow as the compiler allows me to set it. My current solution is to make the read_adc(ADC_START_ONLY); call in a timer set to go off at a slower rate, but is there a better solution?
Also, why does the print statement end up restarting after servicing the interrupt? The compiler version is 4.124. |
|
 |
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Sun Nov 13, 2011 3:34 am |
|
|
Please provide the complete code. There are missing variable definitions, so we can't see if your code is possibly writing out of bounds, which could be simple explanation for unexpected behaviour.
The shown output seems to refer to a different code with different order of print statements. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19539
|
|
| Posted: Sun Nov 13, 2011 3:44 am |
|
|
Some comments:
Avoid, is using the '%' operator inside the interrupt. This itself uses the int16 division operator (using int16 value, which is the default on the DSPIC), unless used with binary values (8, 16 etc.). This then implies that interrupts will be disabled in all int16 divisions in the main code (including those inside the printf etc.)....
CCS, in some of their examples, use the %, but then carefully use binary values, and don't point out that other values may introduce problems...
So:
1) Change the declaration of curradc to int8. No point in using int16 for this.
2) Get rid of the '%' operator. Use:
| Code: |
if (++curradc == NUMADC) curradc=0;
|
Which gives the same effect.
Then you don't show your fuses, so we can't tell if it may be restarting for a watchdog. However I'd suspect something tied up with re-awakening from sleep. You should not have the code 'dropping off the end' as it does. You need to remember that there is no 'operating system', running outside the code, for the code to drop back to. CCS fills the memory after the code (by default), with sleep instructions, and if you look at all examples, the code loops, so it does not drop off the end..... Your printout, won't have actually finished when the fprintf returns (hardware buffer), so the processor goes to sleep with characters still to send (hence the Mee). The behaviour then seems to be implying that one interrupt at least is still working (the ADC should have stopped).
Then, you don't show the declaration of sno[], so your saved adc value could be going into odd parts of memory. Do you really need this?. Accessing an array, is _slow_. Accessing an array indexed by another array, is even slower... Doing this inside an interrupt, is taking a lot of time.
You need to check how the ADC is setup. You _need_ to have SSRC<2:0> set to 111, for auto conversion. Otherwise, when you trigger the conversion, acquisition will automatically end. Don't assume the compiler has automatically done this....
Realistically you should be using a timer to control this. Have the timer trigger the start of the ADC conversion, and then interrupt when the conversion completes. Selecting a conversion clock that is significantly below the optimum value, degrades accuracy on the ADC.
Best Wishes |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Sun Nov 13, 2011 7:01 am |
|
|
| Quote: | | Avoid, is using the '%' operator inside the interrupt. This itself uses the int16 division operator (using int16 value, which is the default on the DSPIC), unless used with binary values (8, 16 etc.). This then implies that interrupts will be disabled in all int16 divisions in the main code (including those inside the printf etc.).... |
The assumption doesn't apply for PIC24/dsPIC, that uses a dedicated DIV instruction without involving reentrance problems. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19539
|
|
| Posted: Sun Nov 13, 2011 9:47 am |
|
|
It seems to still apply, it just doesn't take so long. The setup code round the DIV, is still common, I've checked, and interrupts are disabled if you use %. The loss of time on the PIC24, is tiny compared to the PIC16/18, but it is still worth avoiding it....
Best Wishes |
|
|
xieliwei
Joined: 13 Nov 2011
Posts: 5
|
|
| Posted: Sun Nov 13, 2011 10:27 am |
|
|
Thank you for your replies!
Sorry that I didn't provide the full code earlier, was in a hurry to attend to something.
Here is the full skeletal code (with suggested changes) that replicates the problem:
main.h
| Code: | #include <30F4013.h>
#device ADC=12
#FUSES NOWDT //No Watch Dog Timer
#FUSES FRC_PLL16 //Internal Fast RC Oscillator
#FUSES PUT64 //Power On Reset Timer value 64ms
#FUSES BORV27 //Brownout reset at 2.7V
#FUSES BROWNOUT //Reset when brownout detected
#FUSES MCLR //Master Clear pin enabled
#FUSES NOPROTECT //Code not protected from reading
#FUSES NOWRT //Program memory not write protected
#FUSES NODEBUG //No Debug mode for ICD
#use delay(clock=120000000)
#use rs232(UART2,baud=115200,parity=N,bits=8,errors,stream=debug) |
main.c
| Code: | #include <main.h>
#define NUMADC 12
#byte ADCON1=0x02A2
#bit SSRC0=ADCON1.0
#bit SSRC1=ADCON1.1
#bit SSRC2=ADCON1.2
volatile unsigned int16 adcval[NUMADC] = {0};
volatile int8 curradc = 0;
int8 sno[NUMADC] = {3, 4, 5, 6, 7, 8, 9, 10, 11, 2, 1, 0};
#int_TIMER1
void TIMER1_isr(void){
read_adc(ADC_START_ONLY);
return;
}
#int_ADC1
void ADC1_isr(void)
{
int oldadc = curradc;
unsigned int16 tempadc;
tempadc = read_adc(ADC_READ_ONLY);
adcval[sno[curradc]] = tempadc;
if (++curradc == NUMADC) curradc=0;
set_adc_channel(curradc);
//read_adc(ADC_START_ONLY);
return;
}
void main(){
float meep=0.351245242;
setup_timer1(TMR_INTERNAL|TMR_DIV_BY_256, 0x1D4);
setup_adc_ports(sAN0 | sAN1 | sAN2 | sAN3 | sAN4 | sAN5 | sAN6 | sAN7 | sAN8 | sAN9 | sAN10 | sAN11);
setup_adc(ADC_CLOCK_DIV_64 | ADC_TAD_MUL_31);
SSRC0=SSRC1=SSRC2=1;
enable_interrupts(INT_TIMER1);
enable_interrupts(INT_ADC1);
enable_interrupts(INTR_GLOBAL);
fprintf(debug,"Moop!\r\n");
fprintf(debug,"%5f\r\n", meep);
set_adc_channel(curradc);
//read_adc(ADC_START_ONLY);
fprintf(debug,"Meep!\r\n");
fprintf(debug,"%5f\r\n", meep);
while(1){
fprintf(debug,"OK\r\n");
delay_ms(10000);
}
} |
Output
| Code: | Moop!
0.351245
Meep!
0.351245
OK
Moop!
0.351245
Meep!
0.351245
OK
Moop!
0.351245
Meep!
0.351245
OK |
Somehow the program keeps restarting with this test code.
Regarding the use of sno[], the adc values are supposed to be processed in sequential order. However, due to some hardware design issue, a few sensors had to be swapped around. So my thought was that since I would have to use some sort of sensor mapping anyway, I might as well remap at the adc interrupt instead of doing it multiple times in the main body. Is that line of thought sensible? |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19539
|
|
| Posted: Sun Nov 13, 2011 10:32 am |
|
|
Try increasing the default stack size.
By default on the PIC24, the value selected is too low, especially if you start using things like floating point printf....
Best Wishes |
|
|
xieliwei
Joined: 13 Nov 2011
Posts: 5
|
|
| Posted: Sun Nov 13, 2011 10:44 am |
|
|
| Ttelmah wrote: | Try increasing the default stack size.
By default on the PIC24, the value selected is too low, especially if you start using things like floating point printf....
Best Wishes |
After adding:
#build(stack=1024)
to main.h, it didn't seem to make any difference in execution. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19539
|
|
| Posted: Sun Nov 13, 2011 11:14 am |
|
|
Add a diagnostic at the start, to find what the restart cause is.
Also post the fuses used.
The comment about sno, still applies though. Just have a second counter like curradc, set it to 3 on boot, and have it wrap the same way as curradc. Saves a lot of memory accesses.
Best Wishes |
|
|
xieliwei
Joined: 13 Nov 2011
Posts: 5
|
|
| Posted: Sun Nov 13, 2011 11:23 am |
|
|
Hmm, hang on...
Making the stack size change does fix the problem with the actual code! That's why experience is important! Thank you Ttelmah and others! 
Now I wonder what's causing the test code to reset?
Also, I'm seeing a interrupt disabled to prevent re-entrancy warning for @readbita when I place this code segment into my while(1) loop:
| Code: | if(!input(PIN_A11)){
calibrate = 1;
}else{
calibrate = 0;
}
if(!input(PIN_D9)){
if(run){
run = 0;
delay_ms(1000);
}else{
delay_ms(1000);
run = 1;
}
} |
The warning goes away if I replace the input(PIN_XX) with a dummy variable. Is there a way I can read a button input without having my interrupts disabled? |
|
|
xieliwei
Joined: 13 Nov 2011
Posts: 5
|
|
| Posted: Sun Nov 13, 2011 11:41 am |
|
|
| Ttelmah wrote: | Add a diagnostic at the start, to find what the restart cause is.
Also post the fuses used.
The comment about sno, still applies though. Just have a second counter like curradc, set it to 3 on boot, and have it wrap the same way as curradc. Saves a lot of memory accesses.
Best Wishes |
Using CCS's restart_cause(), it returns RESTART_TRAP_CONFLICT. Using the interrupt routine by FvM ( http://www.ccsinfo.com/forum/viewtopic.php?t=36479 ), it appears to be an address error. Trying to figure out where that address is now.
Are the fuses you want the ones listed in the main.h file I posted earlier?
Regarding your sno solution, great idea! Thank you!
Edit:
The offending instruction is at 06A6
| Code: | 069A E00000 cp0.w 0x0000
069C AF2042 btsc.b 0x0042,#1
069E 370006 bra 0x0006ac
06A0 09352B repeat #13611
06A2 000000 nop
06A4 093FFE repeat #16382
06A6 000000 nop
06A8 E90000 dec.w 0x0000,0x0000
06AA 3AFFFA bra nz, 0x0006a0
06AC 060000 return |
I'm not experienced with assembly at all... |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Sun Nov 13, 2011 5:03 pm |
|
|
| Quote: | | The offending instruction is at 06A6 |
Doesn't look like. It's a simple delay loop without any data accesses.
Once more, the related source code isn't shown. |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
