Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add light pause to ESP32 #1871

Merged
merged 1 commit into from
Sep 10, 2022
Merged

Add light pause to ESP32 #1871

merged 1 commit into from
Sep 10, 2022

Conversation

s-hadinger
Copy link
Contributor

In Tasmota, we generally turn off IR receive while sending IR, to avoid triggering the decoder and potentially polluting timing. However repeatedly calling disableIRIn() and enableIRIn() cause a crash on ESP32 for unresolved reasons.

However we only need to pause reception, not deconfigure it completely. I propose to add pause() to IRrecv as a lightweight option to suspend receiving IR.

It has already been tested on Tasmota and proved to work fine.

@crankyoldgit crankyoldgit self-assigned this Sep 10, 2022
@crankyoldgit crankyoldgit self-requested a review September 10, 2022 11:34
crankyoldgit
crankyoldgit approved these changes Sep 10, 2022
View reviewed changes
Copy link
Owner

@crankyoldgit crankyoldgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Causing a crash doesn't sound good. Is it something we are doing wrong?! I think I used boilerplate code for it. Can't see anything obviously wrong.

But, your proposed solution/additions looks good to me!

Thanks!

@crankyoldgit crankyoldgit merged commit baa43a2 into crankyoldgit:master Sep 10, 2022
@s-hadinger
Copy link
Contributor Author

I have no idea what goes wrong. The long term solution on esp32 would be RMT ;)

@crankyoldgit
Copy link
Owner

I can probably easily port it to using RMT for sending (TX). But for decoding, I don't think RMT works in a way that is suitable for this library. As far as I understand it, RMT (RX) is suitable for when you know the exact protocol you expect. e.g. You give it the parameters of say, the NEC protocol in advance, and it will match that (and only that). vs. our current approach which allows for greater flexibility in protocol detect, albiet at a significant cost. :-/

@s-hadinger
Copy link
Contributor Author

I see. I believe that it would bring more benefits to Tx anyways

@s-hadinger
Copy link
Contributor Author

I have seen a big improvement when driving ws2812 leds. Bitbanging was not reliable enough on esp32 and RMT was almost mandatory. Maybe IR timing is less sensitive to esp32 uncontrolled interrupts

@crankyoldgit
Copy link
Owner

Well, I'll bump my priority on adding RMT(TX) support, but it's still fairly low. I'm so far behind in my worklog for this project. ☹️

