core icon indicating copy to clipboard operation
core copied to clipboard

Fix Auto mode for TCC devices like the Lyric Round

Open nprez83 opened this issue 5 months ago • 3 comments

Proposed change

PR #123490 by @kristof-mattei introduced a change in the way the Auto mode is detected that does not work for TCC devices like the Lyric Round. I own both the Lyric Round (TCC device) and the T5 (LCC device), so I'm able to test code for both types of devices. I went into detail on the difference between the way Auto mode is implemented by TCC and LCC devices in PR #106853 and #106925, but I'll copy it here for ease of reference.

"The source of the problem is the way the two types of devices use the autoChangeoverActive variable, as well as how they display the mode within changeableValues, and which modes they allow you to pass.

For LCC devices, auto mode is enabled in two steps. First, you have to allow it by setting autoChangeoverActive to true. Then, you can set the changeableValues.mode to "Auto" when you'd like to use it. LCC devices allow modes "Off", "Heat", "Cool", and "Auto". Of note, while working on this PR I realized if the user sets autoChangeoverActive to false in the physical device itself, and later tries to pass mode "Auto", it will not work, so I added code to the async_set_hvac_mode_lcc function to take care of this.

For TCC devices, auto mode is enabled directly by setting autoChangeoverActive to true or false. When you do this, changeableValues.mode will indeed display "Auto" (see image below). Nevertheless, TCC devices only allow you to set changeableModes.mode to "Off", "Heat", or "Cool", hence you can't actually pass mode="Auto" at any point or you get the the following error Mode 7 is not allowed. Allowed modes are Cool, Heat, Off. This is confusing, and poor form in the way they implemented the api for TCC devices. I have no idea why they didn't just design both devices to do things the same way."

This has unfortunately happened before, where an LCC device owner submits a PR that breaks something for those of us TCC owners. I wonder if the integration should be split up for the two types of devices, but that's a broader question for the integration owner.

Type of change

  • [ ] Dependency upgrade
  • [X] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [ ] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

See PR #106853.

Checklist

  • [X] The code change is tested and works locally.
  • [X] Local tests pass. Your PR cannot be merged unless tests pass
  • [X] There is no commented out code in this PR.
  • [X] I have followed the development checklist
  • [X] I have followed the perfect PR recommendations
  • [X] The code has been formatted using Ruff (ruff format homeassistant tests)
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [ ] The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • [ ] New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

nprez83 avatar Sep 17 '24 02:09 nprez83