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

Functional Interrupts #2745

Merged
merged 2 commits into from
Aug 27, 2017
Merged

Functional Interrupts #2745

merged 2 commits into from
Aug 27, 2017

Conversation

hreintke
Copy link
Contributor

@hreintke hreintke commented Dec 8, 2016

Imlementation of the possibility to use std::functions for Interrupt callback.

Example usage :

#include <FunctionalInterrupt.h>

class InterruptClass
{
public:
	InterruptClass(){};
	void classFunctionInterrupt(void)
	{
		Serial.printf("Class function interrupt\r\n");
	}
};

bool blinker;
void blink(int blinkPin)
{
	if (blinker)
	{
		digitalWrite(blinkPin,HIGH);
	}
	else
	{
		digitalWrite(blinkPin, LOW);

	}
	blinker = !blinker;
	Serial.printf("Blink on pin %d, value %d\r\n", blinkPin,blinker);
}

void globalInterrupt()
{
	Serial.printf("Global function interrupt\r\n");
}

InterruptClass demoClass;

void setup() {

	Serial.begin(74880);

	pinMode(D1, INPUT);
	pinMode(D2, INPUT);
	pinMode(D5, INPUT);
	pinMode(D4, OUTPUT);

	attachInterrupt(D1, std::bind(blink,D4), CHANGE);

	attachInterrupt(D2, globalInterrupt,CHANGE);

	attachInterrupt(D5, std::bind(&InterruptClass::classFunctionInterrupt,&demoClass), CHANGE );
}


void loop() {

}

@codecov-io
Copy link

codecov-io commented Dec 8, 2016

Codecov Report

❗ No coverage uploaded for pull request base (master@3f5d06b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f5d06b...d498ea2. Read the comment docs.

@igrr
Copy link
Member

igrr commented Jan 6, 2017

This looks pretty good!
I am pondering if we can somehow remove the need for the #include <FunctionalInterrupt.h>... Maybe this can be added to wiring.h under a #ifdef __cplusplus conditional? Would that break anything?

@hreintke
Copy link
Contributor Author

Hi,
I tried but with no positive result.
Have not much experience with c/c++ combinations and the low-level setup of the library.
Will need some help to get an alternative solution.

@devyte
Copy link
Collaborator

devyte commented Aug 27, 2017

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?

@igrr igrr merged commit ba0e049 into esp8266:master Aug 27, 2017
@philbowles
Copy link

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!

@devyte
Copy link
Collaborator

devyte commented Sep 1, 2017 via email

std::function<void(void)> reqFunction;
};

void interruptFunctional(void* arg)
Copy link
Collaborator

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?

Copy link
Member

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)
Copy link
Collaborator

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)> ?

Copy link
Member

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.

@hostmit
Copy link

hostmit commented Aug 31, 2018

Can this be impletemented for ESP32?

@hreintke
Copy link
Contributor Author

hreintke commented Sep 1, 2018

@hostmit See espressif/arduino-esp32#1728

@nixmeer
Copy link

nixmeer commented Oct 10, 2018

just for the record:
#include <FunctionalInterrupt.h> is still necessary

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