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

Implement Overkiz async_execute_commands #79576

Closed

Conversation

nyroDev
Copy link
Contributor

@nyroDev nyroDev commented Oct 4, 2022

Breaking change

Proposed change

While implementing a new entity in PR #78659, I noticed multiple commands was always executing one by one.
This is not usaly a problem for simple entity because they call one command at once.
But for climate entities in the Atlantic/Somfy world, they usually need multiple commands to work correctly.
Doing only 1 call with multiple commands instead of multiple call for each command is great because:

  • it only counts at one api CALL for rate limits
  • it's done this way through the API
  • the canceling could be done for all commands at once
  • it only create 1 history log on overkiz side

The current entity that will benefit this change for now are:

  • somfy_thermostat (3 or 4 calls down to 1)
  • atlantic_heat_recovery_ventilation (3 or 4 calls down to 1)

I will do the changes on other entity when design of the function is correct and somfy_thermostat is validated

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
  • The code has been formatted using Black (black --fast 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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@homeassistant
Copy link
Contributor

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

@nyroDev nyroDev marked this pull request as draft October 4, 2022 07:01
@nyroDev
Copy link
Contributor Author

nyroDev commented Oct 4, 2022

Marked as draft as it's waiting further discussion and implementing details may need to change
previous comment on other PR
#78659 (review)
#78659 (review)

@MartinHjelmare MartinHjelmare changed the title Overkiz: Implement async_execute_commands Implement Overkiz async_execute_commands Mar 2, 2023
MislavMandaric and others added 17 commits October 30, 2023 22:10
…e integration (home-assistant#101196)

* Allow setting hvac mode through set_temperature climate method

* Suggested code simplification when reading hvac mode

Co-authored-by: G Johansson <goran.johansson@shiftit.se>

* Remove unnecessary temperature unit handling from set temperature with hvac mode tests

---------

Co-authored-by: G Johansson <goran.johansson@shiftit.se>
Co-authored-by: Franck Nijhof <git@frenck.dev>
bdraco and others added 26 commits November 7, 2023 19:42
* Bump dbus-fast to 2.13.0

changelog: Bluetooth-Devices/dbus-fast@v2.12.0...v2.13.0

* no change re-release since upload failed due to running out of space on pypi
* add 4 sensors

* no need for extra class
* Fix 5B Gas meter in dsmr

In commit 1b73219 the gas meter broke for 5B.
As the change can't be reverted easily without removing the peak usage
sensors, we implement a workaround.

The first MBUS_METER_READING2 value will contain the gas meter data just
like the previous BELGIUM_5MIN_GAS_METER_READING did.
But this without the need to touch dsmr_parser (version).

Fixes: home-assistant#103306, home-assistant#103293

* Use parametrize

* Apply suggestions from code review

Co-authored-by: Jan Bouwhuis <jbouwh@users.noreply.github.com>

* Add additional tests + typo fix

---------

Co-authored-by: Jan Bouwhuis <jbouwh@users.noreply.github.com>
…3640)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…102967)

* Add support for deleting To-do items in Google Tasks

* Cleanup multipart test

* Fix comments

* Add additional error checking to increase coverage

* Apply suggestions and fix tests
Co-authored-by: Thibaut <thibaut@etienne.pw>
…tiple_commands' into overkiz/execute_multiple_commands
@nyroDev
Copy link
Contributor Author

nyroDev commented Nov 8, 2023

Oops, my merge/rebase messed up the whole PR.
So sorry for all personns that have been notified for nothing.

I'm closing this PR and start a new one to not bother all of you...

@nyroDev nyroDev closed this Nov 8, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2023
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.