Skip to content

Support extra parameter on `attachInterrupt() #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

paulo-raca
Copy link

When writing an arduino library, it is difficult to connect interrupt handlers with the component instance that should be notified.

In C, the common pattern to fix this is to specifying a void* parameter with the callback, where the listener can store any context necessary.

This patch adds the new function attachInterruptParam() to implement this pattern.

This PR mirrors the changes for AVR on arduino/Arduino#4519.

In C, the common pattern to fix this is to specifying a void* parameter with the callback, where the listener can store any context necessary.

This patch adds the new function attachInterruptParam() to implement this pattern.

This is a port of arduino/Arduino#4519, but for Arduino Due.
@paulo-raca
Copy link
Author

paulo-raca commented Sep 1, 2017

Just like the AVR implementation, on ARM the callback argument is sent on a register (r0).

This means that we can safely pass arguments to a callback that doesn't expect any, and it will silently ignore them, allowing us to be backwards-compatible with no-argument interrupt callbacks with minimal overhead.

How much overheader? Only 2 instructions.

Let's look at the assembly of the assemly of ISR handler:

Previous:

   [...]
    if(callbacksPioA[pin]) callbacksPioA[pin]();
   80934:       b2dd            uxtb    r5, r3 // r5=pin
   80936:       4b07            ldr     r3, [pc, #28]   ; (80954 <PIOA_Handler+0x34>) 
   80938:       f853 3025       ldr.w   r3, [r3, r5, lsl #2]
   8093c:       b103            cbz     r3, 80940 <PIOA_Handler+0x20>
   8093e:       4798            blx     r3
   [...]

New:

   [...]
    if(callbacksPioA[pin]) callbacksPioA[pin](callbackParamsPioA[pin]);
   80968:       4b08            ldr     r3, [pc, #32]   ; (8098c <PIOA_Handler+0x38>)
   8096a:       b2ed            uxtb    r5, r5
   8096c:       f853 3025       ldr.w   r3, [r3, r5, lsl #2]
   80970:       b11b            cbz     r3, 8097a <PIOA_Handler+0x26>
   80972:       4a07            ldr     r2, [pc, #28]   ; (80990 <PIOA_Handler+0x3c>)
   80974:       f852 0025       ldr.w   r0, [r2, r5, lsl #2]
   80978:       4798            blx     r3
   [...]

@paulo-raca
Copy link
Author

To be honest, I don't think it ever will.

The original PR on the AVR port has been pending for over 2 years without merge (despite apparent consensus that it would be useful and a pretty low performance overhead).

If you know any way of getting their attention, please do / let me know.

@facchinm
Copy link
Member

facchinm commented Mar 5, 2018

@paulo-raca we are definitely going to merge it (maybe slightly modified) but it's being delayed since it's part of Chainsaw project to unify APIs between platforms. I truly hope we'll be able to ship it before next month but we are severely outnumbered... Sorry again for the delay

@paulo-raca
Copy link
Author

@facchinm thanks for the reply, I'm glad to hear it.

Is there any way I can help?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@paulo-raca
Copy link
Author

Closing by lack of interest

@paulo-raca paulo-raca closed this Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants