sidoh / esp8266_milight_hub

Replacement for a Milight/LimitlessLED hub hosted on an ESP8266
MIT License
932 stars 219 forks source link

MQTT state not sent after command topic #700

Open Erriez opened 3 years ago

Erriez commented 3 years ago

Describe the bug

A light state may be out of sync between Homeassistant - MiLight-Hub. In this case the light on/off button in Homeassistant flips back to the previous state after a few seconds.

Root cause: MiLight hub does not send a MQTT state topic when it receives a command topic without a change.

A possible solution would be send the light state always after each light command.

Steps to reproduce

MiLightHubMQTTNotSendingState2

Expected behavior

Always send a MQTT state after a command. Consider to make this permanent, or configurable.

Setup information

Firmware version

See next.

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

{
  "firmware": "milight-hub",
  "version": "1.10.7",
  "ip_address": "10.0.0.206",
  "reset_reason": "Power on",
  "variant": "d1_mini",
  "free_heap": 17016,
  "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": 0,
  "csn_pin": 15,
  "reset_pin": 0,
  "led_pin": -2,
  "radio_interface_type": "nRF24",
  "packet_repeats": 100,
  "http_repeat_factor": 1,
  "auto_restart_period": 0,
  "mqtt_server": "10.0.0.249",
  "mqtt_username": "****",
  "mqtt_password": "****",
  "mqtt_topic_pattern": "milight/cmd/:device_type/:device_id/:group_id",
  "mqtt_update_topic_pattern": "",
  "mqtt_state_topic_pattern": "milight/state/:device_type/:device_id/:group_id",
  "mqtt_client_status_topic": "",
  "simple_mqtt_client_status": false,
  "discovery_port": 48899,
  "listen_repeats": 1,
  "state_flush_interval": 10000,
  "mqtt_state_rate_limit": 500,
  "mqtt_debounce_delay": 500,
  "mqtt_retain": true,
  "packet_repeat_throttle_sensitivity": 0,
  "packet_repeat_throttle_threshold": 200,
  "packet_repeat_minimum": 3,
  "enable_automatic_mode_switching": false,
  "led_mode_wifi_config": "Flicker",
  "led_mode_wifi_failed": "On",
  "led_mode_operating": "Off",
  "led_mode_packet": "Fast blip",
  "led_mode_packet_count": 2,
  "hostname": "milight-hub",
  "rf24_power_level": "MAX",
  "rf24_listen_channel": "LOW",
  "wifi_static_ip": "10.0.0.206",
  "wifi_static_ip_gateway": "10.0.0.138",
  "wifi_static_ip_netmask": "255.255.255.0",
  "packet_repeats_per_loop": 10,
  "home_assistant_discovery_prefix": "",
  "wifi_mode": "n",
  "default_transition_period": 500,
  "rf24_channels": [
    "LOW",
    "MID",
    "HIGH"
  ],
  "device_ids": [
    21937
  ],
  "gateway_configs": [],
  "group_state_fields": [
    "state",
    "brightness",
    "color",
    "mode",
    "color_temp"
  ],
  "group_id_aliases": {
    "FUT089": [
      "fut089",
      21937,
      0
    ]
  }
}

Homeassistant configuration.yaml

light:
  - name: MiLight FUT089 0x55B1 Zone 1
    unique_id: milight_fut089_0x55B1_1
    command_topic: "milight/cmd/fut089/0x55B1/1"
    state_topic: "milight/state/fut089/0x55B1/1"
    <<: *MILIGHT_RGBCCT_PARAMS

    # Use YAML anchor for common settings for other lights.
    <<: &MILIGHT_RGBCCT_PARAMS
      platform: mqtt
      schema: json
      brightness: true
      color_temp: true
      rgb: true
      effect: true
      effect_list: 
        - night_mode
        - white_mode

Additional context

See screen capture above.

Erriez commented 3 years ago

Removing the commented optimization below looks like a workaround in lib/MiLightState/GroupState.cpp:

bool GroupState::setState(const MiLightStatus status) {
  //if (!isNightMode() && isSetState() && getState() == status) {
  //  return false;
  //}

  setDirty();
  ...

In this case the state topic is always sent, even when MiLight-Hub thinks there is no light change. Then both Homeassistant and MiLighthub becomes in sync about every light state and the light button does not flip back.

Can someone review this proposal? Then I'm happy to create a pull request.

poudenes commented 3 years ago

Nice, noticed this bug also... not always. Only when reboot HA and my lights went on even when they where off in HA Maybe this happen because of above issue.

Erriez commented 3 years ago

Nice, noticed this bug also... not always. Only when reboot HA and my lights went on even when they where off in HA Maybe this happen because of above issue.

You can give it a try to open the MiLight-Hub project in VCode, comment the lines in my previous comment and flash it to the board.

Yesterday I noticed that Homeassistant also contains a Scene optimization that it won't send a light state when it thinks the light is already in a specific state. This also results in lights out of sync after a restart or missing packets. I'm not sure if this optimization can be disabled. Switching to another scene is only reliable when all lights are always refreshed.

poudenes commented 3 years ago

Nice, noticed this bug also... not always. Only when reboot HA and my lights went on even when they where off in HA Maybe this happen because of above issue.

You can give it a try to open the MiLight-Hub project in VCode, comment the lines in my previous comment and flash it to the board.

Yesterday I noticed that Homeassistant also contains a Scene optimization that it won't send a light state when it thinks the light is already in a specific state. This also results in lights out of sync after a restart or missing packets. I'm not sure if this optimization can be disabled. Switching to another scene is only reliable when all lights are always refreshed.

I really dont know how to do that. I will wait for a possible update. I have tried this kind of tweaks in past, but always brick my things haha

clau-bucur commented 3 years ago

I was just trying to make an HA automation so when I change the state of the "All / 0" group, all the lights to adopt that same state. It was not working however, the changes were not propagating correctly. Now with this change, all the lights follow the All group successfully:

bool GroupState::setState(const MiLightStatus status) {
  //if (!isNightMode() && isSetState() && getState() == status) {
  //  return false;
  //}

  setDirty();
  ...

My automation:

- alias: Mi / Bedroom / Follow
  trigger:
    platform: state
    entity_id: light.mi_bedroom
  action:
    choose:
      - conditions: "{{ is_state(trigger.entity_id, 'off') }}"
        sequence:
          service: light.turn_off
          entity_id:
            - light.mi_bedroom_clau
            - light.mi_bedroom_miha

    default:
      service: light.turn_on
      entity_id:
        - light.mi_bedroom_clau
        - light.mi_bedroom_miha

What's more is that I don't have to manually specify the attributes (brightness, rgb, etc), they are automatically copied (not sure how though but I don't really care now).