[Build] Add compatibility with C++20

[tonhuisman]

Feature:

• Make IRremoteESP8266 compatible with C++20
  • Fix use of deprecated volatile operators like ++ and volatile method arguments
  • Add build environment for using ESP32 IDF 5.x and fix compilation for 'regular' ESP32 environment, still using IDF 4.x
  • Fix differences for IDF 5.x build using code by Tasmota team (by @s-hadinger )

Resolves #2039

[tonhuisman]

Looks like the linter and doxygen need some fix to support conditional compilation 👎

[NiKiZe]

Have you considered defining internal types separately, all the #if s are becoming quite messy?

[tonhuisman]

Have you considered defining internal types separately, all the #if s are becoming quite messy?

I'll think about that, but IMHO it'll be about as messy... 😞

[tonhuisman]

Have you considered defining internal types separately, all the #if s are becoming quite messy?

Now using typedefs for everything that was defined volatile (though most still using volatile).

[NiKiZe]

I'm for the atomic_ changes, not sure about the rest which is more relevant to platformio than c++?

[Jason2866]

@tonhuisman Why going back to core 2.0.x? Makes no sense. The platform 2023.10.03 is NOT based on IDF5.1
The left compile error is just a missing include of gpio.h

EDIT: Comment race condition ;-)

[Jason2866]

@tonhuisman Adding this fixes the compile error and no code change regarding interrupts are needed

#if defined(ESP32)
#if ( defined(ESP_ARDUINO_VERSION_MAJOR) && (ESP_ARDUINO_VERSION_MAJOR >= 3) )
#include <driver/gpio.h>
#endif  // ESP_ARDUINO_VERSION_MAJOR >= 3
#endif
#endif  // UNIT_TEST

[tonhuisman]

Adding this fixes the compile error

Thanks! 🦸
That was what I didn't know I was searching for, but had to find...

[crankyoldgit]

FYI. I've seen this PR and been thinking about how to do this "better" in a way.
My plan is to create a new branch/PR etc and try to get rid of as much of the existing volatile stuff and replace it with atomic where appropriate. I think I can wittle it down to only a few variables/memory pointers etc. As this will be a major change that could introduce race-conditions and concurrency issues, I want to make sure this is tested on real hardware & in real situations as much as practical before I release it.

Btw .. thanks ever so much for all the work you've all put into this PR, it's awesome. You've done a great job. I just want to clean up something we inherited from the original IRremote library and the early days of this library. That stuff was left pretty much unchanged because it was magic code & worked, and assumed people know exactly what they were doing. My research in to volatile and what it does makes it sort of clear the use of it was more a kuldge than the right way to fix it. You're work has forced me to look at it and research it, and realise it only kind of does what we wanted.

In short, I want to enlist you guys (@tonhuisman & @Jason2866, tasmota etal) in helping to better fix it, and I don't want to just copy a lot of your work and pretend it was my work.

[Jason2866]

Looking forward to do testing on all ESP32x MCUS. Considering to do a RMT driver?
It looks like all newer ESP32 MCUs have timing issues with the current driver.

[crankyoldgit]

I've changed my mind on the approach.
I'll look at approving/merging this as is and do the other changes later. I'm quite time-poor now-a-days so best I not hold this up.

[crankyoldgit]

What tests on actual hardware have been done? i.e. Does it still capture okay on ESP32s & 8266s?

[crankyoldgit]

Does this hurt or cause an issue if it is always included?
i.e. Can we just include this header all the time?

[tonhuisman]

I'm more inclined to only add includes when really needed, trying to avoid unneeded code being pulled in the build or having unexpected interactions.

[sagariotminds]

Thank you for your update regarding the library versions. I will proceed with verifying these library versions and ensure compatibility. Should I encounter any issues or require further clarification, I will reach out to you.

…

On Fri, 31 May, 2024, 8:18 am Zhentao-Lin, ***@***.***> wrote: V2.0.14 before can use this library, V2.0.15-V2.0.17 I have not tested, it should be OK. Neither V3.0.0 is possible. — Reply to this email directly, view it on GitHub <#2040 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BBRO5NNRSZBIPRNCWJQPKG3ZE7QGJAVCNFSM6AAAAAA5WJEGFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBRGE2DCNRYGM> . You are receiving this because you commented.Message ID: ***@***.***>

[mboehmerm]

Works for me without errors, if add "bool timerStarted(hw_timer_t *timer);" to "esp32-hal-timer.h" and "esp32-hal-timer.c" and use this function in "IRrecv.cpp".

Base were the commits 9cfcebc and db98aa0

Test and files can be found here : https://github.com/mboehmerm/Three-IPS-Displays-with-ST7789-170x320-240x280-240x320/tree/main/IRremoteESP8266

Added in esp32-hal-timer.h

bool timerStarted(hw_timer_t *timer);

Added in esp32-hal-timer.c

bool timerStarted(hw_timer_t *timer) {
  return timer->timer_started;
}

In the file IRrecv.cpp i replaced

  timerWrite(timer, 0);
  timerStart(timer);

with

  timerWrite(timer, 0);
  if (timerStarted(timer))
    timerRestart(timer);
  else
    timerStart(timer);

and

  timerStop(timer);

with

  if (timerStarted(timer))
    timerStop(timer);

I hope there is a better solution without modifying the esp32-hal-timer files.

[Jason2866]

