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

Tellduslive: update_interval is wrong #20169

Closed
kenvase opened this issue Jan 16, 2019 · 13 comments
Closed

Tellduslive: update_interval is wrong #20169

kenvase opened this issue Jan 16, 2019 · 13 comments

Comments

@kenvase
Copy link

kenvase commented Jan 16, 2019

Home Assistant release with the issue:
0.85.1

Last working Home Assistant release (if known):
0.84.x

Operating environment (Hass.io/Docker/Windows/etc.):
ubuntu 18.04

Component/platform:
https://www.home-assistant.io/components/tellduslive/

Description of problem:
Entity state updates are slow. Up to a minute.

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

Traceback (if applicable):

Additional information:
After testing I changed the interval value in init.py like this:

79 interval = timedelta(seconds=5)
80 # interval = timedelta(seconds=entry.data[KEY_SCAN_INTERVAL])
81 _LOGGER.debug('Update interval %s', interval)

Then it works. Looks like it's getting the scan_interval (or something) in seconds(60) instead of update_interval in seconds(5)

@MartinHjelmare
Copy link
Member

CC @fredrike

@fredrike
Copy link
Contributor

fredrike commented Jan 17, 2019

With the configuration integration the custom update interval configuration have been depricated. It is however still configured if you are configuring via configuration.yaml.

I don't see a good way to configure it via configuration integration. Perhaps you have an idea how this should be implemented and shown during configuration and setup.

@kenvase
Copy link
Author

kenvase commented Jan 18, 2019

For me you don't have to be able to configure this. A 5 sec interval will do it "good enough". The major problem now is that it uses 60 sec scan interval for the update interval as well. 60s doesn't cut it if you are using motion sensors for triggering lights etc. I haven't looked at the complete code but I would think that replacing [KEY_SCAN_INTERVAL] on row 79(as I did with a static 5s) in init.py with something like [KEY_UPDATE_INTERVAL] would do the trick. Don't know if that value/parameter exist as is. I might be able to have a look over the weekend but I belive that you are a much better programmer than me ;)

@kenvase
Copy link
Author

kenvase commented Jan 21, 2019

I haven't had time to look at it. To much family ;) . But as it seem 5 sek is to little. it will break HA if there is a lot of entities(>20). I did end up with 10 sek.

@sebba12
Copy link

sebba12 commented Jan 23, 2019

@fredrike the docs https://www.home-assistant.io/components/tellduslive/ says it can be changed?
Why have this changes from 5 to 60, as mentioned above tellduslive will be useless for most of us that have sensors in it.

@fredrike
Copy link
Contributor

fredrike commented Jan 23, 2019

It have always been 60s as default update interval. As you can see here https://github.com/home-assistant/home-assistant/blob/6df5e712f7ab7e4275c7eca09e8494641d5485c2/homeassistant/components/tellduslive.py#L41

And as mentioned in the original post the value is currently set here: https://github.com/home-assistant/home-assistant/blob/c8d885fb78d747478dbd0e5253ff6d9bcf804f9c/homeassistant/components/tellduslive/__init__.py#L79

It can be changed either by editing the .storage/core.config_entries file or by setting up tellduslive via configuration.yaml as you mention @sebba12.

As I've been saying, give me a different reasonable default interval (that doesn't hit the api limits or makes ha unresponsive #20268)

@sebba12
Copy link

sebba12 commented Jan 24, 2019

Okey Fredrik thanks for clearing some things up.
As long it is changeable i don't see a reason for it to change since the API-calls can differ (it is possible to pay for more API request).
So as long it can be changed in configuration.yaml the default can still be 60.
just notic that the doc says Minimal value can’t be less then 300. But the defult is 60 I belive it's a typo?

@xydix
Copy link

xydix commented Mar 5, 2019

Hi. I am about to upgrade from 0.64.3 to 0.88.2
Right now I am using both parallely because there is much work upgrading so many versions and learning lovelace.

One thing i noticed is the Telldus component have problem in 0.88.2
In my old 0.64.3, when i turn a switch on or off, the state change is showed instantly in the hass UI, In 0.88.2, when turnng a switch off, I see the switch turn off, but the state in hass is still on until next poll from the telldus server.
I know this is a polling platform but the old one (0.64.3) changed the state in the UI when i turned on or off. Then if the command failed it showed the previously state after polling. When using the UI to turn a switch on or off, HA should show this change the same second i press the button.

@fredrike
Copy link
Contributor

fredrike commented Mar 6, 2019

@molobrakos Do you remember what you did to make the changes show up instantly?

Edit: It is due to the removal of self.changed()#18780 (comment) , I will make some tests of how to make it more responsive.

@fredrike
Copy link
Contributor

fredrike commented Mar 6, 2019

@xydix Create a new issue with the current description and I'll ship a fix asap.

@fredrike
Copy link
Contributor

fredrike commented Mar 6, 2019

I think this should be closed by the documentation added in home-assistant/home-assistant.io#8481

@kenvase
Copy link
Author

kenvase commented Mar 6, 2019

I close it now

@kenvase kenvase closed this as completed Mar 6, 2019
@xydix
Copy link

xydix commented Mar 6, 2019

@fredrike No need for a new issue, right? If I understand things right you solved it?

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

No branches or pull requests

6 participants