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

[LIFX] Update ThingStatus at most once a second #3816

Merged
merged 2 commits into from Jul 10, 2017

Conversation

Projects
None yet
3 participants
@wborn
Copy link
Contributor

commented Jul 7, 2017

With this PR the LifxLightHandler will only update the ThingStatus when there is a change.

Each received message currently always results in a ThingStatus.ONLINE update. This causes a lot of unnecessary ThingStatusInfoEvent s, logging etc. which generate unnecessary CPU load.

Update LIFX ThingStatus only when changed
Signed-off-by: Wouter Born <eclipse@maindrain.net>
@kaikreuzer

This comment has been minimized.

Copy link
Member

commented Jul 8, 2017

It isn't recommended for bindings to do this check. The framework already checks whether there is a change or not and actually issues different events (updated vs changed). The logging can be configured to only log change events (that's the default in openHAB), so additional updates should do no harm.
By filtering it, you now suppress the information on when the light has been seen online last (we do not show this anywhere yet, but I would like to have such a feature in future).

So I vote against this PR, sorry 😎

@wborn

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

When my 38 lights just remain online the light state is continuously polled. Each received message results in an online update. This PR saves with my 38 lights about ~1535 updates per minute.

When you start sending commands to your lights for some animation this number will rapidly increase because more messages are received.

The online/offline status works very well for LIFX lights. So if you want to know when the light was last online you can still use the transition from online to offline for this.

@kaikreuzer

This comment has been minimized.

Copy link
Member

commented Jul 8, 2017

~1535 updates per minute.

Ok, that's a lot indeed :-)

you can still use the transition from online to offline for this

Not, if it hasn't gone offline at all. But ok, if you say that an ONLINE status cannot be older than a second in any case (as otherwise it would have gone OFFLINE already), my argument probably does not matter to much here.
Nonetheless: With your change, we would see a "last ONLINE message" which has a date from when it went online instead of "now-1s". So could you find a way to at least regularly do a setStatus, even if it hasn't changed? Doing it once a second would still reduce the number of messages from 1535 to only 38 :-)

Use 1 second interval for unchanged ThingStatusInfo updates and rewor…
…k code to use Java 8 Durations

Signed-off-by: Wouter Born <eclipse@maindrain.net>
@wborn

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

@kaikreuzer with the new commit it will now again allow for same status updates with a 1 second interval. I've also replaced some durations to use the Java 8 Duration so the units are more clear.

@kaikreuzer
Copy link
Member

left a comment

Nice, thanks!

@kaikreuzer kaikreuzer changed the title Update LIFX ThingStatus only when changed [LIFX] Update ThingStatus at most once a second Jul 9, 2017

@sjka sjka merged commit 6b91d19 into eclipse:master Jul 10, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ip-validation
Details

@wborn wborn deleted the wborn:lifx-thingstatus branch Jul 11, 2017

@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

@kaikreuzer kaikreuzer added the bug label Dec 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.