arduino / arduino/ArduinoCore-avr
Add a version of attachInterrupt that more efficiently dispatches both rising and falling ISRs on the same GPIO pin
- Dominant language
- C
- Stars
- 1.5k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
At the moment the Arduino platform only allows the user to attach one interrupt type per pin, because of underlying limitations of interrupt configuration on GPIO peripherals. If a user wishes to receive an interrupt on both rising and falling edges, while also receiving information about which event caused the interrupt, they are unable to do so directly. Instead, they usually end up implementing something like this:
```
const int InterruptPin = 5;
void setup()
{
pinMode(InterruptPin, INPUT);
attachInterrupt(digitalPinToInterrupt(InterruptPin), pin_isr_dispatch, CHANGE);
}
void pin_isr_rising()
{
/* ... */
}
void pin_isr_falling()
{
/* ... */
}
void pin_isr_dispatch()
{
if (digitalRead(InterruptPin) == HIGH)
pin_isr_rising();
else
pin_isr_falling();
}
```
There are a few downsides to this approach. The first is that the Arduino core offers no standardised API call for hardware that can support separate ISRs for rising and falling on the same pin, and the second is that the `digitalRead` has unnecessary overheads for this use-case.
I propose an API that looks something like the following:
```
void attachInterrupt(uint8_t digitalPin, void (*risingUserFunc)(void), void (*fallingUserFunc)(void));
```
I am, of course, open to suggestions as to alternative function definitions.
For the AVR core, where the hardware doesn't natively support paired ISRs like this, the API would take the digital pin number from the first parameter and use it to find the interrupt number, input port number, and input port bit offset (as per `digitalRead`) ahead of time. The `userFunc` array used by the existing `attachInterrupt` implementation would probably be replaced with a struct array where each element contains a pair of function pointers and the associated port number & bit offset values. The actual ISR itself would be pointed to a dispatch function that pulls the pin information from the array and calls `portInputRegister` (or similar) to read its value and call the correct user function depending on the pin value.
This approach offers the following performance increases on every interrupt:
* Removes a call to `digitalPinToTimer`, along with the subsequent `NOT_ON_TIMER` check plus potential `turnOffPWM` call.
* Removes a call to `digitalPinToPort`, along with the subsequent `NOT_A_PIN` check.
* Removes a call to `digitalPinToBitMask`.
* Replaces three calls (user's dispatch function, `digitalRead`, and the user's rising/falling function) with two calls (internal dispatch function, user's rising/falling function).
The tradeoff is data size, where each 16-bit user function pointer would be replaced with a 48-bit struct (two 16-bit pointers plus a pair of uint8_t values for the port and bit offset respectively). Considering that the maximum number of interrupts supported is 8, this increase does not seem excessive.
This API could also be reused for other cores where the underlying hardware has better support for this use-case. For example, if the underlying hardware supports directly assigning separate rising/falling ISRs to a single pin, it would facilitate that. On hardware that does not offer separate ISRs, but does provide a signal as to whether a `CHANGE` interrupt was triggered by a rising or falling signal, we could use that to perform the dispatch instead of needing to manually read the pin state.
While I ordinarily might have put this together as a PR rather than an issue, in this case I'm unfamiliar with both the Arduino project's policies around API changes and the minutiae of GPIO configuration registers on AVR platforms, so I thought it best to start here first. If there is support for this feature, and no insurmountable objections are raised, then I am willing to make an attempt at implementing this myself, provided that if no-one more qualified and familiar with the project wishes to do so themselves.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by locating the existing attachInterrupt implementation, its ISR dispatch path, and the digitalRead and portInputRegister code used by the AVR core. Compare the proposed paired rising/falling callbacks with the current API and hardware constraints; done means an agreed API and implementation approach that preserves existing behavior while dispatching both edge types efficiently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- embedded-iot
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100