search

crankyoldgit / IRremoteESP8266

Infrared remote library for ESP8266/ESP32: send and receive infrared signals with multiple protocols. Based on: https://github.com/shirriff/Arduino-IRremote/
GNU Lesser General Public License v2.1
2.84k stars 810 forks source link

Bang & Olufsen support plus higher precision and frequency PWM generation #2074

Open danielwallner opened 4 months ago

danielwallner commented 4 months ago

Bang & Olufsen 455 kHz protocol support with additional improvements to the PWM generation. Uses fixed point pulse widths for higher precision and polling of micros instead of delay to be able to reach the frequencies required. On ESP8266 lower than 160 MHz a free running PWM mode was also added to get as close to 455 kHz as possible. A bonus is that the lower frequencies now are accurate to within a few hertz. Some additional commits were necessary to be able to run the tests with -Wall -Wextra -Werror on macOS. The commit that comments unused constants may need a bit of cleanup. Note that the current test isn't compatible with micros polling and the old code is used for tests.

NiKiZe commented 4 months ago

Might it be worth doing separate PRs for the commits to see that each step is OK? Especially the sending change and the added protocol.

First 2 commits looks good

The Cast looks like s bug and should be investigated some more.

As you say the consts needs cleanup when using / / use separate lines to not modify the existing ones, but if truly unused we should consider removing them if they do not provide any value in some other way.

Having different variants between ESP and tests does reduce the reason to have tests? Should/could we consider some other way of dealing with that?

danielwallner commented 4 months ago

Ok, I'll try to separate this into more than one PR.

Yes, the cast definitely looks like a bug.

What should I do with the unused constants? Some of them are explicitly commented as being there for documentation. Does that apply to all of them or should they just be removed if unused?

I can't think of a way to test the PWM that doesn't include ifdefs with delays inside the PWM loop. That's probably better than essentially disabling the test as it now though.

Also, I forgot to mention that the calibration no longer does anything but was kept to not break things.

There's also an additional test failure on macOS in IRac_test that I didn't look into.

danielwallner commented 4 months ago

I now what the IRac_test failure is now. _swingh_prev is not reset in IRLgAc::stateReset. But I also noticed that ir_Delonghi_test fails about once every five runs too. The message in TestDecodeDelonghiAc is decoded as unknown when it fails. I have not been able to find the cause for this. I also haven't checked if this is a problem with clang everywhere or just on macOS.