sidoh / esp8266_milight_hub

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

Inconsistency in REST API for night mode #554

Closed Zer0x00 closed 4 years ago

Zer0x00 commented 4 years ago

Describe the bug

If you trigger the night mode via REST API the "state" property is left on "ON". If you trigger the night mode via remote (FUT089) the "state" property is set to "OFF".

Steps to reproduce

1, PUT request to http://milight-hub.local/gateways/0x91E6/fut089/2 with the body

{  
    "effect":"night_mode"
}

1.1. GET request to http://milight-hub.local/gateways/0x91E6/fut089/2?blockOnQueue=true Return value:

{
    "state": "ON",
    "brightness": 0,
    "bulb_mode": "night",
    "color": {
        "r": 255,
        "g": 255,
        "b": 255
    }
}
  1. Trigger night mode via FUT089 remote (long press off-button)

2.1 GET request to http://milight-hub.local/gateways/0x91E6/fut089/2?blockOnQueue=true Return value:

{
    "state": "OFF",
    "brightness": 0,
    "bulb_mode": "night",
    "color": {
        "r": 255,
        "g": 255,
        "b": 255
    }
}

Expected behavior

In both situations the state should be "ON".

A fix could maybe be implemented like this (pseudocode):

if(bulb_mode === "night"){
    state = "OFF"
}

Setup information

Firmware version

1.10.4

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

{ 
   "firmware":"milight-hub",
   "version":"1.10.4",
   "ip_address":"192.168.0.102",
   "reset_reason":"Software/System restart",
   "variant":"nodemcuv2",
   "free_heap":17872,
   "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":"",
   "mqtt_username":"",
   "mqtt_password":"",
   "mqtt_topic_pattern":"",
   "mqtt_update_topic_pattern":"",
   "mqtt_state_topic_pattern":"",
   "mqtt_client_status_topic":"",
   "simple_mqtt_client_status":false,
   "discovery_port":48899,
   "listen_repeats":3,
   "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":"On",
   "led_mode_operating":"Off",
   "led_mode_packet":"Flicker",
   "led_mode_packet_count":3,
   "hostname":"milight-hub",
   "rf24_power_level":"MAX",
   "rf24_listen_channel":"LOW",
   "wifi_static_ip":"192.168.0.102",
   "wifi_static_ip_gateway":"192.168.0.1",
   "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",
      "HIGH"
   ],
   "device_ids":[ 
      37350
   ],
   "gateway_configs":[ 

   ],
   "group_state_fields":[ 
      "state",
      "brightness",
      "mode",
      "color_temp",
      "bulb_mode",
      "computed_color"
   ],
   "group_id_aliases":{ 
      "Wohnzimmer":[ 
         "fut089",
         37350,
         2
      ]
   }
}
sidoh commented 4 years ago

What actual problem is this presenting?

Right now, night mode is decoupled from state. It honestly behaves more like its own state than it does an effect: bulbs in night mode doesn't respond to commands (as if it were off), but the LEDs aren't completely off (as if it were on). But it was more convenient to represent it as a separate field.

The reason the remote results in status=OFF is because long-pressing the off button results in an status=OFF packet before it emits a night mode packet (check sniffer logs).

So I'd file this under "expected behavior" given my understanding.

Zer0x00 commented 4 years ago

Thank you for your assitance. I am working on a homebridge integration via REST API which has to turn on the bulb depending on the state if the user wants to control the brightness. If there's a inconsistency I would have to check also the bulb mode and send an on-command depending on this. Otherwise I could just send an on command if the bulb is off. Hope that the problem here is clear.

You are right, since the remote sends first a off-command the state gets off and after the night mode packet is sent.

The WebUI should mimic this behaviour imho. So if the night mode is activated the state should also be set to offline because you can't control the bulbs without sending an on packet.

sidoh commented 4 years ago

An alternative is to just always send state=ON when you're trying to change state. HomeAssistant's builtin MQTT integration does this, for example. The only cost is that an extra packet is sent. Given that there's no way to guarantee that bulbs receive commands, this is more robust anyway.

State handling around night mode is already far more complex than I'd like for it to be, and I'm very hesitant to add more special casing.

Zer0x00 commented 4 years ago

Alright, I'll implement it like HomeAssistant does.