@mboehmerm Dont know what's causing the need that you have to modify files.
The PR is working well in EspEasy and Tasmota without any further changes with Arduino Core 3.0.x

[mboehmerm]

@mboehmerm Dont know what's causing the need that you have to modify files. The PR is working well in EspEasy and Tasmota without any further changes with Arduino Core 3.0.x

This is the first solution i found, that works with Arduino IDE without any errors. Is there a better solution? Where can i find it?

[tonhuisman]

the first solution i found, that works with Arduino IDE without any errors. Is there a better solution?

🤔 (... sorry, too obvious...)

[almirus]

I get an error

16:49:14.291 -> E (1146905) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.322 -> Timestamp : 001146.959
16:49:14.322 -> Library   : v2.8.6
16:49:14.322 -> 
16:49:14.322 -> Protocol  : UNKNOWN
16:49:14.322 -> Code      : 0xEBA17F (11 Bits)
16:49:14.322 -> uint16_t rawData[22] = {3060, 1620,  468, 4140,  5816, 5818,  5814, 5816,  5816, 5816,  5814, 5818,  5816, 5816,  5816, 5814,  5818, 5816,  5814, 5816,  5814, 5818 }E (1146978) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.354 -> E (1146979) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.354 -> E (1146980) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.354 -> E (1146985) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.354 -> E (1146991) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.450 -> E (1147072) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.450 -> E (1147078) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.450 -> E (1147084) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.450 -> E (1147090) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.450 -> E (1147096) gptimer: gptimer_start(348): timer is not enabled yet
16:49:14.489 -> ;  // UNKNOWN EBA17F
16:49:14.489 -> 
16:49:14.489 -> 
16:49:14.489 -> Timestamp : 001147.149
16:49:14.523 -> Library   : v2.8.6
16:49:14.523 -> 
16:49:14.523 -> Protocol  : UNKNOWN
16:49:14.523 -> Code      : 0x5B39BFD5 (12 Bits)
16:49:14.523 -> uint16_t rawData[23] = {3058, 1622,  470, 5494,  5818, 5816,  5814, 5818,  5816, 5816,  5816, 5816,  5812, 5818,  5816, 5816,  5816, 5814,  5818, 5816,  5816, 5816,  5816};  // UNKNOWN 5B39BFD5
16:49:14.523 -> 
16:49:14.523 -> 
16:59:14.509 -> E (1747136) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.541 -> E (1747138) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.541 -> E (1747138) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.541 -> E (1747142) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.541 -> E (1747148) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.541 -> E (1747154) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.573 -> E (1747160) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.573 -> E (1747166) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.573 -> E (1747172) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.573 -> E (1747177) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.573 -> E (1747183) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.573 -> E (1747189) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.605 -> E (1747195) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.605 -> E (1747201) gptimer: gptimer_start(348): timer is not enabled yet
16:59:14.605 -> E (1747206) gptimer: gptimer_start(348): timer is not enabled yet

[Buddy-Matt]

I'm also seeing the same error as @almirus regarding gptimer when using Arduino IDE targeting a M5Stack NanoC6.

The library also fails to receive the NEC codes that were previously working under ESP32 2.0.17 - so I don't believe they're spurious either.

[NiKiZe]

I'm considering merging #2144 which only has changes for ESP32 CoreV3, that way we don't risk breaking anything for earlier versions?

After that we could look into merging any other changes? How does that sound?
Or any reasons for not doing that?

[tonhuisman]

We have been using these adjustments on the Arduino 3.x/IDF 5.x builds of ESPEasy since I started this PR, so I see no reason not to merge this, and other relevant changes.
I still don't fully understand why PR 2144 was created, though, since this PR has been open for quite some time...

[NiKiZe]

We have been using these adjustments on the Arduino 3.x/IDF 5.x builds of ESPEasy since I started this PR, so I see no reason not to merge this, and other relevant changes. I still don't fully understand why PR 2144 was created, though, since this PR has been open for quite some time...

In principle I agree, #2144 is a duplicate, but there is also about the cleanest solution. (read: easiest to review)
#2040 (this PR) has 3? different fixes mixed in one PR. I'm not able to verify these changes.
The reason for me starting to look into merging #2144 was #2144 (comment)

In it's current state it essentially only has added lines and no changes, which I can validate does not create any new issues for <V3
Another part, which currently is over my head, is the mentions about IDF, for example #1575, this library is for Arduino, even if that pulls in something else.

My hope is that this PR can be rebased on #2144 and then review the remaining parts.

[tonhuisman]

In principle I agree, #2144 is a duplicate, but there is also about the cleanest solution. (read: easiest to review)
#2040 (this PR) has 3? different fixes mixed in one PR. I'm not able to verify these changes.

The scope of this PR is to instate compatibility with C++20, not 'just' Arduino 3.x.
To have compatibility with C++20 the 3 mentioned changes are required to avoid compiler warnings in the build log.

[swoboda1337]

Is this ever going to get merged?

[crankyoldgit]

This appears to be failing the unit tests. Perhaps it needs to be re-based on the recent PRs that address the automated build tests?

[tonhuisman]

This appears to be failing the unit tests. Perhaps it needs to be re-based on the recent PRs that address the automated build tests?

It just needed an update of the platform package, as the old one was over 2 years old... 🤔 Now, all is fine again.

[crankyoldgit]

LGTM. Thanks for fixing.