| View previous topic :: View next topic |
| Author |
Message |
JamesW
Joined: 23 Apr 2007
Posts: 91
Location: Rochester, England
|
| PIC24 Trap conflict restart in simple routine... help?? |
 Posted: Thu Oct 30, 2014 5:18 am |
 |
|
Hi Folks,
I am writing a simple routine to read from an ADC and convert to a temperature, but I am continually getting trap resets - and cannot obviously see why.
I am only getting the error when global interrupts are enabled (which makes sense as it's normally caused by saving variables on the stack).
The code is now in a loop where it just runs the routine below on a loop counter.
My ADC is running in 10 bit mode (so a max value of 1024).
| Code: |
void Read_Temperature_Sensor()
{
F_32 WorkingVoltage = 0.0;
F_32 Temperature = 0.0;
UI_16 TempValue = 0;
UI_8 i = 0;
restart_wdt();
/* SAMPLE ADC 16 TIMES */
for (i=0;i<16;i++)
{
TempValue += read_adc();
restart_wdt();
}
/* DIVIDE RESULT BY 16 */
TempValue = TempValue >> 4;
fprintf(DEBUG_PORT,"\r\n ADC %04lu", TempValue);
/* CONVERT TO VOLTS 3.6V IS FULL SCALE ON ADC */
WorkingVoltage = (float) (3.6 / 1024.0 * (float) TempValue);
fprintf(DEBUG_PORT,"\r\n WORKING VOLTAGE %4.2g", WorkingVoltage);
/* SUBTRACT 0.1V FROM THE VOLTAGE - AS WE WILL THEN END UP AT ZERO VOTLS */
WorkingVoltage = (float) (WorkingVoltage - 0.1);
fprintf(DEBUG_PORT,"\r\n WORKING VOLTAGE2 %4.2g", WorkingVoltage);
/* I THINK THIS SHOULD BE -40 DEGREE SUBTRACTION BUT OTHER CODE USES -50 DEGREES */
Temperature = ((WorkingVoltage * 100.0) - 50.0);
fprintf(DEBUG_PORT,"\r\n TEMPERATURE %4.2g", Temperature);
}
|
To clarify - my variable type definitions are :
| Code: |
/* TYPE DEFINITIIONS TO COMPLY WITH MISRA */
#define UI_8 unsigned int8
#define SI_8 unsigned int8
#define UI_16 unsigned int16
#define SI_16 signed int16
#define UI_32 unsigned int32
#define SI_32 signed int32
#define F_32 float
#define F_64 double
|
My RS232 debug output looks like this when global interrupts are disabled
ADC 0221
WORKING VOLTAGE 0.77
WORKING VOLTAGE2 0.67
TEMPERATURE 17.69
when global interrupts are enabled, I get
RESTART : CAUSE 15
ADC 0221
WORKING VOLTAGE
I assume that this is caused by my mix of variables, and the maths involved - can anyone offer any guidance as to how to code it in a manner that the processor doesn't find offensive!
Thanks in advance
James |
|
 |
JamesW
Joined: 23 Apr 2007
Posts: 91
Location: Rochester, England
|
|
| Posted: Thu Oct 30, 2014 5:26 am |
|
|
Not that I think it is processor dependant, but I am using a 24FJ256DA206.
Compiling using version 5.030
And also, I have noticed that my UI_8 and SI_8 are the same - but that isn't the cause of this. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19545
|
|
| Posted: Thu Oct 30, 2014 6:11 am |
|
|
First comment.
You code is a classic example of doing no good at all with a watchdog.
Anything that has multiple 'restart_wdt' calls scattered through it, might as well not be using the watchdog at all.
Do a search here, and online, for how to use a watchdog properly, but meanwhile get rid of it.
Then simplify:
| Code: |
/* DIVIDE RESULT BY 16 */
TempValue = TempValue >> 4;
fprintf(DEBUG_PORT,"\r\n ADC %04lu", TempValue);
/* CONVERT TO VOLTS 3.6V IS FULL SCALE ON ADC */
WorkingVoltage = (float) (3.6 / 1024.0 * (float) TempValue);
fprintf(DEBUG_PORT,"\r\n WORKING VOLTAGE %4.2g", WorkingVoltage);
//Can just as well be coded as:
WorkingVoltage = 0.0002197265*TempValue;
|
Faster. Smaller. Less to go wrong.
Realistically, I'd say I can't see anything here that could be done much better by using scaled integers.
Now I can't see what is causing your problem, except possibly a stack overflow. This used to be common if you used a lot of maths (particularly if the routine is 'deep' inside other code), but is not that common now (the default was increased). However worth just increasing the size and seeing what happens. |
|
|
JamesW
Joined: 23 Apr 2007
Posts: 91
Location: Rochester, England
|
|
| Posted: Thu Oct 30, 2014 6:38 am |
|
|
Changed the maths (thanks).
The processor still restarts.
Thanks for the slapped hand  I will have a look at the watchdog examples and do it properly in the future.
Increased the stack size to 0x200 (512 rather than the default 256) and it works perfectly. I'll be damned!
The irritating thing is that the maths is only in the unit to make sure the temperature sensor is working, in the final version the data will be uploaded to t'internet as an integer!
Thanks again, I'd have never thought of that (I've lost count of the number of times you've helped - you're a star).
James |
|
|
JamesW
Joined: 23 Apr 2007
Posts: 91
Location: Rochester, England
|
|
| Posted: Fri Oct 31, 2014 11:58 am |
|
|
I've had a search on these forums, but the search on this site brings up pages and pages that mention watchdogs with no real examples of how to do it properly.
Likewise there are pages and pages online, with not much luck at a "good practice" guide
Can anyone point me in the direction of a guide on how to use a watchdog properly please?
Cheers for all the help
James |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19545
|
|
| Posted: Sat Nov 01, 2014 9:32 am |
|
|
| My guess would be that the Read routine is being called from another routine, which is itself called from another. and possibly another. depending on the nature of the calls, you could easily have a couple of hundred bytes on the stack at this point. The maths routines then make another call level, and the overflow results. Look at the call tree display if you have the IDE. On each box, if there is a little arrowhead at the bottom, something else is called from this. Click on the box and it opens out. Chase down the tree to the Read_temperature routine and see just how much is above it.... |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9245
Location: Greensville,Ontario
|
|
| Posted: Sat Nov 01, 2014 9:47 am |
|
|
silly question... but.... if you delete the use of the WDT does your code function OK ?
If so, then perhaps have just ONE call to restart based on the WDT(with a long delay), test, and see what happens?
WDT are not normally setup to run until program code is 'up and running'.
Jay |
|
|
SuperDave
Joined: 22 May 2008
Posts: 63
Location: Madison, TN
|
|
| Posted: Sat Nov 01, 2014 11:14 am |
|
|
My soapbox.
| Code: | | WorkingVoltage = 0.0002197265*TempValue; |
There's a difference between accuracy and precision. I'm not sure how accurate your sensor is but the precision of calculation is limited by the least precise element. Which is not to say that you can ignore the sensor.
In this case you have a 10 bit converter (1024) which you sample 16 times increasing the precision to 14 bits (16384) which you then throw away by right shifting 4, so you're back to 1 part in 1024. That reading may be more accurate since you are essentially filtering out the noise but it is no more precise. So, Ttelmah's suggestion to use the code above is better since it avoids the right shift but there's no reason to carry the decimal past 21973 (rounding) since that's a part in 22 thousand and already more precise than the reading precision of a part in 16384.
For example (keeping full precision for the moment and assuming that 100 and 50 are both infinitely precise and infinitely accurate)
full scale 0.0002197265 * 16384 * 100 - 50 = 309.9998976 fprint 309.99
full scale -1 0.0002197265 * 16383 * 100 - 50 = 309.9779249 fprint 309.97
Note that a part in 16 thousand of the ADC results in 2 parts in 30 thousand at fprint despite the precision of a part in 2.2 million in the first constant. And, to repeat, completely ignores the initial accuracy of the sensor which is likely in the +/- 1 degree range.
For real life example, visit a ball park where the outfield distance shows 300 feet (top of the wall, bottom of the wall, from the front of home plate or the back or the center?) and below that shows 91.44 meters!!! Silly. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19545
|
|
| Posted: Sat Nov 01, 2014 3:42 pm |
|
|
Very true.
However remember that the length of the number makes no difference. It is converted at compile time (not run time), into a standard single precision floating point number. The maths is the same however many digits are there. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9245
Location: Greensville,Ontario
|
|
| Posted: Sun Nov 02, 2014 6:26 am |
|
|
re. analog sensor
Another area to be concerned about is the wiring and power feeding the temperature sensor. You just can''t power from the PIC VDD using unshielded wires and no signal conditioning. All analog sensors need a proper filtered 'solid' supply of electrons usually fed through lowC shielded cable. No amount of 'fancy math' will correct for noise like EMI, ringing, etc. Just a few feet of poor wiring will kill a great 10bit ADC into a 8 bit unit!
Also...
generally speaking, temperature sensing and control is a slooooow process. Home furnaces sense every few seconds, have 15 minute control loops, good to +-1*C.
I can't speak about your application( don't know what your project is) but odds are you do not need microsecond response and 5 digit readings.
hth
jay |
|
|
JamesW
Joined: 23 Apr 2007
Posts: 91
Location: Rochester, England
|
|
| Posted: Mon Nov 03, 2014 4:40 am |
|
|
Thanks guys,
The temperature sensor is fitted to the board to provide the ambient temperature of the unit. It is read every 30s and saved to the data log.
Once a day all this information is uploaded. In this application accuracy and speed is not an issue (as mentioned, the data will be uploaded to the server as a 16 bit int, and the actual conversion done at the other end.
The only reason the maths was there in the first place, was the unit does a power on self test at the beginning, and outputs data to a serial port. (So the guys testing the unit can see the temperature sensor is working.
WDT position made no difference. |
|
|
SuperDave
Joined: 22 May 2008
Posts: 63
Location: Madison, TN
|
|
| Posted: Wed Nov 05, 2014 8:29 am |
|
|
Small points.
If the only reason for the calculation is to see if it's working then the only output needed is the raw ADC reading. The tech can know the reading should be between x and y. Think of the ADC as °JW.
If speed is important (it apparently isn't), then dividing by 45516 instead of multiplying by 0.0002197265 is much faster. Then multiply by 1000 instead of 100 to put the decimal in the correct place before subtracting the 50. Divide is slower then multiply but using integer math is much much faster than floating point math. |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
