Maint: convert IR library to Arduino-IRremote

[Graimalkin]

IRremoteESP8266 appears abandoned ( last updated July 2023 ).

It's based on Arduino-IRremote, which IS being maintained, and is not quite a drop in swap.

So, I swapped them. 😛

I did have to make a choice here to either re-order all of ir_codes.h or swap the byte order for results.decodedRawData so that it could mimic results.value. I went with swapping the byte order via esp8266Value(), because changing all of ir_codes.h sounds unpleasant to review, and like it's just asking for issues.

Also added in some debugging output for the serial monitor so it's easier to see what's going on.

License changes from GNU GPL -> MIT, so that seems fine here.

This is part of a bigger change that allows WLED to work with MagiQuest wands from Great Wolf Lodge. The wands aren't working with the old IRremoteESP8266 library, but they do with this one! I'm just breaking the changes up for ease of processing. (https://github.com/Graimalkin/WLED/pull/2)

Testing:

Tested with an ESP32 dev board, TSOP4838 IR sensor, and generic LED light strip. Also grabbed one of the white 44 key IR remotes from Amazon -- https://www.amazon.com/dp/B0982BVCSD

Colors changed, brightness goes up and down, can swap to presets.

I'd recommend trying on a few more hardware stacks if you have it.

Summary by CodeRabbit

• Refactor

  • Updated infrared remote control support to use a newer IR library, improving compatibility and reliability.
  • Enhanced debug output for IR remote decoding.
  • Maintained compatibility with previous IR code values.

• Chores

  • Updated dependency for the IR remote library to a newer version from a different source.

[coderabbitai[bot]]

Walkthrough

The changes update the infrared (IR) remote control functionality to use the newer Arduino-IRremote library (version 4.4.2) instead of IRremoteESP8266. The IR handling code is refactored to use the new IrReceiver API, with added debug output and compatibility helpers. Infrared-related header includes are removed from the main header file.

Changes

Files / Groups	Change Summary
platformio.ini	Switched IR remote library dependency from [email protected] to [email protected] via a GitHub URL.
wled00/ir.cpp	Refactored to use IrReceiver API; removed IRrecv pointer; added decodeTypeToStr helper; reversed bits for select protocols; enhanced IR handling and debug output.
wled00/wled.h	Removed conditional inclusion of IRremoteESP8266-related headers regardless of WLED_DISABLE_INFRARED flag.

---

📜 Recent review details

Configuration used: CodeRabbit UI Review profile: CHILL Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0dce70a1b7548865df4be31f241ba1cd5d19423 and 59f829b86fcc1f4074da5c547a49f4fa9124d4fa.

📒 Files selected for processing (2)

• wled00/ir.cpp (2 hunks)
• wled00/wled.h (0 hunks)

💤 Files with no reviewable changes (1)

• wled00/wled.h

🚧 Files skipped from review as they are similar to previous changes (1)

• wled00/ir.cpp

✨ Finishing Touches

• [ ] 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[blazoncek]

Thank you! I hope this also reduces the final WLED binary size. Have you compared those? If it increases it instead it will be a tough call. Other than that one of the possible rewrites is to convert everything to JSON based decoding.

[DedeHai]

checked for code size: looks like it saves about 3k on a C3 build in current state. I would request more testing. IR is a feature used in many setups.

[netmindz]

While the library we were using had started life as being "based off" the library you are swapping to here, there is mention on GitHub about almost total rewrite for version 2.0

There is also the question about which remotes are supposed by which library. The change brings support for the remote you are trying to use if I'm understanding correctly, but which remotes might we loose support for by swapping?

[Graimalkin]

While the library we were using had started life as being "based off" the library you are swapping to here, there is mention on GitHub about almost total rewrite for version 2.0

There is also the question about which remotes are supposed by which library. The change brings support for the remote you are trying to use if I'm understanding correctly, but which remotes might we loose support for by swapping?

So right now, platformio.ini is by default enabling these IR remote types:

  -D DECODE_HASH=true
  -D DECODE_NEC=true
  -D DECODE_SONY=true
  -D DECODE_SAMSUNG=true
  -D DECODE_LG=true

All of these are supported by the new Arduino-IRremote library.

Of these, it looks like in the IRremoteESP8266 (old) repo, NEC based remotes ( like my white 44 key one ), Sony, and Samsung have their results.value info come in in LSB (least significant bit) order but their address and command have been hit by reverseBits which does what it sounds like - reverses the bits. This puts these in MSB order. So when I reconstruct results.value from decodedRawData, it needs to be reversed if we want the values to match what is already in ir_codes.h.

Arduino-IRremote on converting from MSB first to LSB first -- https://github.com/Arduino-IRremote/Arduino-IRremote/tree/master#how-to-convert-old-msb-first-32-bit-ir-data-codes-to-new-lsb-first-32-bit-ir-data-codes

I did make a change to the code to only convert those remotes that I found in IRremoteESP8266 that were being reversed ( and that arduino-IRremote mentioned needing it ). You can see those changes here before the debug logging changes: https://github.com/wled/WLED/pull/4732/commits/b0dce70a1b7548865df4be31f241ba1cd5d19423

Anyways, I went ahead and ordered a few more IR remotes that are supported per docs and mentioned in ir_codes.h. These should be here in a couple of weeks, so I'm going to mark this a DRAFT for now, and re-open when the remotes come in and I can test them out.

[blazoncek]

Just so you are informed: Many users (including me) use various remotes using JSON IR remote option. For example: I have an old Apple TV remote that I repurposed for WLED.

If the new library reverses bits all of these remotes will stop working until JSON files are updated (and accompanying JSON files in WLED-Docs) or this reversal is taken care of.

EDIT: I'm very fond of these.

[Graimalkin]

Just so you are informed: Many users (including me) use various remotes using JSON IR remote option. For example: I have an old Apple TV remote that I repurposed for WLED.

If the new library reverses bits all of these remotes will stop working until JSON files are updated (and accompanying JSON files in WLED-Docs) or this reversal is taken care of.

EDIT: I'm very fond of these.

I believe the Apple TV remote is NEC based, so would need the reversebit to work. I think it'll work with these changes as is. If you get a chance, can you check it out?

Most NEC remotes should work as-is. There are some weird 16 bit address extended NEC remotes that might potentially see issues - but I'm not sure without testing with one.

That little remote you linked looks like another NEC remote. I grabbed these ones for testing with (In addition to the 44 key one I have):

https://www.aliexpress.us/item/2255800121523134.html?spm=a2g0o.order_list.order_list_main.10.63e318028Xdvoc&gatewayAdapt=glo2usa

[blazoncek]

If you get a chance, can you check it out?

It will be more than 2 weeks before I'm back home. Where all my equipment is.

[Graimalkin]

If you get a chance, can you check it out?

It will be more than 2 weeks before I'm back home. Where all my equipment is.

It's all good. It's about 2 weeks until I get those remotes in also. 👍

[Graimalkin]

So I got my collection of remotes in, and I'm taking a look at this some more today.

I'm currently testing with:

• white 24-key IR remote with R,G,B + 12 color-tones
• blue 40-key IR remote with keys for 25%, 50%, 75% and 100%
• white 44-key IR remote with up/down arrows for the colors R,G and B
• black 6-key IR remote with CH up/down + Vol up/down

All of these are NEC based remotes, and they all seem to be working correctly. I can change colors, dim, turn off, and even repeat press for dimming up / down.

I also grabbed one of the TV remotes around here (Samsung) which according to platformio.ini is being decoded, to see if the codes stayed the same between MAIN / this branch. These remotes aren't in ir_codes.h, or supported in the drop down. BUT they should be getting decoded. They're going to use a variant of the NEC standard, so 🤷. Here's some testing from it:

Button                                 MAIN                               branch
power                                 0xE0E040BF                      0xE0E040BF
vol up                                0xE0E0E01F                      0xE0E0E01F
vol down                              0xE0E0D02F                      0xE0E0D02F
channel up                            0xE0E048B7                      0xE0E048B7
channel down                          0xE0E008F7                      0xE0E008F7
1                                     0xE0E020DF                      0xE0E020DF
2                                     0xE0E0A05F                      0xE0E0A05F
3                                     0xE0E0609F                      0xE0E0609F

That's about it for the remotes that I have available for testing with.

Let me know if there are next steps, or if I should just abort.

[DedeHai]

If you can, please test on all platforms, especially ESP8266, C3 and S2 as those have a single core.

[willmmiles]

When I looked at Arduino-IRremote I noticed this pragraph. If it is true, it might be a dealbreaker or may require additional safeguards implemented.

TL;DR: WLED will be fine. That warning is intended for people trying to do IR and LEDs using bit-banging on slower hardware and 2017-era libraries.

--

So as it happens, I was looking through our current IR library earlier because I was curious as to whether or not it used the RMT hardware on ESP32. Sending and receiving IR data is what the RMT device was intended for, after all!

It turns out that no, IRremoteESP8266 is already just using a generic GPIO interrupt, and timestamping when the line changed state. Inefficient perhaps compared to using the dedicated hardware; but works well enough and isn't particularly platform specific. (This is good for me, in the context of replacing the RMT driver, as there's no way to share the RMT interrupt between two different drivers ... )

