ESP8266 crashes with receiver enabled

[RalfJL]

Hi,
I am building a 433 MHz receiver around a Adafruit HUZZAH ESP8266.
The receiver is going to switch two different light channels. It should receive "toggle" commands from some wall switches (using battery powered Attiny85 and 443 MHz sender) and it is connected via WIFI to a MQTT bus to send and receive commands for home automation (currently openHAB).
This way not only the two light channels can be switch via MQTT but any 433Mhz receiver in my house using openHAB.

Though the standalone receive demo from RCSwitch works flawlessly, I do get crashes, as soon as I integrate the RCSwitch library into my Sketch. The sketch is mainly connecting via WiFI to a MQTT bus to send and receive messages or commands.
The crashes always happen in the Interrupt service routine (ISR) or any function called by the ISR in the receive code of RCSwitch.

After reading a lot I found that it is absolutely necessary to have all ISR functions and all functions called from the ISR in RAM on the ESP8266, not in PROGMEM.
Currently the following modifications to RCSwitch.cpp helped in getting the crashes go away.
Please consider the changes to be integrated into the release.
Anybody also trying to do this on a busy ESP8266 (WiFi) should also note that you need the ESP8266 core >= 2.2.0. Starting with this release the micro() function is also in RAM. If it is not, you will get crashes in that function.

Changes I did:
Have the protocols in RAM
#ifdef ESP8266
// because of timing reasons we keep that in memory on esp8266
static const RCSwitch::Protocol proto[] = {
#else
static const RCSwitch::Protocol PROGMEM proto[] = {
#endif

Changes to receiveProtocol (called by ISR)
#ifdef ESP8266
bool ICACHE_RAM_ATTR RCSwitch::receiveProtocol(const int p, unsigned int changeCount) {
#else
bool RCSwitch::receiveProtocol(const int p, unsigned int changeCount) {
#endif

Protocol pro;
 #ifdef ESP8266
pro = proto[p-1];
#else
    memcpy_P(&pro, &proto[p-1], sizeof(Protocol));
#endif

Changes to ISR:

ifdef ESP8266

void ICACHE_RAM_ATTR RCSwitch::handleInterrupt() {

else

void RCSwitch::handleInterrupt() {

endif

Please let me know if someone has a better idea.

Thanks

Ralf

[WoodsterDK]

Nice find.... I have been looking a lot for this.
Will try this later today, to see if it solves the problem.

[WoodsterDK]

Seems to solve my instability issue.

[fingolfin]

I tried to fix this in 90d00c1 based on your description. Could you please verify whether that actually works for you?

[RalfJL]

Yes,

thanks, works
I like the implementation.

[WheresWally]

I had to do this to the .cpp file to get it to work on a ESP8266-01 with the data connected to GPIO-02

#ifdef ESP8266
void ICACHE_RAM_ATTR RCSwitch::handleInterrupt() {
#else
//void RCSwitch::handleInterrupt() {
void RECEIVE_ATTR RCSwitch::handleInterrupt() {
#endif

Otherwise, thanks heaps.

I am making a range extender for a CareAlert smart dialer using 2x EPS8266 with a 434Mhz Receicer on one unit and a 434Mhz transmitter on the other.
Which now works....

[eiannone]

@WheresWally , this shouldn't be necessary, as you can see in the macro starting at line 43, where RECEIVE_ATTR is already substituted with ICACHE_RAM_ATTR for ESP8266:

#ifdef ESP8266
    // interrupt handler and related code must be in RAM on ESP8266,
    // according to issue #46.
    #define RECEIVE_ATTR ICACHE_RAM_ATTR
#else
    #define RECEIVE_ATTR
#endif

[gadget1999]

I also have similar issue on ESP8266 that random crash happens when interrupt is called. I also added ICACHE_RAM_ATTR to RCSwitch::handleInterrupt() and now problem is solved.

[fingolfin]

@gadget1999 Well, we already have a fix for that in the code -- are you using the latest version of rc-switch? If so, can you please test whether ESP8266 is #defined for you? E.g. put this into your code, and compile it:

#ifdef ESP8266
#error  ESP8266 is defined
#else
#error ESP8266 is NOT defined
#endif

If the issue persists with the latest rc-switch, as taken from this repository, please open a new issue for this, instead of adding to an old, already closed one.

[gadget1999]

@fingolfin , thanks for the direction, I can reproduce the issue with "#error ESP8266 is defined". I'll open a new issue then.

[PTDreamer]

@gadget1999 Well, we already have a fix for that in the code -- are you using the latest version of rc-switch?

The fix is not on the latest release (v2.52) which is what is used with for example platformio library manager.
Could you please pick a branch/revision with code you consider stable and hit the create release button :)

[gadget1999]

sorry never used github to commit code before, I'll need to learn how to do it first and make the change later. All the change I made locally was the same as @WheresWally did above:

#ifdef ESP8266
void ICACHE_RAM_ATTR RCSwitch::handleInterrupt() {
#else
//void RCSwitch::handleInterrupt() {
void RECEIVE_ATTR RCSwitch::handleInterrupt() {
#endif

[benthans1]

Here is the actual bug:
/* helper function for the receiveProtocol method */
static inline unsigned int diff(int A, int B) {
//return abs(A - B); // it's not a language build in function and not declared with RECEIVE_ATTR
//replacement:
if ((A - B)>0) return (A - B); else return -(A - B) ;
}

[benthans1]

Someone should fix this bug in the master lib and release a new version. I'm not currently willing to be in charge of such a release myself. If you are the right person to do it, please go ahead, but don’t mention me in the release, please.

[benthans1]

Ok, I get it (having received a few messags from people that happen to know me). I need to explain further, so here comes...

The C library function int abs(int x) returns the absolute value of int x.

Note that it is a library function (stdlib.h) not a build in compiler function!
Since it is part of stdlib.h it is almost always available to you and usually there is no reason to concern yourself with using it. However in this special case we are dealing with an interrupt routine that has to always be available! If some other software has locked the memory that it needs you will be faced with a fubar if/when the interrupt occurs. It is therefore NOT ok to use a libary function in your interrupt routine!

Note:
In case you dont't know , 'fubar' is old millitary slang. It stand for "Fucked Up Beyond All Recogniction".

[benthans1]

Sadly the use of micros() within an interrupt routine also seem to be unsafe, certainly on ESP32 platforms. Unfortunately this is not so easy to replace and I have yet to figure out what to suggest doing about it. At the moment I can’t even suggest a workaround. Also at the moment I lean towards thinking that this should be solved outside the scope of this project as the use of micros() in an interrupt routine is so fundamental that my thinking is that it really needs to be addressed in the platform lib not in a project lib.
Comments welcome please!