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

Add option to debounce MQTT state updates #615

Closed
annego15 opened this issue Apr 13, 2020 · 2 comments · Fixed by #617
Closed

Add option to debounce MQTT state updates #615

annego15 opened this issue Apr 13, 2020 · 2 comments · Fixed by #617

Comments

@annego15
Copy link
Contributor

annego15 commented Apr 13, 2020

Describe the bug

When I send a command over MQTT with multiple changes (for example hue and level), multiple status messages get sent, with the parameters changing one by one. That means, when I change both level and hue, the level gets changed first and a status message is sent with the old hue and just after that the full new status is sent. This is a problem, because it leads to jumping values in my control dashboard, as the old hue gets pushed as a update after I changed it with a command.

Steps to reproduce

send MQTT 1: {"state":"ON","level":95,"hue":250}
send MQTT 2: {"state":"ON","level":90,"hue":240}

received status messages after MQTT 2:
1: {"state":"ON","level":95,"hue":250,"saturation":100,"bulb_mode":"color"}
2: {"state":"ON","level":90,"hue":240,"saturation":100,"bulb_mode":"color"}

Expected behavior

Don't send status message 1 and send only 2 (only the full status update)

expected message after MQTT send 2:
{"state":"ON","level":90,"hue":240,"saturation":100,"bulb_mode":"color"}

Setup information

MQTT messages normally sent with Openhab, but also tested with MQTT.fx client.

Firmware version

1.10.5 (d1_mini)

Output of http://milight-hub.local/about

{"firmware":"milight-hub","version":"1.10.5","ip_address":"192.168.1.2","reset_reason":"External System","variant":"d1_mini","free_heap":17704,"arduino_version":"2_4_2","queue_stats":{"length":0,"dropped_packets":0}}

Output of http://milight-hub.local/settings

{"admin_username":"","admin_password":"","ce_pin":4,"csn_pin":15,"reset_pin":0,"led_pin":-2,"radio_interface_type":"nRF24","packet_repeats":50,"http_repeat_factor":1,"auto_restart_period":0,"mqtt_server":"192.168.1.141:1883","mqtt_username":"****","mqtt_password":"****","mqtt_topic_pattern":"milight/commands/:device_id/:device_type/:group_id","mqtt_update_topic_pattern":"","mqtt_state_topic_pattern":"milight/states/:device_id/:device_type/:group_id","mqtt_client_status_topic":"","simple_mqtt_client_status":false,"discovery_port":48899,"listen_repeats":0,"state_flush_interval":10000,"mqtt_state_rate_limit":500,"packet_repeat_throttle_sensitivity":0,"packet_repeat_throttle_threshold":200,"packet_repeat_minimum":3,"enable_automatic_mode_switching":false,"led_mode_wifi_config":"Fast toggle","led_mode_wifi_failed":"Fast blip","led_mode_operating":"On","led_mode_packet":"Flicker","led_mode_packet_count":3,"hostname":"milight-hub","rf24_power_level":"MAX","rf24_listen_channel":"LOW","wifi_static_ip":"","wifi_static_ip_gateway":"","wifi_static_ip_netmask":"","packet_repeats_per_loop":10,"home_assistant_discovery_prefix":"","wifi_mode":"n","default_transition_period":500,"rf24_channels":["LOW","MID","HIGH"],"device_ids":[0,1,2],"gateway_configs":[],"group_state_fields":["state","level","hue","saturation","mode","color_temp","bulb_mode"],"group_id_aliases":{"Terrasse Hinten":["rgb_cct",0,1]}}
@annego15 annego15 added the bug label Apr 13, 2020
@annego15 annego15 changed the title Status message MQTT gets sent multiple times after a single command Status message MQTT get sent multiple times after a single command Apr 13, 2020
@annego15 annego15 changed the title Status message MQTT get sent multiple times after a single command Status message MQTT gets sent multiple times after a single command Apr 13, 2020
@sidoh
Copy link
Owner

sidoh commented Apr 14, 2020

Makes sense, and thanks for the detailed report.

I'm going to re-categorize this as an enhancement. MQTT state updates were designed to work to work this way, so the behavior you're describing is expected (although not desirable).

To elaborate a bit:

  1. State updates are rate limited by a simple global throttle
  2. For important architectural reasons, each packet that is sent or received produces a state update
  3. Therefore, if you issue a command that results in more than one packet, you'll get at least two state updates (with the last one guaranteed to reflect the final state)

The path forward is to either:

  1. Add a global debounce interval: don't send out state updates until the newest packet is older than a certain threshold. This is simpler, but would mean that issuing a bunch of commands for different devices all at once would cause state updates to lock up until sends quiet down.
  2. Add a bulb-specific debounce threshold. This probably has the actual desired behavior, but is a bit more complex.

It's come up before (see #576), so makes sense that this should be an option (and probably the default).

I won't have the chance to look at this for some time. If anyone else wants to take a crack, the relevant class is BulbStateUpdater. Option (1) is probably a pretty quick change.

@sidoh sidoh added enhancement and removed bug labels Apr 14, 2020
@sidoh sidoh changed the title Status message MQTT gets sent multiple times after a single command Add option to debounce MQTT state updates Apr 14, 2020
@annego15
Copy link
Contributor Author

annego15 commented Apr 14, 2020

Thank you for the comprehensive answer! I tried my best to add a global debounce delay for the MQTT state updates. (See #617) For me it works perfectly now.

Thank you for your work!

sidoh pushed a commit that referenced this issue Apr 18, 2020
Adds a setting that delays state updates from being sent until an update hasn't occurred for a configurable threshold

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

Successfully merging a pull request may close this issue.

2 participants