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

Set DataCoordinator update interval to 10 seconds #148

Merged
merged 1 commit into from
Feb 21, 2022
Merged

Set DataCoordinator update interval to 10 seconds #148

merged 1 commit into from
Feb 21, 2022

Conversation

ehendrix23
Copy link
Contributor

Currently the DataCoordinator is set to call the update for the integration every minute, this change is to change it to 10 seconds instead.
There are many integrations within HASS that are called every 10 seconds.

With the polling interval, the API calls to Tesla are not increased unless one sets the polling interval to less than 1 minute. When the update call is made but the polling interval is not expired it would not call the Tesla API.
Further, nothing will be recorded in HASS either as long as there is no state change. So this change will not increase for example DB size or so for HASS. It would just add a bit off overhead which is very minimal.

Advantage of this change is that during certain events (i.e. driving) the polling interval can be set to lower then 1 minute and one can then get more up-to-date data.
Note, TeslaFi for example polls every 10 seconds.

@alandtse
Copy link
Owner

I'm not sure I think this should be a default settings this low. Is there a reason you can't just manually adjust this at your end if you want it this low?

@alandtse
Copy link
Owner

@Olen thoughts on this or perhaps we open the minimum as an option?

@alandtse
Copy link
Owner

Hmm. I just realized that we only use this to clamp the minimum value. Perhaps it's worth allowing it. To be honest, if people wanted this frequency, the correct answer is websockets. That was providing 4 updates per seconds while driving if I recall.

@ehendrix23
Copy link
Contributor Author

websockets

Based on comments I saw following that it seems that Tesla only allows 1 client for streaming and it would thus break the app (or the integration broken if the app takes the stream).

FYI, if the state does not change then nothing is added in HASS irrespective on number of times polled. So this would not add any more records in HASS unless the state has changed.

@alandtse
Copy link
Owner

The concern is polling the Tesla servers so much that they decide to ban third party apps. We're already not officially endorsed. The fact Teslamate caused a worldwide reset of tokens is already not a good sign.

@ehendrix23
Copy link
Contributor Author

The concern is polling the Tesla servers so much that they decide to ban third party apps. We're already not officially endorsed. The fact Teslamate caused a worldwide reset of tokens is already not a good sign.

That’s why I put it for 10 seconds and not less as TeslaFi polls every 10 seconds as well.

@Olen
Copy link
Contributor

Olen commented Feb 21, 2022

I have also used 10 seconds in a personally developed app I used for a long time without any problem. However, I do understand Alans concern. I don't know anything about the size of the user base of TeslaFi, Teslamate and Tesla in HA, so I don't know of changing the interval to 10 seonds here will even be noticeable on Teslas servers.

With that said, I think it is ok to change the minimum to 10 seconds, and use the new service call to adjust the actual polling interval dynamically.

Also, I have not looked at the code of the official Tesla APP for many years, but from watching its behaviour, I believe it uses websockets as soon as the car wakes up. If it is correct (and I have not verified this myself) that the websocket API only accepts one client at a time, this is a showstopper for using websockets in HA as long as users might want to use the offical APP in addition to HA. A simple test here would be to open the APP on two or more phones at the same time, and see if they both get live data from a car that is driving or charging.

@alandtse
Copy link
Owner

Assuming all downloads at the latest are active we're at 2.7K users. Probably a drop in the bucket. Since this is just a clamp, I'm ok with it.

@alandtse alandtse merged commit 0d87757 into alandtse:dev Feb 21, 2022
@alandtse
Copy link
Owner

@ehendrix23 I know you have a few more changes coming, let me know when you want are ready to spawn a release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants