Modify MyMainESP8266 for core 2.6 and 3.0 (#1496)

[virtual-maker]

Simplify MyMainESP8266.cpp for core 2.6, 2.7, & 3.0.x and replace ICACHE_RAM_ATTR by IRAM_ATTR.

This solution works with ESP8266 cores 2.6x, 2.7x and 3.0x according the description in: 1496#issuecomment-898485845 This solution doesn't need any changes in existing MySensors example sketches for the setup() code.

Additionally added is a check for the ESP8266 core major version 3 and added some #if #else to keep the compatibility with the older version 2 core.

Also are updated the code locations for use of IRAM_ATTR in two example sketches.

[d-a-v]

These changes are obfuscating and unstable regarding the future, like they were in the past. Next versions of esp8266 Arduino core may change again and the whole process of adapting MyMainESP8266.cpp will start, again. Hacking an arduino core may not be the way to go, especially when an API allows one to avoid it.

1513 removes the need of these hacks by simply removing this file (MyMainESP8266.cpp) and using the dedicated esp8266-Arduino API for calling ::_process().

YMMV

[mfalkvidd]

Thanks @d-a-v

Unfortunately, I don't have enough knowledge to determine pros and cons myself, so I will need to rely on input from others.

@virtual-maker do you have any input on #1524 vs #1513?

[d-a-v]

I could also have been citing @Yveaux

On the other hand, I'm not happy with including files from the Arduino core (also for other architectures) as part of the MySensors stack. The main esp8266 startup code is being modified on a regular basis (see https://github.com/esp8266/Arduino/commits/master/cores/esp8266/core_esp8266_main.cpp) so we will always struggle to keep up.

... and this is exactly what #1513 answers to.

~Plus, it is not sure the changed code is still compatible with previous version of the arduino core (I haven't checked myself)~ (comment deleted per title)

@virtual-maker can you please elaborate on this comment ?

But this would break all sketches floating around in the web.

[virtual-maker]

Hi all, the original problem was the code in file MyMainESP8266.cpp. This code was practically a copy from the old Arduino core for ESP8266 (probably ver. 2.6), namely the file core_esp8266_main.cpp. In this rather extensive file, only the two calls of setup() and loop() have been replaced or extended to the corresponding function calls of MySensors, _begin() and _process().

However, in the ESP8266 version 3.x this core file has changed considerably, so that the copy in MySensors no longer fits and works. My first suggestion was to copy the new core code from core_esp8266_main.cpp into MySensors MyMainESP8266.cpp and adjust the two calls accordingly.

To this end, @Yveaux commented as quoted above and notes that the same problem exists with the other hardware architectures (e.g. ESP32 and SAMD) and with copies of outdated main.cpp files from the respective Arduino core inside the MySensors library.

Both the solution in PR #1513 and this #1524 avoid duplicating the core code in MySensors for ESP8266.

However, PR #1513 does not provide a solution (compared to #1524) for calling to _begin(). For this, the user must adapt the setup() function in the sketch and call _begin() manually, as suggested by @Yveaux:

void setup()
{
    static bool runonce = true;
    if (runonce)
    {
        runonce = false;
        Serial.begin(MY_BAUD_RATE, SERIAL_8N1, MY_ESP8266_SERIAL_MODE, 1);
        _begin();
    }
    // ...your regular setup() code...
}

This means that at least the MySensors examples to the ESP8266 have to be adapted. Furthermore, it has to be explained to the user that other MySensors node sketches and examples have to be extended accordingly. Solution #1524 avoids this and is compatible to the existing approach and examples.

Furthermore, I think that the solution here can also be adopted for other hardware architectures to avoid the problems with outdated code copies from the respective Arduino cores in MySensors library.

[d-a-v]

Thank for explanation @virtual-maker

I think we all agree that rewriting a core file in an application or a library is not a preferred solution.

The aforementioned function schedule_recurrent_function_us() was added in esp8266 Arduino core to specifically target this kind of issue.

1513's pull request has been living for months and left without bringing to my attention the missing call to _begin() from setup(). I would have proposed something if I was aware. For example backward portability can be solved by adding calls in constructors (like MyMessage) or sending functions send().

Anyway, I am not a user, and you are a maintainer. I simply hope that, as an esp8266 Arduino maintainer, and schedule_recurrent_function_us() author, this will not prevent users and make them come to us because they cannot upgrade to a new version of the esp8266 arduino core and think it's our duty to fix things.

[virtual-maker]

@d-a-v Hi David, I'm really sorry that the communication on your PR #1513 went so badly. I wasn't aware of the motivation you had. MySensors gets help directly from the esp8266 Arduino core team, and nobody responds. Very bad!

Since you wrote that you are not a user of MySensors, I had tried to clarify the setup problem with @kasparsd in this comment. Unfortunately I didn't get an answer. Here, too, the communication went badly. What a pity!

I think your ESP8266 feature schedule_recurrent_function_us() is a very good solution to do tasks in the background. Unfortunately MySensors is not an ideal use case for it and would require some rework in the library, in the examples and also in the CI tests.

I think the MySensors core team should clarify internally how such a communication failure can be avoided in the future.

Many thanks again for your offer to help! BR Immo