 |
 |
| View previous topic :: View next topic |
| Author |
Message |
VanHauser
Joined: 03 Oct 2005
Posts: 88
Location: Ploiesti, Romania
|
| Trouble with an array |
 Posted: Fri Nov 04, 2005 3:38 pm |
 |
|
I have a sequence of code running in an interrupt every 250 ms on a PIC18f452 running at 8 MHz. Here is the code:
| Code: | #define MEASURE_BUF_LEN 24
#INT_TIMER1
void isr_tmr1() {
static short measure_tick_now = FALSE;
static signed long buffer[MEASURE_BUF_LEN];
static int buf_i = 0;
static short buf_full = FALSE;
signed long average;
int i;
if (measure_allowed) {
measure_tick_now = !measure_tick_now; // Tick measure once every 2 interrupts
measure.raw = (signed int16)read_adc();
buffer[buf_i] = measure.raw;
buf_i++;
if (buf_i == MEASURE_BUF_LEN) {
buf_i = 0;
buf_full = TRUE;
}
if (measure_tick_now) {
average = 0;
if (buf_full) {
for (i=0; i<MEASURE_BUF_LEN; i++)
average += buffer[i];
average /= MEASURE_BUF_LEN;
}
else {
for (i=0; i<buf_i; i++)
average += buffer[i];
average /= buf_i;
}
if (average == 0)
measure.value = 0;
else {
measure.value = measure.factor * (signed int32)(average - config.adc_4ma);
measure.value += config.ma4;
if (measure.value < 0) measure.value = 0;
else
if (measure.value > 999999) measure.value = 999999;
}
if (measure.value < record.min) record.min = measure.value;
if (measure.value > record.max) record.max = measure.value;
#ifndef RECORDER_DEBUG
record.ymd_end = YmdToLong(time.year, time.month, time.day);
#else
record.ymd_end = MSToLong(time.minute, time.second);
#endif
measure.tick = TRUE;
}
}
|
There is some more code in the interrupt, but this section is creating problems. After a few seconds of running, the LCD connected to the PIC starts behaving weird, displaying strange characters and shifting lines with the shown strings. I have no idea what is going on. If i remove the buffer averaging algorythm and if I use only the last ADC value, the system runs ok. Thanks in advance to anyone that can help me. |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Fri Nov 04, 2005 4:02 pm |
|
|
What's your timer1 interrupt period ? You've got a lot of
time-consuming code inside your isr. I wonder if you have
interrupts coming so often that shortly after you exit from the
isr, you get interrupted again and thrown right back into it.
My suggestions are:
1. Move all that code out of the isr, and just use the isr to set
a global flag which you will poll inside a loop, in main(), and
execute the code if the flag becomes set. Then clear the flag.
2. Make sure your interrupts are set to occur wide enough apart
so that you have time to execute all that massive code before
another interrupt occurs. |
|
|
VanHauser
Joined: 03 Oct 2005
Posts: 88
Location: Ploiesti, Romania
|
|
| Posted: Fri Nov 04, 2005 4:42 pm |
|
|
| I would put all the code in a loop, but the PIC also displays some information screens and menus with configurable parameters, so I need some kind of multitasking. The interrupt's period is 250 ms. I have tested it with a blinking LED, it just shows short bursts of light at the time the isr is executed, so there's no problem here. I suspect the array overlaps something in RAM. Could that be possible? It shows the same symptoms if i declare it global. I am sure it's not a coding error, it's a pretty simple algorythm. |
|
|
dyeatman
Joined: 06 Sep 2003
Posts: 1933
Location: Norman, OK
|
|
| Posted: Fri Nov 04, 2005 4:49 pm |
|
|
Not sure what you are doing in the main loop but if you see the LED flash at all I would bet the ISR is much longer than you think. The only way to know for sure is to use a scope to monitor the toggling of an output pin on entry and exit of the ISR. You said you are triggering the ISR every 250ms. Any significant amout of writing to the LCD could take a big chunk of that depending on how you are doing it.
Keep in mind the TOTAL time you have for EVERYTHING associated with that ISR trigger inside and outside the ISR is 250ms max otherwise one cycle starts "stepping" on the next.
Without being able to see the rest of your code it is almost imposible to answer the question you posed about the array since we have no way to know what you do with it outside the ISR.
What PCM told you earlier is excellent advice... |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Fri Nov 04, 2005 4:59 pm |
|
|
| Quote: |
I suspect the array overlaps something in RAM. Could that be possible? |
Look in the .SYM file in your project folder, and check if your buffer
array addresses overlap the addresses of some other variables.
| Quote: | | I would put all the code in a loop, but the PIC also displays some information screens and menus with configurable parameters, so I need some kind of multitasking |
The way you're doing it now will inject this huge delay at random
locations in your program. I don't think this will affect the LCD routines,
but you could test this by disabling global interrupts when you call the
LCD routines.
Question:
Do you have interrupts disabled when you read the global variables
for display by the LCD ? Some of your variables are more than one
byte in size. What if an interrupt occurs while the code is reading
a 32-bit value ? The output will appear to be garbled. |
|
|
VanHauser
Joined: 03 Oct 2005
Posts: 88
Location: Ploiesti, Romania
|
|
| Posted: Sat Nov 05, 2005 9:17 am |
|
|
Those delays caused by the ISR were indeed messing up the LCD routines. I have modified the lcd_send_byte() function to disable global interrupts while it executes. Now my code works fine. Seems that my LCD (a 24x2 made by Seiko Instruments) doesn't like to wait too long for the data  Thanks a lot guys! |
|
|
neil
Joined: 08 Sep 2003
Posts: 128
|
| ISR length |
| Posted: Mon Nov 07, 2005 7:41 am |
|
|
Hi, although your code may now be working, it will only take a minor change to upset the boat!
The best advice I can give is to take all that code outside the ISR. The *only* things that should be done in an ISR is: Set a flag, initialize a variable, stuff a byte into a buffer. That kind of thing, not maths.
You are adding, dividing, multiplying (signed 32 bit ints I might add!) Remember the PIC is only an 8bit processor, and if you are using a PIC16 or below, it can't do hardware multiplying, so relies on software generated multiplication.
Also, I notice an ADC read in the interrupt as well. This will cause problems, as by default it starts a conversion, then waits for the answer.
Interrupts cannot be treated like normal code execution, they are a special exception which need to be treated with caution!
Here is an example of my timer1 ISR:
| Code: | #int_TIMER1 // Has 4µs Resolution. Rollover 262ms.
TIMER1_isr() {
set_timer1(53035); // Make Rollover period approx. 50ms.
sample = 1; // Set a flag to mark an event in main loop.
if(prescale-- == 0){
prescale=20;
LED1=OFF; // Clear Framing error LED on each pass.
}
}
|
The flag "sample" could be used to trigger all the code you have in the ISR inside your main loop. The code you have as it stands looks like it is using the interrupt directly to run the code on a timer event.
Hope this is of some help!
Neil. |
|
|
Ttelmah
Guest
|
|
| Posted: Mon Nov 07, 2005 8:32 am |
|
|
Worth adding, that you can use an ADC in an interrupt, by taking advantage of the ability to 'seperate' the operations. So:
| Code: |
#int_timer1
void timer_tick(void) {
static int8 state;
static int16 temp;
switch(state) {
case 0:
read_adc(ADC_START_ONLY);
break;
case 1:
temp=read_adc(ADC_READ_ONLY);
break;
}
state=(state+1)&1;
}
|
This way the ADC is read on alternate interrupts, but the time involved inside the interrupt is tiny. Obviously in the 'real' code, something will then have to be done with the value.
I'd suggest actually having the interrupt tick faster, to allow this approach. Then I'd also limit the number of array accesses involved. In the code posted, all the averaging, is done from the array. Instead, have a static variable, maintaining the 'sum', and when each reading is taken, just add the reading to this (so there is only one addition in each interrupt), and the addition is done to a variable at a fixed location. Then when the average is required, take this sum, perform the division, and clear it to start a 'new' sum. Make the division into a binary value (2,4,8 etc.), and the division can be done using rotation, and this will involve only a few machine cycles. This will compare to every array access in the current approach taking perhaps 30 machine cycles, and with a measure buffer length of 24, perhaps giving something in the order of 24*30 cycles for just the data accesses, and then the division too is slow because of being a non binary number (use a measure buffer length of 16, or 32).
Perform the final conversion between a 'reading', and a 'value', outside the interrupt (converting up to an int32, then performing a multiplication, also takes a significant amount of time...).
Best Wishes |
|
|
jds-pic
Joined: 17 Sep 2003
Posts: 205
|
|
| Posted: Mon Nov 07, 2005 11:06 am |
|
|
just to be picky... and generate some conversation... 
.................... state=(state+1)&1;
49C0: MOVLW 01
49C2: ADDWF 51,W
49C4: ANDLW 01
49C6: MOVWF 51
.................... state=1-state;
49C0: MOVLW 01
49C2: BSF FD8.0
49C4: SUBFWB 51,F
.................... state=state^0x01;
49C0: MOVLW 01
49C2: XORWF 51,F
jds-pic |
|
|
Neutone
Joined: 08 Sep 2003
Posts: 839
Location: Houston
|
|
| Posted: Mon Nov 07, 2005 2:28 pm |
|
|
If you really want to pick it apart
| Code: | .................... Initialized=1;
28EE: BSF Initialized
....................
.................... Initialized=!Initialized;
28F0: BTG Initialized
|
The best way to toggle a bit is to toggle a bit. Although that does not leave much room for expanding the number of states. |
|
|
Ttelmah
Guest
|
|
| Posted: Mon Nov 07, 2005 3:28 pm |
|
|
Yes.
I must admit that the code posted, is really 'cut down' from a real sampler that I use. In the original, it selects the ADC channel on the first pass, starts the ADC on the second, performs the read on the third, and generates the output value, with a rolling average, on the fourth. For this, state=(state+1)&3, is I think the most efficient way of doing it, but for the 'two state' example, just toggling the bit would be better.
Best Wishes |
|
|
|
|
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
