-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Functional Interrupts #2745
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
Functional Interrupts #2745
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2745 +/- ##
========================================
Coverage ? 27.8%
========================================
Files ? 20
Lines ? 3625
Branches ? 656
========================================
Hits ? 1008
Misses ? 2441
Partials ? 176 Continue to review full report at Codecov.
|
This looks pretty good! |
Hi, |
Hello, I find this to be far too useful to not merge asap, given that it basically allows passing a context to an ISR. @igrr Is it possible to merge as-is (taking into account a branch update, of course), and open a new issue to cover removing the need of the header include? |
I LOVE you guys! This is exactly the code I was looking into writing for my current project...and so of course I agree with devyte that it should be in as soon as possible. I have already done something similar by extending the Ticker class, allowing functional calls on timers. My overall project attempts to serialise all external events into a central task queue to avoid WDTs, resource clashes etc. It's pretty much done and dusted and working apart from the final piece in the jigsaw...which is THIS! You have saved me weeks of hair-pulling! I now just need to get this into my own dev code and have a single interrupt global handler with pin Arg which pushes the appropriate function into the task queue...job done! I'm a bit of a newbie with github and "branches" and "pulls" so I'd appreciate some advice please, on how to get the latest version into my arduino ide without breaking anything! |
Read using git version of the core in readthedocs. You have to delete (or
move out) the ide version of the core to make sure it's picked up.
I'm already using this, and it works very well, so thank you @igrr for
merging so quickly.
…On Sep 1, 2017 6:02 AM, "Phil Bowles" ***@***.***> wrote:
I LOVE you guys! This is *exactly* the code I was looking into writing
for my current project...and so of course I agree with devyte that it
should be in as soon as possible. I have already done something similar by
extending the Ticker class, allowing functional calls on timers. My overall
project attempts to serialise all external events into a central task queue
to avoid WDTs, resource clashes etc. It's pretty much done and dusted and
working apart from the final piece in the jigsaw...which is THIS!
You have saved me weeks of hair-pulling! I now just need to get this into
my own dev code and have a single interrupt global handler with pin Arg
which pushes the appropriate function into the task queue...job done!
I'm a bit of a newbie with github and "branches" and "pulls" so I'd
appreciate some advice please, on how to get the latest version into my
arduino ide without breaking anything!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2745 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AQC6BnpCXtj1718mvfyTcw5bkXTkgR7lks5sd8gRgaJpZM4LH-rF>
.
|
std::function<void(void)> reqFunction; | ||
}; | ||
|
||
void interruptFunctional(void* arg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@igrr it's a bit late, but shouldn't this have the ICACHE_RAM_ATTR tag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right, under the current model it should have ICACHE_RAM_ATTR.
((ArgStructure*)arg)->reqFunction(); | ||
} | ||
|
||
void attachInterrupt(uint8_t pin, std::function<void(void)> intRoutine, int mode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@igrr what about here? should this be std::function<void ICACHE_RAM_ATTR (void)> ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more tricky. You can't add attributes to function prototypes like that, and furthermore the invoke method of std::function (which gets called when you use operator() on the function object) is not placed into IRAM either (it's a part of libstdc++).
I think the only way to work around this issue is to implement interrupt masking for the duration of SPI flash operation (#3579), then IRAM_ATTRs will not be needed at all.
Can this be impletemented for ESP32? |
just for the record: |
Imlementation of the possibility to use std::functions for Interrupt callback.
Example usage :