I just took a quick look through the Arduino-IRremote code, it seems like they've taken a different approach: essentially sampling the receiver with a 20kHz (!!) timer interrupt. This also works, but it'll be really wasteful of CPU - you pay the cost for all those interrupts even when the IR line is idle.

That said: I don't anticipate that it will cause any (new) issues with NeoPixelBus LED output. Any time we're using real hardware (I2S, RMT, UART, whatever) we're insulated from small CPU timing gaps; and even the bit-banging driver allows interrupts through after every pixel is written. (I should know; I PR'd that feature to NeoPixelBus myself to fix some glitching/crashes on an 8266 ;)

[DedeHai]

not using hardware pin-interrupts is a very bad design...

[DedeHai]

Ran a quick test, the polling eats up 10% of CPU time i.e. I see a 10-15% drop in FPS. Not acceptable.

[willmmiles]

not using hardware pin-interrupts is a very bad design...

In fairness to the devs, (ab)using a timer is almost certainly the most portable solution. It's the equivalent of bit-banging -- highest CPU costs, most vulnerable to timing issues, but works absolutely everywhere with every protocol on any pin that can do GPIO.

Having spent time digging through FastLED and NeoPixelBus - both libraries that aim to reach for good performance everywhere, with a large number of custom drivers included - I can appreciate why a library maintainer (or team) might opt to say "no, we are not interested in being a library of optimized drivers for every chip ever -- we just do the one simple thing all the time; if you don't want that, go elsewhere".

