0

I have a code that I need to read an AD channel in a timer interrupt (with precision time).

Everything is okay if I just read the AD. But I need to use a digital filter, and if I put just a multiplication inside interrupt a have a warning:

This is ok:

#int_RTCC
void  RTCC_isr(void) 
{
   set_adc_channel(0);      
   delay_us(40);  
   unsigned int16 aD = read_adc();
}

But this get warning:

#int_RTCC
void  RTCC_isr(void) 
{
   set_adc_channel(0);      
   delay_us(40);  
   unsigned int16 aD = read_adc();
   aDfilter = aDfilter * 8 + aD * 2;
}

interrupts disabled during call to prevent reentrancy (@MUL3232)

I wouldn't want a timer disable, because I need precision. How can I solve this?

  • What is the warning? Probably you need to do calculation like: `aDfilter = (float)aDfilter * 0.8f + (float)aD * 0.2f;` Btw, you want to *prevent reentrancy* by disabling interrupts? Makes no sense or maybe you need different word, like *prevent recursion*? – unalignedmemoryaccess Sep 26 '17 at 13:04
  • Sory, the correct is: aDfilter = aDfilter * 8 + aD * 2 Don´t need float. The problem is that I´m using multiplication inside and outside interrupts. Is there a way that do this? – Antonio Rafael da Silva Filho Sep 26 '17 at 13:14
  • Basically all your problems originate in this: a PIC is not a PC. – Lundin Sep 26 '17 at 13:21
  • "interrupts disabled during call to prevent reentrancy" Eh? During call of what? What do you mean with "prevent re-entrancy", your problem is rather the lack of re-entrancy. – Lundin Sep 26 '17 at 13:23
  • @Lundin I'm guessing that the error should be 'interrupts disabled during call to prevent reentry'. – Martin James Sep 26 '17 at 13:30
  • What is the type of `aDfilter`? – chux - Reinstate Monica Sep 26 '17 at 14:34

3 Answers3

0

Problem 1: PIC usually means 8 bit CPU. An 8 bit CPU cannot read a 16 bit value atomically (aDfilter). If your RTC interrupt triggers while the main program has only read half the value, your program will crash and burn. You need some means of re-entrancy, which is what the compiler is telling you.

Problem 2: PIC usually means horribly slow CPU, with big interrupt latency. Therefore you should not have arithmetic inside an ISR. This certainly involves floating point calculations. Multiplication of integers might even be bad enough.

Problem 3: PIC usually means horribly slow CPU without a FPU, which means you shouldn't use floating point numbers to begin with. You'll end up invoking software libraries with float support, which is incredibly slow. Nothing in the code shown indicates a need to use floating point in this program.

Solution: Use integers. Implement re-entancy with synchornization means. This is fairly easy to do on single-core microcontrollers where the interrupt always block further interrupts, simple example. Then outsource the digital filter calculation to the caller application. Just add a flag to tell it that there is new data available.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • 1
    'you'll end up invoking software libraries with float support, which is incredibly slow' that, plus, if you use FP outside the interrupt too, you need an FP library that is reentrant, (as if using PIC doesn't bring enough problems). – Martin James Sep 26 '17 at 13:27
  • Worse - the FP library, if reentrant, wil use stack. That means that everything that gets interrupted must have enough stack to support it, or a swap to a separate interrupt stack. It might be necessary to call some lengthy 'FPinit()' function on every use. It's all very messy, even if it works:( – Martin James Sep 26 '17 at 14:13
  • @MartinJames And then... PIC... meaning 1970s architecture with fixed stack depth. How deep calls will the float library use? Very interesting to have that inside an ISR. – Lundin Sep 26 '17 at 14:20
0

You can avoid using int32 multiplication by shifting, something like:

#int_RTCC
void  RTCC_isr(void) 
{
   set_adc_channel(0);      
   delay_us(40);  
   unsigned int16 aD = read_adc();
   aDfilter = aDfilter << 3 + aD * 2;
}
Lisandro
  • 89
  • 1
  • 4
0

(message) interrupts disabled during call to prevent reentrancy (@MUL3232)
I wouldn't want a timer disable, because I need precision.

This is a false concern.

Code is simply protecting itself from an unusual situation that good design can avoid from happening.


When running the timer interrupt service routine (ISR), the re-entrantcy of another timing interrupt would normally only happen in 2 cases:

  1. The timer ISR frequency is too high. This means that the time is takes to handle 1 timer ISR is so short another is occurring before this completes. This is not a good design. To solve, insure the timer frequency is not so high.

  2. Latency. A timer ISR occurred while handling some other ISR that is taking a long time, thus blocking this timer ISR call. By the time this ISR is executed another timer interrupt had occurred. To solve, insure all ISRs together do not take longer than the timer period.

With good design then a 2nd timer interrupt will not occur and temporarily disabling the time to do the multiplication will not block a timer interrupt occurrence.


Or simplify code

aDfilter * 8 needing a @MUL3232 call implies weak optimizations or unnecessary use of signed math. Make code easier for the compiler by using unsigned math.

// some_signed_32_bit_type aDfilter
uint32_t aDfilter;

// and if that is not enough change code
// aDfilter = aDfilter * 8 + aD * 2;
aDfilter = (aDfilter << 3) + aD * 2;

Potential performance improvement. Depending on overall design, the delay_us(40); should be able to be eliminated. If only 1 channel is used, the following is possible - depending on various requirements.

#int_RTCC
void  RTCC_isr(void) {
  // delay_us(40);  // not needed is timer period >> 40us
  unsigned int16 aD = read_adc();
  set_adc_channel(0);    // select channel now  (also at initialization)
  aDfilter = aDfilter * 8 + aD * 2;
}

Potential bug. If the read_adc() is in a mode such that the most significant digit is set (e.g. 10 bit conversion shifted 6 left) and the compiler has a 16-bit int/unsigned, then aD*2 overflows. In that case use:

uint32_t aDfilter;
...
aDfilter = aDfilter*8 + (uint32_t)read_adc()*2;
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256