Skip to content
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

Support extra parameter on attachInterrupt() #58

Closed
wants to merge 1 commit into from

Conversation

paulo-raca
Copy link
Contributor

@paulo-raca paulo-raca commented Jan 4, 2019

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 was originally developed on arduino/Arduino#4519, and it's contains lengthy discussions about the implementation strategies and overhead.

A similar PR is available for SAM devices: arduino/ArduinoCore-sam#44

Overhead summary:

  • CPU: it needs 2 extra instructions (4 cycles) before the user funcion is called.
  • RAM: intFuncParam takes 2*EXTERNAL_NUM_INTERRUPTS extra bytes (10 bytes of RAM on Arduino Pro Micro)
  • Flash: Adds 56 bytes of Flash on Arduino Pro Micro

I do believe these are very acceptable values for a very useful feature.

@PaulStoffregen
Copy link
Contributor

STM32 and ESP8266 are using C++ std::function to accomplish this. I am planning to do the same on Teensy in the near future.

stm32duino/Arduino_Core_STM32#159

https://github.com/esp8266/Arduino/blob/master/cores/esp8266/FunctionalInterrupt.cpp

@paulo-raca
Copy link
Contributor Author

paulo-raca commented Jan 4, 2019

Nice!
std::function is definitely better, unfortunately it doesn't exist on AVR, so I cannot reuse it. :(
(I'm not sure about Due...)

If attachInterruptParam gets added, we can add a simple shim on these platforms for API consistency, something like this:

inline void attachInterruptParam(uint8_t pin, void (*)(void*) callback, int mode, void* param) {
    attachInterrupt(pin, [callback, param]() { callback(param); }, mode);
}

(I haven't really tested it)

@PaulStoffregen
Copy link
Contributor

There's been talk of upgrading to gcc 7. Any idea if newer AVR toolchains will someday support std::function?

It'd really be a shame to do this another way if AVR will in the future have std::function...

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.
@paulo-raca
Copy link
Contributor Author

I've just made a minor updated to this PR:

@Harvie
Copy link

Harvie commented Jul 27, 2019

Any news on this one?

@facchinm
Copy link
Member

Hi @Harvie ,
a slightly different version of this PR (with the same capabilities indeed) was merged in API core (https://github.com/arduino/ArduinoCore-API/blob/master/api/Interrupts.h#L19).
For the feature to rollout on every core we need to find the time for moving to API (here's the relevant branch https://github.com/arduino/ArduinoCore-avr/tree/namespaced_api)

@Harvie
Copy link

Harvie commented Jul 29, 2019

@facchinm cool! can't wait for this to be available on Arduino Nano/Uno.

@matthijskooijman
Copy link
Collaborator

matthijskooijman commented Sep 23, 2019

I think this PR can be closed, since this will be migrated from the ArduinoCore-API repo rather than implemented separately here in any case. In the meanwhile, we have #85 and arduino/ArduinoCore-API#95 to track this issue.

@facchinm, @paulo-raca, agreed?

@matthijskooijman
Copy link
Collaborator

I think this PR can be closed, since this will be migrated from the ArduinoCore-API repo rather than implemented separately here in any case. In the meanwhile, we have #85 and arduino/ArduinoCore-API#95 to track this issue.

Seems this is not really true: ArduinoCore-API has some code, but that is only for converting a reference argument to a pointer argument. And it specifies an attachInterruptParam function, but of course does not provide any implementation for it, which could be taken from this PR.

For additional thoughts on the API and implementation, see arduino/ArduinoCore-API#99

@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
Contributor Author

It's been 5 years since I first sent this PR 😳

Time to give up

@paulo-raca paulo-raca closed this Apr 9, 2021
@huster-songtao
Copy link

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.

7 participants