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

fix: updated breeze codes and fix test #749

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

YogevBokobza
Copy link
Collaborator

@YogevBokobza YogevBokobza commented Jan 17, 2024

Description

Describe what you did and why.

Related issue (if any): fixes #issue_number_goes_here

Checklist

  • I have followed this repository's contributing guidelines.
  • I will adhere to the project's code of conduct.

Additional information

Anything else?

@pull-request-size pull-request-size bot added the size: xxl Pull request has more then 1000 lines label Jan 17, 2024
@auto-me-bot auto-me-bot bot added the status: needs review Pull request needs a review label Jan 17, 2024
Copy link

codecov bot commented Jan 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b620b30) 98.54% compared to head (60fbd92) 98.54%.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #749   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files          11       11           
  Lines        1098     1098           
=======================================
  Hits         1082     1082           
  Misses         16       16           

@YogevBokobza
Copy link
Collaborator Author

@thecode Do You have a way to validate the DB against the issue opened?

@thecode
Copy link
Collaborator

thecode commented Jan 17, 2024

While this is an updated database the relevant AC that was mentioned in #705 still not fixed

    "SWTA7003": {
        "IRSetID": "SWTA7003",
        "OnOffType": 0,
        "IRWaveList": [

The OnOffType should be 1 for ACs that are using Toggle command (so that the device doesn't know if the AC is on or off and we add buttons to allow manual override of the device state.

I can't simply modify this file since the codes for the AC also needs to be changes.

While I have no objection to update the DB (and we should update it when we get a new one) I don't understand how it is not reflecting what the user see in Switcher App. If the App shows manual state override and user needs it (for ACs that has separate on of commands this is irrelevant anyhow so he wouldn't look for it) the file should also indicate the same.

@auto-me-bot auto-me-bot bot added status: approved Pull request is approved and removed status: needs review Pull request needs a review labels Jan 17, 2024
@YogevBokobza
Copy link
Collaborator Author

While this is an updated database the relevant AC that was mentioned in #705 still not fixed

    "SWTA7003": {
        "IRSetID": "SWTA7003",
        "OnOffType": 0,
        "IRWaveList": [

The OnOffType should be 1 for ACs that are using Toggle command (so that the device doesn't know if the AC is on or off and we add buttons to allow manual override of the device state.

I can't simply modify this file since the codes for the AC also needs to be changes.

While I have no objection to update the DB (and we should update it when we get a new one) I don't understand how it is not reflecting what the user see in Switcher App. If the App shows manual state override and user needs it (for ACs that has separate on of commands this is irrelevant anyhow so he wouldn't look for it) the file should also indicate the same.

Good point. I will ask the Switcher dev and update back.

@TomerFi TomerFi merged commit 9c3fe96 into TomerFi:dev Jan 18, 2024
10 checks passed
@auto-me-bot auto-me-bot bot added status: merged Pull request merged and removed status: approved Pull request is approved labels Jan 18, 2024
@TomerFi
Copy link
Owner

TomerFi commented Jan 18, 2024

3.4.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: xxl Pull request has more then 1000 lines status: merged Pull request merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants