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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce ecobee throttle #115968

Merged

Conversation

marcolivierarsenault
Copy link
Contributor

@marcolivierarsenault marcolivierarsenault commented Apr 22, 2024

Proposed change

Ecobee now support, near instant, update on their API.

In the past the API would refresh every 3 min or so, polling it faster was useless. Now, even if thermostat initiated changes are updated every 3 min, the one done via the API are reflected instantly.

In other word. This makes the Home Assistant integration a lot more responsive as changes made on HA side, does not get reverted until the 3 min update. 馃殌

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.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@MartinHjelmare MartinHjelmare changed the title reduce ecobee throttle Reduce ecobee throttle Apr 22, 2024
@MartinHjelmare
Copy link
Member

What's the API request interval limits at the provider?

@MartinHjelmare MartinHjelmare marked this pull request as draft April 22, 2024 14:28
@marcolivierarsenault
Copy link
Contributor Author

marcolivierarsenault commented Apr 22, 2024

@MartinHjelmare
According to their doc:

  1. Other Limitations.
    a. Rate Restrictions. Your Implementation may not request queries from the Services at a rate greater than 1 request per second per thermostat. If your Implementation requests queries at a greater rate, ecobee may throttle or otherwise limit access to the Services by your Implementation.

https://www.ecobee.com/home/developer/api/introduction/licensing-agreement.shtml

That being said, I have never manage to reach it.

Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks!

@MartinHjelmare MartinHjelmare marked this pull request as ready for review April 22, 2024 14:42
@MartinHjelmare MartinHjelmare merged commit 65b2c15 into home-assistant:dev Apr 22, 2024
19 of 20 checks passed
@dxmnkd316
Copy link

dxmnkd316 commented Apr 22, 2024

I would also note that there are separate limits in their terms.

  1. Usage Limits.
    a. Implementations using the Services may at no cost generate up to 85,000 queries ("usage limit"), per month starting on the date you register for developer access ("Registration Date"). More information on how the usage limit is calculated can be found at http://www.ecobee.com/developers. Every month, your Implementation receives a new allotment of 85,000 queries. Unused free queries are not carried over from month to month.

At once every 5 seconds, you'd go through your limit in 5 days. I would not recommend this change if I'm reading this right. Can someone confirm?

Assuming I'm reading this correctly, I wouldn't go faster than every 31-35 seconds. Between reboots, reloads, loss of connections, etc., you could have several other queries each month.

Edit: in fact, 31.5 seconds would use up all queries in 31-day months with just regular queries, not including reloads, etc.

@dxmnkd316
Copy link

dxmnkd316 commented Apr 22, 2024

Also worth noting that the Developers Documentation still states that the API only offers updated data once every three minutes

API USAGE LIMITS

DO NOT poll at an interval quicker than once every 3 minutes, which is the shortest interval at which data might change.
DO NOT have more than 2-3 open HTTP requests with the ecobee API at any given time; wait until the ecobee server has responded before issuing additional API requests.
If you are a Utility or EMS account and you are using management sets as your selection target, DO NOT have more than 1 open HTTP request with the ecobee API at any given time; wait until the ecobee server has responded before issuing additional API requests.

Source: https://developer.ecobee.com/home/developer/api/documentation/v1/operations/get-thermostat-summary.shtml

@marcolivierarsenault
Copy link
Contributor Author

@dxmnkd316 would there be a way to have the throttle at a small number "like few seconds" but limit home assistant "refreshes" to 3 minutes.

The issue is: We want it to be more responsive when we initiate a change from HA.

So
change a setting in HA -> refresh -> get updated value

but the HA fresh when nothing happens can stay 3 min

@MartinHjelmare
Copy link
Member

MartinHjelmare commented Apr 22, 2024

Refractor the integration to use an update coordinator.

https://developers.home-assistant.io/docs/integration_fetching_data#coordinated-single-api-poll-for-data-for-all-entities

For now, I suggest we revert this PR since there were additional limits that we will hit.

@dxmnkd316
Copy link

dxmnkd316 commented Apr 22, 2024

@marcolivierarsenault

Not sure. I find that running both the HomeKit and core integration versions of ecobee is a way to improve response times for a couple of features.

  1. Motion/occupancy changes are instant with HomeKit with a limitation of a few seconds for changing state. I want to say it was like 5 seconds. For instance, I can't trigger a motion detection within 5 seconds of another motion being detected, not that I'd want to anyways.
  2. Sensors might be able to be improved by seconds to minutes, but not reliably. Sometimes you'll be able to see changes in equipment running on either HomeKit or the core integration sooner than the other. You could create a third set of sensors that trigger on any state changes from either integration's sensors. The theoretical maximum is the full 3 minutes, assuming your equipment changes state one second after the last poll of the API and your HomeKit integration updates immediately after. But that's almost certainly too much work for a benefit that's unreliable, variable, and of questionable value.

It's possible that the HomeKit data would allow for more rapid refreshes without a query of the server. But I haven't looked into how HomeKit handles data refreshes, whether they have refresh limits even locally, etc. That would be where I would focus your research efforts after checking whether the server-side data is refreshed more often than 3 minutes. The downside with the HomeKit integration is that it's significantly more limited in the data it provides. Some temperatures are rounded to integers (individual sensors are not), fan status is not available, limited preset modes are available, etc.

As for overwriting sensors created by an integration, I'm less familiar with how the actual code works in that respect for Home Assistant when mixing commands and API requests. I suppose it's theoretically possible, but you might start to get conflicting signals. The last thing I would want is my A/C unit fighting ecobee and Home Assistant causing it to cycle. Nor would I really want automations using that signal if it's going to have a lot of chatter.

But as for the changes in this PR, I think it would be wise to test several (>5) API queries spaced 40 seconds apart to determine if the data is indeed updated more frequently than 180 seconds on the server side.

Again, assuming I read the quota info right from the terms, I don't think anything more frequent than 35 seconds would be advisable regardless. I would hate to have 8,500+ people lose their ecobee dev access for three weeks because they hit their quota.

marcolivierarsenault added a commit to marcolivierarsenault/core that referenced this pull request Apr 22, 2024
@marcolivierarsenault
Copy link
Contributor Author

just opened a PR to revert

@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 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.

None yet

3 participants