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.

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

make IRHVAC send incremental changes. #18310

Merged
merged 1 commit into from
Jun 10, 2023

Conversation

TMaYaD
Copy link
Contributor

@TMaYaD TMaYaD commented Mar 31, 2023

Description:

This change will allow mqtt messages simpler by allowing something like publish cmnd/ir-bridge/IRHVAC {"Power": "On", "Incremental": true}.
The simpler format is required for working with mqtt clients like iotMQTTPanel or similar on android.

Limitations:

  • Requires full message be sent at-least once. I work around this with a system#boot rule
  • Gets messy with multiple devices. I have only one AC per room, and this doesn't doesn't interfere with other IR devices.

TODO:

  • Persist irac_prev_state across reboots.
  • Support multiple devices.

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works with Tasmota core ESP8266 V.2.7.4.9
  • The code change is tested and works with Tasmota core ESP32 V.2.0.7
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

@TMaYaD
Copy link
Contributor Author

TMaYaD commented Mar 31, 2023

I do not have an ESP32 IR Blaster to test.

@s-hadinger
Copy link
Collaborator

Thanks. I would rather have this in the standard build rather than a compile option. Can we think of a distinct command syntax for incremental? For ex adding a json field like "incremental":true?

@TMaYaD
Copy link
Contributor Author

TMaYaD commented Apr 3, 2023

My original idea was to leverage Mem<x> (or similar) as stores.

{..., "Storage": "Mem1"} would store and/or retrieve from Mem1. This will allow incremental operations for multiple devices, if desired.

However, I have a lot of learning to do before I understand the code base enough to do that.

@s-hadinger
Copy link
Collaborator

Relying on Mem1 sounds overly complicated to me. Why not go back to your original idea, but as a runtime option, not a compile-time option?

@github-actions
Copy link

This PR has been automatically marked as stale because it hasn't any activity in last few weeks. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Action - Issue left behind - Used by the BOT to call for attention label Apr 30, 2023
@Jason2866
Copy link
Collaborator

@TMaYaD Will you do the changes?

@Jason2866 Jason2866 removed the stale Action - Issue left behind - Used by the BOT to call for attention label May 1, 2023
@TMaYaD
Copy link
Contributor Author

TMaYaD commented May 4, 2023

Yes. I'm away on vacation right now. Will get back to it after 14th. Need to be home to test home automation ;)

@TMaYaD TMaYaD force-pushed the feature/incremental-irhvac branch from 48686c7 to 47dbd17 Compare May 6, 2023 16:42
This change will allow mqtt messages simpler by allowing something like
`publish cmnd/ir-bridge/IRHVAC {"Power": "On"}`.
The simpler format is required for working with mqtt clients like
iotMQTTPanel or similar on android.

Limitations:
- Requires full message be sent at-least once. I work around this with
   a `system#boot` rule
- Gets messy with multiple devices. I have only one AC per room, and
  this doesn't doesn't interfere with other IR devices.

TODO:
- [ ] Change the build flag `INCREMENTAL_IRAC`to a `SetOption`
- [ ] Persist `irac_prev_state` across reboots.
- [ ] Support multiple devices.
@TMaYaD TMaYaD force-pushed the feature/incremental-irhvac branch from 47dbd17 to 045399c Compare May 16, 2023 10:46
@TMaYaD
Copy link
Contributor Author

TMaYaD commented May 16, 2023

@Jason2866 Updated and tested on my IR blaster. It turned out to be simpler than I expected.

@github-actions
Copy link

This PR has been automatically marked as stale because it hasn't any activity in last few weeks. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Action - Issue left behind - Used by the BOT to call for attention label Jun 10, 2023
@s-hadinger
Copy link
Collaborator

bump

@s-hadinger s-hadinger merged commit 2f616ee into arendst:development Jun 10, 2023
@pkkrusty
Copy link
Contributor

Nice improvement. Looking forward to testing this. Will make node-red flows simpler too.

@s-hadinger s-hadinger mentioned this pull request Aug 26, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Action - Issue left behind - Used by the BOT to call for attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants