Skip to content

Conversation

@erwindouna
Copy link
Contributor

@erwindouna erwindouna commented Jul 8, 2024

Breaking change

Proposed change

Migrate Tado to use Python Tado Async. This is an asynchronous library, aligned with Tado developers to implement a more robust and stable library, dedicated for HA. It gives direct control within the HA community and no third-party library is used anymore.
This is compatible with the current TadoConnector that handles all interaction within HA. Python Tado HA has the same method names to be 100% compatible in the current state (reducing any further refactoring in the integration at this moment).

This is a big draft: users with devices I don't have (AC's, WaterHeater) need to gathered to test their devices.

Type of change

  • Dependency upgrade
  • 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

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • 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:

@home-assistant
Copy link

home-assistant bot commented Jul 8, 2024

Hey there @chiefdragon, mind taking a look at this pull request as it has been labeled with an integration (tado) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of tado can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign tado Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@erwindouna
Copy link
Contributor Author

@EtienneSOU, would you be willing to test out this PR? You're an owner of an air conditioning and it would be great to see if this newly added library handles the AC well.

@EtienneSOU
Copy link
Contributor

Yes, I'll check it but I will be out of my home for a week.

@MartinHjelmare
Copy link
Member

This is compatible with the current TadoConnector that handles all interaction within HA. Python Tado HA has the same method names to be 100% compatible in the current state (reducing any further refactoring in the integration at this moment).

This isn't true since the current connector doesn't await the library methods. The new library has coroutine functions as methods that need to be awaited.

@EtienneSOU
Copy link
Contributor

From what I've checked on Python Tado Async I'll have to rework the fan levels, vertical swings and horizontal swings to make it work with the new bindings.

If we merge it now, all AC users will loose fan and swing modes we've just added.

@erwindouna
Copy link
Contributor Author

From what I've checked on Python Tado Async I'll have to rework the fan levels, vertical swings and horizontal swings to make it work with the new bindings.

If we merge it now, all AC users will loose fan and swing modes we've just added.

It won't be merged any time soon. I need to refactor some more stuff to make the newly added library operational. I'll ping you when I tested it further on my end (meaning the DeviceTracker and Thermostat what I can do). :)

@MartinHjelmare
Copy link
Member

I'd recommend following PEP8 for naming conventions in the new library. Particularly look at the package name and the attribute names of the models. For package names underscore should be avoided and for attribute names they should be lowercase snake_case.

https://peps.python.org/pep-0008/#naming-conventions

If the keys in the JSON are named following camelCase convention you can use field aliases in mashumaro to convert to snake_case in the model attribute names.

https://github.com/Fatal1ty/mashumaro?tab=readme-ov-file#field-aliases

@erwindouna
Copy link
Contributor Author

I'd recommend following PEP8 for naming conventions in the new library. Particularly look at the package name and the attribute names of the models. For package names underscore should be avoided and for attribute names they should be lowercase snake_case.

peps.python.org/pep-0008

If the keys in the JSON are named following camelCase convention you can use field aliases in mashumaro to convert to snake_case in the model attribute names.

Fatal1ty/mashumaro#field-aliases

Done and done.

@ethemcemozkan
Copy link
Contributor

ethemcemozkan commented Sep 1, 2024

@erwindouna Hey as we discussed I cloned your fork and triggered the setup flow using my own tado account.
I received this error(something to do with unsupported DazzleMode) during initial setup which caused initial setup to fail.


2024-09-01 19:49:36.728 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry Hollanda Ev for tado
Traceback (most recent call last):
  File "<string>", line 50, in __mashumaro_from_dict__
  File "<string>", line 9, in __mashumaro_from_dict__
mashumaro.exceptions.MissingField: Field "enabled" of type bool is missing in DazzleMode instance

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/workspaces/ha-core/homeassistant/config_entries.py", line 604, in async_setup
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/ha-core/homeassistant/components/tado/__init__.py", line 83, in async_setup_entry
    await tadoconnector.setup()
  File "/workspaces/ha-core/homeassistant/components/tado/tado_connector.py", line 73, in setup
    self.zones = await self.tado.get_zones()
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/tadoasync/tadoasync.py", line 234, in get_zones
    return [Zone.from_dict(zone) for zone in obj]
            ^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 52, in __mashumaro_from_dict__
mashumaro.exceptions.InvalidFieldValue: Field "dazzle_mode" of type DazzleMode in Zone has invalid value {'supported': False}

I will try to add a workaround for this can continue testing the actual functionality

@ethemcemozkan
Copy link
Contributor

ethemcemozkan commented Sep 1, 2024

Problem happens because of the Hot Water zone, it is not compatible with the library dataclass definition. Returned json item for the hot water zone is:

{'id': 0, 'name': 'Hot Water', 'type': 'HOT_WATER', 'dateCreated': '2022-06-22T10:06:53.818Z', 'deviceTypes': ['BR02', 'SU02'], 'devices': [{...}, {...}], 'reportAvailable': False, 'showScheduleSetup': False, 'supportsDazzle': False, 'dazzleEnabled': False, 'dazzleMode': {'supported': False}, 'openWindowDetection': {'supported': False}}

enabled attribute in the dazzleMode attribute is missing
Edit: After making it optional as a local workaround, found the same problem in open_window_detection, Missing attributes are enabled and timeout_in_seconds. See erwindouna/python-tado#81 for more detail
Edit 2: Successfully added the integration after the workaround but I cannot change climate entities due to /workspaces/ha-core/homeassistant/components/tado/climate.py:703: RuntimeWarning: coroutine 'TadoConnector.reset_zone_overlay' was never awaited self._tado.reset_zone_overlay(self.zone_id)

@emontnemery emontnemery changed the title Refactor to Python Tado HA Migrate tado to tadoasync Oct 16, 2024
@erwindouna
Copy link
Contributor Author

Messed up a rebase. Redoing this in a new PR.

@erwindouna erwindouna closed this Oct 21, 2024
@erwindouna erwindouna deleted the tado_python_tado_ha branch October 21, 2024 17:40
@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants