Add ecobee set_sensors_used_in_climate service

[myztillx]

Breaking change

Proposed change

Ecobee allows for setting sensors active for programs. This pull request adds a service call which will set the active sensors for a specific program. The reason this can be useful is if you know sensors will need to be participating at a specific time. My use case is I have a room that is poorly insulated. This room is rarely used unless guests are coming. If there is a calendar event for guests, I can automatically include this sensor in programs without having to do it manually through the ecobee app.

I have tested these changes with my own thermostats and remote sensors. Though I am not sure how to write tests within HomeAssistant.

This PR is dependent on https://github.com/home-assistant/core/pull/102900 for the version upgrade.

Type of change

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [X] 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: https://github.com/home-assistant/home-assistant.io/pull/29612

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.
• [ ] I have followed the development checklist
• [ ] I have followed the perfect PR recommendations
• [x] 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:

• [X] Documentation added/updated for www.home-assistant.io

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

• [x] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [X] 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.

To help with the load of incoming pull requests:

• [ ] I have reviewed two other open pull requests in this repository.

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[home-assistant[bot]]

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

Code owner commands

Code owners of ecobee 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 ecobee Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[myztillx]

This is my first pull request on Home Assistant that includes an integration update and documentation change. Please let me know if I did anything incorrect during the submission of this request, and I will correct it.

[myztillx]

PR https://github.com/home-assistant/core/pull/102900 merged. Changing from draft and merge from 'dev'.

[marcolivierarsenault]

🎩 the pr, works great,

Waiting for the renaming

[myztillx]

Updated naming as @marcolivierarsenault suggested. Went with set_sensors_used_in_climate as I felt it was the most descriptive of what is actually happening.

[myztillx]

I think I accidentally requested another review from Marc-Olivier. Sorry about that...

[myztillx]

@marthoc and @marcolivierarsenault, are there any other changes that need to be made to this PR for approval?

[myztillx]

@edenhaus it seems that the CI is failing on code that is unchanged by this PR. Do I need to try and fix all those as well?

[edenhaus]

@edenhaus it seems that the CI is failing on code that is unchanged by this PR. Do I need to try and fix all those as well?

The following test are not failing on the dev, so it means your PR is the reason why they are failing now. Please give it a look and make sure the CI and specially all tests for ecobee are passing.

[myztillx]

It seems that the tests really did not like the .get() method for some reason... I have modified the code to pass the tests. It is a little more verbose now, but I think I retained all the safety as before.

I tested the new code and it works as before for actually setting the participating sensors.

[myztillx]

@edenhaus is there anything else needed for this PR?

[myztillx]

Is there anything left to get this PR merged?

[myztillx]

@edenhaus sorry for the ping, I just really want to get this PR merged. What is still necessary to get this merged?

[myztillx]

@edenhaus are you able to approve this, or does more work need to be completed?

[myztillx]

Made requested changes.

[myztillx]

The ecobee climate platform has tests, in tests/components/ecobee/test_climate.py

Please add tests which cover the new functionality.

@emontnemery, is this something you would be able to assist with? I am pretty new to the home assistant code base and coding in general.

I can see how I would add a test for the remote_senors property. I am unsure how I would be able to add one for the set_sensors_used_in_climate method. This is because it calls the device registry of home assistant to get a list of the registered sensors and their names.

[myztillx]

Not sure how to handle a test where the function call accesses the device registry.

[emontnemery]

The ecobee climate platform has tests, in tests/components/ecobee/test_climate.py Please add tests which cover the new functionality.

@emontnemery, is this something you would be able to assist with? I am pretty new to the home assistant code base and coding in general.

I can see how I would add a test for the remote_senors property. I am unsure how I would be able to add one for the set_sensors_used_in_climate method. This is because it calls the device registry of home assistant to get a list of the registered sensors and their names.

Please message me on discord (I'm @emontnemery there too) 👍

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!

[myztillx]

Not stale, just haven't had time to get assistance on writing tests.

[frenck]

Drafting this PR, as there is a merge conflict + changes requested that haven't been implemented at the time of writing.

../Frenck

[emontnemery]

Not stale, just haven't had time to get assistance on writing tests.

Alright, when you have the time, contact me on Discord and we'll get it sorted 👍

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!

[myztillx]

Sorry for the long delay, I will have some time next week to work on this.

[myztillx]

I was working with emontnemery and he updated the test helper in #126587. I am not sure about how all that works, but I had to undo that change because it seems that the ecobee remote sensors were not added to the device registry with the change.

[myztillx]

After talking with emontnemery, he assisted in getting the correct test common file. Also added some negative tests at his request.

[emontnemery]

Please mark the PR ready for review when the comments have been addressed

[emontnemery]

@myztillx As discussed on Discord, some additional tweaks are needed before we can merge the PR. Please mark the PR ready for review once done 👍