Interrupt Queueing

[NuclearPhoenixx]

Hi, I'm using attachInterrupt() on a standard Pi Pico and it seems like interrupts can still be triggered when the processor itself is currently in the ISR. It sort of queues all the interrupts that occur and executes them one by one after another, at least it looks like it by what I can tell. I am doing an ADC measurement in the ISR which when queued doesn't make sense anymore. (i.e. it reads the wrong values, because it's too late so to speak)

I am sure there is a way to disable such a behavior, right? I already tried wrapping my ISR in noInterrupts() and interrupts() as well as detachInterrupt() and attachInterrupt(), but it doesn't solve the issue.

[maxgerhardt]

Can you give a minimal example of what's going and what's expected? A button ISR triggers an ADC reading, but if during reading the interrupt is triggered another time it pre-empts the running execution or still queues the next ISR call?

Disabling interrupts should avoid pre-emption / nesting per https://stackoverflow.com/a/45807834.

[NuclearPhoenixx]

You can have a look at my code here: https://github.com/OpenGammaProject/Open-Gamma-Detector/blob/main/software/opengamma_pico/opengamma_pico.ino

I declare attachInterrupt() in L720 and the eventInt function used for the ISR can be found in L595.

What's happening inside eventInt doesn't matter that much aside from the fact that the ADC is used and I think you can also ignore all the other functions used in the loops or for Serial handling. In this sketch there are no noInterrupts() or detachInterrupt() calls yet, but I tested both of them at the beginning and end of the eventInt function like I described above.

What happens here is that a comparator triggered by gamma radiation (randomly) goes HIGH on a digital pin for something like 1 µs and then goes back to LOW. However, once I'm inside the function, it's possible that another gamma pulse comes in and triggers that ISR too, queueing it, because the function takes at least ~20 µs to execute. Problem is, once the first function call concludes, there is nothing to measure anymore because the signal that I wanted is long gone by now. The software doesn't know this of course and keeps on measuring which by this point is noise only. I obviously don't want this behavior, so I'd prefer that there cannot be any new interrupts when I'm currently inside the eventInt function.

Hope I explained that well enough.

[maxgerhardt]

I see what you mean. Weird that it didn't stop generating interrupts although you called detachInterrupt(). The only thing I can think of is that the function call wasn't fast enough and didn't take effect before yet another IRQ event was generated. The most direct way of turning off interrupt generation would be to write into the GPIO or IRQ peripheral registers. Like e.g. here, but in "expanded / direct form" where it's just the write to the register with a direct pointer. I'll have to take a look at the datasheet, but basically you'll want to write the EN (enable) register of the IRQ peipheral for your specific GPIO pin or bank.

[maxgerhardt]

Could you add this at the very top of your interrupt routine

  // disable interrupt generation for this pin ASAP
  // directly uses Core0 IRQ Ctrl (core1 does not set the interrupt)
  hw_clear_bits(
    &(iobank0_hw->proc0_irq_ctrl.inte[INT_PIN / 8]), 0b1111 << (INT_PIN % 8 )*4u
  );

and at the very bottom

  // reenable now
  hw_set_bits(
    &(iobank0_hw->proc0_irq_ctrl.inte[INT_PIN / 8]), 0b0100 << (INT_PIN % 8 )*4u
  );

and retest.

[NuclearPhoenixx]

Thanks a lot for your time! I just checked it and it definitely seems to help a bit, although, it still doesn't seem to get rid of the effect (I can go to about 550 ints/sec instead of 500). If I change the number of ADC measurements to take inside the function, it still changes a lot. (L605). If I do 5 vs 1 analogRead(), for example, I can go from 550 ints/sec to about 2k ints/sec without much of an issue, so something's clearly still happening here.

Now, I could do only one, of course, and it also helps a lot to overclock the Pico, but now I'm very much interested what's really going on here 🤔

[maxgerhardt]

Hm. I guess there's more code before the user-specified ISR callback function is executed.

A totally different approach would be to use FreeRTOS. The ISR function would then only enqueue a "Measurement Request" that has a timestamp in it, into a queue (and abort if the queue is already full). A FreeRTOS task waits on an element of the queue (and is woken up pretty much immediately when something is put in the queue). The FreeRTOS task can then inspect the given timestamp (e.g. produced micros() or some other high resolution timer) and decide whether or not to process the request.

[PontusO]

Overclocking will do nothing for you. The SDK function adc_read() will wait for each analog sample until it is available. Also not really something you want to do in interrupt context. That's why see such a dramatic difference when changing the number of samples you take.

…

On April 25, 2023 13:03:36 NuclearPhoenix ***@***.***> wrote: Thanks a lot for your time! I just checked it and it definitely seems to help a bit, although, it still doesn't seem to get rid of the effect (I can go to about 550 ints/sec instead of 500). If I change the number of ADC measurements to take inside the function, it still changes a lot. (L605). If I do 5 vs 1 analogRead(), for example, I can go from 550 ints/sec to about 2k ints/sec without much of an issue, so something's clearly still happening here. Now, I could do only one, of course, and it also helps a lot to overclock the Pico, but now I'm very much interested what's really going on here 🤔 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[NuclearPhoenixx]

The SDK function adc_read() will wait for each analog sample until it is available. Also not really something you want to do in interrupt context. That's why see such a dramatic difference when changing the number of samples you take.

Well yes, obviously. But that's not what this question is about.

[Andy2No]

@Phoenix1747 There are no doubt better ways to use the ADC, by going a bit more direct to the hardware, but I think you would still run into the problem of the fundamental bandwidth of the internal ADC being too low - if you needed a sample within, say, 1 microsecond of an event / detection, you would need at least a 1MHz bandwidth ADC, or more realistically, a faster one than that.

I've just had an idea, which might work better. You can make a simple sample and hold circuit with an analog switch, a suitable capacitor and a FET input op amp. Your ISR could simply grab and hold the sample, then set a flag to say to read it. The held value will decay, but potentially quite slowly. If you can read it with the ADC within a reliable interval, that may be good enough, or perhaps you could also use a timer to measure how long it's been since the flag was set by the ISR - make the ISR store the time in clock cycles, for example. You can then use a lookup table or just an exponential floating point calculation, to compensate for the time taken, to give you a more accurate reading.

Coping with more arriving before the ADC reading can be taken, could be done by just adding more sample and hold circuits, e.g. a quad op amp chip could provide two or maybe four, with a four channel analog switch. Since there will definitely be a limit, set by your hardware design, to how many events you can handle in a few microseconds, the code doesn't need to be very complicated - just an ISR that can set flags allocated from a small pool, held in an array, and some code in a loop that polls for the flags being set and acts on them, freeing up a flag for re-use as it deals with one. The ISR can just read through the array of flags to find a free one, since there can only be a very limited number of them - there's no need for anything more complicated like lists or dynamic allocation.

Also, the ISR code linked to, used a calculation as part of disabling the interrupt for the pin, but that just produces a constant value from another constant. Maybe you could be passing the pre-calculated values instead, or have the ISR pull them out of an array, if it can only be passed a pin number. I always wondered with things like Z80 ISRs if it was possible for another interrupt to happen before the single instruction that was used to disable them. In this case, there are quite a lot of clock cycles where the ISR is calculating values to use to disable them, which seems like asking for trouble.

[NuclearPhoenixx]

Oh no, the ADC is not really the problem here. I am already using a sample and hold circuit and it works great. The issue here is that it's possible that there are multiple other interrupts while one is already going. I could move the ADC stuff out of the interrupt and into the loop, however, I don't know if it would be stable this way with getting interrupted all the time. On top of that, the ISR still takes a good 10 µs to conclude, so the issue still persists, even if it's better than before. It would be great if I could just turn off this queueing behavior alltogether, I'd be more than happy with that. But I guess, there is not a significantly better (faster) way to do so than what @maxgerhardt suggested.

Just adding more sample and hold circuits is not an option at the moment, unfortunately. I can only buffer one analog signal at a time for now. And, like I said, it wouldn't be a problem. I'm okay with losing some of the signal alltogether (by intermediately disabling interrupts) if that means I can avoid the other issues.

Also, the ISR code linked to, used a calculation as part of disabling the interrupt for the pin, but that just produces a constant value from another constant.

Not entirely sure what you mean by that, can you please elaborate?

[Andy2No]

@Phoenix1747 It needs to get to the point where it's setting control bits as soon as possible after the start of the ISR, to reduce the chances of another interrupt happening first. I'm guessing that for any given pin number, there is only one possible value for &(iobank0_hw->proc0_irq_ctrl.inte[INT_PIN / 8]), 0b1111 << (INT_PIN % 8 )*4u, so those values could presumably be calculated in advance, e.g. in setup(), then stored in an array. If so, retrieving them from the array might be quicker, even though that's a simple looking calculation. The fewer clock cycles you can do it in, the better.

[NuclearPhoenixx]

Wouldn't the compiler optimize something like this away? At least the 0b1111 << (INT_PIN % 8 )*4u part is constants only, I'd think it would know this, but I'm probably wrong if you say it like that.

[earlephilhower]

Is there a more robust way of handling this?

I may not have followed the whole issue in depth, but it seems like you're fighting a race condition. Is there some other, non timing easy of getting around this? I fear you're exchanging and easily debugged common failure with a failure that will hit once in a blue moon and be impossible to reproduce it fix.

Just brainstorming, but can you ignore any interrupts that happen too frequently(use the core cycle count) or too close to the exit time of a prior interrupt? Would it be possible to use level triggers instead of edge?

[NuclearPhoenixx]

Just brainstorming, but can you ignore any interrupts that happen too frequently(use the core cycle count) or too close to the exit time of a prior interrupt?

Yes, I could implement a check at the start of ISR that just compares the current core cycle count to the exit time of the prior interrupt. That's actually not a bad idea and should be reasonably fast to execute, I think. I'll check that out.

[earlephilhower]

Another idea, have you tried explicitly clearing the GPIO IRQ at the end of your ISR, right before returning? That would throw out anything that happened during your ISR cleanly w/o you keeping track of anything.

There should be an NVIC function and maybe an Pico-SDK wrapper call you could use. One note, though, is that there is only one IRQ for all GPIO pins, so if you have other GPIO-based IRQs you'll lose those, too.

[NuclearPhoenixx]

Oh, that sounds great! I haven't looked at that, but that's pretty much a perfect solution for my issue here. I only have this one interrupt on my device, so that wouldn't be a problem.

[NuclearPhoenixx]

I found this function in the pico SDK: static void irq_clear (uint int_num). I think that's what I need to get the job done.

https://www.raspberrypi.com/documentation/pico-sdk/hardware.html#ga14fa98b8bec09df3e7409a75cf2f5359

But how do I find out the correct interrupt number for my digital pin interrupt?

[earlephilhower]

https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf

Section 2.3.2, it's the SIO_IRQ_CORE0 you'll be wanting to clear, I think

[Andy2No]

@Phoenix1747 I haven't looked at the code, but if INT_PIN is a constant, isn't &(iobank0_hw->proc0_irq_ctrl.inte[INT_PIN / 8]), 0b1111 << (INT_PIN % 8 )*4u the same every time? If so, just evaluate it once, during setup() and store it in a global or static variable.

But, yes, avoiding the possibility of another interrupt happening before interrupts are disabled, would be much better.

A simple SR latch, made from two NAND or NOR gates could do it - the detection circuit would set the latch, then the ISR would clear it as the last step before it exits. There would still be the small possibility of another interrupt before the ISR actually does return, so ideally, it should have a monostable circuit to add a small delay to the reset - two more gates and a capacitor + resistor, so a quad NAND or NOR gate chip would do it.

Maybe the PIO could be programmed to simulate that sort of circuit? You'd need a state variable that can be set by the external detector circuit, then a delayed reset that can be initiated by the CPU, counting enough clock cycles to allow for the ISR to finish up.

If it doesn't need to respond immediately, and in a set amount of time, maybe it would be better to just not use interrupts at all, and poll the detector in a tight loop. You could potentially have one core doing other things, provided you can prioritise the one doing the polling.

[NuclearPhoenixx]

Yup, INT_PIN is a const. I'll try your suggestion of evaluating it once and storing it in a static variable.

Good idea with the SR latch, I'll think about that.

[Andy2No]

@Phoenix1747 Here, there's an example of a simple logic circuit which detects pulses with a lock out period (retriggerable one-shot monostable multivibrator). More pulses arriving within the lockout period get ignored:

https://www.allaboutcircuits.com/textbook/digital/chpt-10/monostable-multivibrators/

The sort of hardware that's needed just takes one 74HC series 14 pin chip (e.g. 74HC00 quad NAND gate or 74HC02 quad NOR gate), or similar, and a couple of discrete components. It can be built on the smallest sized breadboard, e.g. https://www.pololu.com/product/1490 (47 × 35 mm (1.84 × 1.37") and there are prototyping PCBs you can buy very cheaply, to make a soldered version. Add a 0.1uF (100nF) capacitor between the Vcc and GND pins, as close as possible to the Vcc pin, to decouple (smooth) the power supply to avoid glitches.

You'd just need to place that circuit between your detector and the CPU pin.

[MIKHANYA]

I don't get it. What is the actual answer ? No example code included. How should the code look when I want to disable the interrupt and assign for example a new.
What are some ways to use interrupts thanks to your kernel if the detachInterrupt() after attachInterrupt() don’t work?
Here is my example code that I need to work on RP2040. Pls help me to make it work on it.
My code quick overview, 2 buttons using interrupts. I want to disable the interrupt and change it to another one in order to count how long the button is held down.

#include <Arduino.h>
volatile unsigned long button_time = 0;
volatile unsigned long last_button_time = 0;
volatile unsigned long rise_button_time = 0;
struct Button
{
  const uint8_t Pin;
  uint32_t numberKeyPresses;
  bool pressed;
  bool UNpressed;
};
Button button6 = {6, 0, false};
Button button7 = {7, 0, false};

void isr61() //// RISING interrupt occured, Button unpressed
{
  rise_button_time = millis();
  button6.UNpressed = true; // going to enable falling interrupt
}

void isr6() //// FALLING interrupt occured, pressed
{
  button_time = millis();

  if (button_time - last_button_time > 250)
  {
    attachInterrupt(button6.Pin, isr61, RISING); // enable rising interrupt
    button6.numberKeyPresses++;
    button6.pressed = true;
    last_button_time = button_time;
  }
}

void isr7() //// FALLING interrupt occured, Button pressed
{
  button7.numberKeyPresses++;
  button7.pressed = true;
}
void setup(void)
{
  Serial.begin(115200);
  pinMode(6, INPUT);
  pinMode(7, INPUT);
  attachInterrupt(6, isr6, FALLING);
  attachInterrupt(7, isr7, FALLING);
  delay(1000);
}
void loop(void)
{
  if (button6.pressed)
  {
    Serial.print(button6.numberKeyPresses);
    Serial.println(" FALL #6");
    button6.pressed = false;
  }
  if (button6.UNpressed)
  {
    attachInterrupt(button6.Pin, isr6, FALLING); // enable falling interrupt
    Serial.println("RISE #6");
    button6.UNpressed = false;
    if (rise_button_time - last_button_time > 500)
    {
      Serial.println("Long Press Detected");
    }
  }
  if (button7.pressed)
  {
    Serial.print(button7.numberKeyPresses);
    Serial.println(" FALL #7");
    button7.pressed = false;
  }
}

[MIKHANYA]

@NuclearPhoenixx Thanks for the answer. Maybe I'm doing something wrong, I compile the code in PlatformIO and have error.
'iobank0_hw' was not declared in this scope; did you mean 'padsbank0_hw'?
I put those lines of code inside my isr function. And predefined const int INT_PIN = 15;

[NuclearPhoenixx]

Hm, I'm using the Arduino IDE. No idea.

[MIKHANYA]

@maxgerhardt I'm sorry to bother you, but could you help me? I know you are familiar with Raspberry Pi PlatformIO. Could you help to solve the problem with compiling the code? Or could you share your experience using interrupts and disabling them using PlatformIO?

[maxgerhardt]

If it's just about measuring the time a button was pressed, I think that's already a solved problem with the OneButton library. It has a function available on how long it was pressed. https://github.com/mathertel/OneButton/blob/master/src/OneButton.h#L246-L250. It should work on the Pico, too.

[MIKHANYA]

@maxgerhardt Thanks for the answer. Yes to read button state. Pressed, held, and released. But for a joystick, there are actually 5 buttons. And I wanted to use 5 interrupts, is it possible ? If I understand correctly, the OneButton library does not work with interrupts? What seems to be ineffective in using processor resources?