nao-pon pushed a commit to nao-pon/IRremoteESP8266 that referenced this pull request Sep 13, 2022
@s-hadinger @nao-pon
Add light pause to ESP32 (crankyoldgit#1871)
e25bb69 
In Tasmota, we generally turn off IR receive while sending IR, to avoid triggering the decoder and potentially polluting timing. However repeatedly calling disableIRIn() and enableIRIn() cause a crash on ESP32 for unresolved reasons.

However we only need to pause reception, not deconfigure it completely. I propose to add pause() to IRrecv as a lightweight option to suspend receiving IR.

It has already been tested on Tasmota and proved to work fine.
nao-pon pushed a commit to nao-pon/IRremoteESP8266 that referenced this pull request Sep 13, 2022
@s-hadinger @nao-pon
Add light pause to ESP32 (crankyoldgit#1871)
2c8dea8 
In Tasmota, we generally turn off IR receive while sending IR, to avoid triggering the decoder and potentially polluting timing. However repeatedly calling disableIRIn() and enableIRIn() cause a crash on ESP32 for unresolved reasons.

However we only need to pause reception, not deconfigure it completely. I propose to add pause() to IRrecv as a lightweight option to suspend receiving IR.

It has already been tested on Tasmota and proved to work fine.
crankyoldgit added a commit that referenced this pull request Sep 15, 2022
@crankyoldgit
v2.8.3 release
ac3c15b 
_v2.8.3 (20220915)_

**[Bug Fixes]**
- Fix `#if` for DECODE_COOLIX48 (#1796)
- Add missing `prev`s to `decodeToState()` (#1783)

**[Features]**
- Add `pause()` function to ESP32 when receiving. (#1871)
- ARGO: Argo add `sendSensorTemp()` (#1858 #1859)
- HAIER_AC160: Experimental detail support. (#1852 #1804)
- BOSCH144: Add IRac class support (#1841)
- Mitsubishi_AC: update left vane in `IRac` class (#1837)
- Basic support for Daikin 312bit/39byte A/C protocol. (#1836 #1829)
- Experimental basic support for Sanyo AC 152 bit protocol. (#1828 #1826)
- GREE: Add model support for `YX1FSF`/Soleus Air Windown A/C (#1823 #1821)
- Experimental basic support for Bosch 144bit protocol. (#1822 #1787)
- Experimental basic support for TCL AC 96 bit protocol. (#1820 #1810)
- Add basic support for clima-butler (52bit) RCS-SD43UWI (#1815 #1812)
- TOTO: An experimental _(s)wipe_ at support for Toto Toilets. (#1811 #1806)
- CARRIER_AC128: Experimental Basic support for Carrier AC 128bit protocol. (#1798 #1797)
- HAIER_AC160: Add basic support for Haier 160bit protocol. (#1805 #1804)
- DAIKIN: Add basic support for 200-bit Daikin protocol. (#1803 #1802)
- FUJITSU: Improve handling of 10C Heat mode. (#1788 #1780)
- FUJITSU: Improve handling of short (command only) messages. (#1784 #1780)

**[Misc]**
- Improve the `_IRREMOTEESP8266_VERSION_VAL` macro (#1875 #1870)
- SONY: Update supported devices. (#1872)
- SAMSUNG: Update supported devices (#1873)
- NEC: Update supported devices (#1874)
- Give IRmacros.h smaller scope to avoid impacting projects using IRremoteESP8266 (#1857 #1853 #1851)
- Inhibit protocol names for not-included protocols (#1853 #1851)
- Test out codeql static analysis (#1842)
- Remove pylint disable=no-self-use (#1817)
- Fujitsu General: update supported devices (#1813)
- DAIKIN: Update supported devices (#1808 #1807)
- Fujitsu: Update supported remote info. (#1801 #1794)
- DAIKIN128: Update supported devices (#1754)
- Voltas: Add link to manual for 122LZF A/C. (#1800 #1799 #1238)
- Daikin128: Additional unit test. (#1795 #1754)
- MIDEA: Update supported devices (#1791 #1790)
@crankyoldgit crankyoldgit mentioned this pull request Sep 15, 2022
Merged
crankyoldgit added a commit that referenced this pull request Sep 16, 2022
@crankyoldgit
v2.8.3 release (#1878)
84f596e 
**_v2.8.3 (20220915)_**

**[Bug Fixes]**
- Fix `#if` for DECODE_COOLIX48 (#1796)
- Add missing `prev`s to `decodeToState()` (#1783)

**[Features]**
- Add `pause()` function to ESP32 when receiving. (#1871)
- ARGO: Argo add `sendSensorTemp()` (#1858 #1859)
- HAIER_AC160: Experimental detail support. (#1852 #1804)
- BOSCH144: Add IRac class support (#1841)
- Mitsubishi_AC: update left vane in `IRac` class (#1837)
- Basic support for Daikin 312bit/39byte A/C protocol. (#1836 #1829)
- Experimental basic support for Sanyo AC 152 bit protocol. (#1828 #1826)
- GREE: Add model support for `YX1FSF`/Soleus Air Windown A/C (#1823 #1821)
- Experimental basic support for Bosch 144bit protocol. (#1822 #1787)
- Experimental basic support for TCL AC 96 bit protocol. (#1820 #1810)
- Add basic support for clima-butler (52bit) RCS-SD43UWI (#1815 #1812)
- TOTO: An experimental _(s)wipe_ at support for Toto Toilets. (#1811 #1806)
- CARRIER_AC128: Experimental Basic support for Carrier AC 128bit protocol. (#1798 #1797)
- HAIER_AC160: Add basic support for Haier 160bit protocol. (#1805 #1804)
- DAIKIN: Add basic support for 200-bit Daikin protocol. (#1803 #1802)
- FUJITSU: Improve handling of 10C Heat mode. (#1788 #1780)
- FUJITSU: Improve handling of short (command only) messages. (#1784 #1780)

**[Misc]**
- Improve the `_IRREMOTEESP8266_VERSION_VAL` macro (#1875 #1870)
- SONY: Update supported devices. (#1872)
- SAMSUNG: Update supported devices (#1873)
- NEC: Update supported devices (#1874)
- Give IRmacros.h smaller scope to avoid impacting projects using IRremoteESP8266 (#1857 #1853 #1851)
- Inhibit protocol names for not-included protocols (#1853 #1851)
- Test out codeql static analysis (#1842)
- Remove pylint disable=no-self-use (#1817)
- Fujitsu General: update supported devices (#1813)
- DAIKIN: Update supported devices (#1808 #1807)
- Fujitsu: Update supported remote info. (#1801 #1794)
- DAIKIN128: Update supported devices (#1754)
- Voltas: Add link to manual for 122LZF A/C. (#1800 #1799 #1238)
- Daikin128: Additional unit test. (#1795 #1754)
- MIDEA: Update supported devices (#1791 #1790)
@crankyoldgit
Copy link
Owner

FYI, the changes mentioned above have now been included in the new v2.8.3 release of the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crankyoldgit crankyoldgit crankyoldgit approved these changes

Assignees

@crankyoldgit crankyoldgit

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@s-hadinger @crankyoldgit