Unfortunately, the long term impact of such a choice is that we, the developer community, never get to settle on the "one battle-tested library that everyone uses"; we end up with a panopoly of competing libraries, forcing people to choose between performance vs protocol support vs hardware support vs active maintenance vs ease-of-use .. sigh.

[DedeHai]

That may have sounded wrong. Not questioning the devs of that library: if it's to be universal and for arduino purposes, using timer polling is the easiest option, no doubt. For our purposes however, it's a bad design so it can't be used. The easy part is to read the signal, the hard part is to decode it and support all the formats. So if we get a library that just does the decoding, writing a function that reads pin interrupt timings should not be a huge effort.

[willmmiles]

Ran a quick test, the polling eats up 10% of CPU time i.e. I see a 10-15% drop in FPS. Not acceptable.

Agreed - that's a significant regression; I don't want to merge that for all users. I would like to see some path forward for the ultimate goal here, though - my family has good memories of Great Wolf Lodge, we're hoping to visit sometime soon. ;)

As I see it, that leaves:

• Someone forks IRremoteESP8266 and adds MagiQuest protocol support, at least until the devs there get back on their feet. (I see recent commits in the repo, but it's hard to know how active they'll be, if they'll move through the backlog). We're already keeping a stable of forked dependencies in the project; it's not the first time and won't be the last.
• Split the IR support out to a "core usermod". (I'd like to do this anyways, if only to make it possible to exclude the IR library dependency entirely when it's not needed!) Then it's possible to have two (or more) different implementations using different backing libraries via each module's library.json. This allows easy build time selection. (Or potentially runtime selection, if someone includes both.)
• Both of the above?
• Any other ideas?

The easy part is to read the signal, the hard part is to decode it and support all the formats. So if we get a library that just does the decoding, writing a function that reads pin interrupt timings should not be a huge effort.

I did a quick search and I couldn't find one - basically every library that does decoding also includes one or more hardware drivers to read into their own idiosyncratic decoding format. (Also the hardware drivers are legitimately nontrivial -- you need precise enough timestamping, which isn't standard on most platforms, so it has to be set up by the driver.) IMO it's probably easier to port the decoding from one library to another than it is to replace the drivers for one of the existing libraries. YMMV, though.

https://github.com/IRMP-org/IRMP might be worth looking at; it supports both timer-based and pin-irq-based sampling. Although curiously the README consistently links over to IRremoteESP8266 (?!?).

[Graimalkin]

Ok, going to go ahead and close this then as not a good change due to the CPU increase.

Thanks for all of the feedback folks!