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

rest api incorrectly reports light is turned off after setting bri=0 #1111

Closed
ErnstEeldert opened this issue Jan 8, 2019 · 8 comments
Closed
Labels

Comments

@ErnstEeldert
Copy link

ErnstEeldert commented Jan 8, 2019

When I set bri=0 state, the state on=false is set. But the light itself is still on at minimal brightness. Like so:

$ PUT /lights/47/state '{ "on": true, "bri": 255, "transitiontime": 0 }' && GET /lights/47 | jq .

[{"success":{"/lights/47/state/on":true}},{"success":{"/lights/47/state/bri":255}}]
{
  "ctmax": 454,
  "ctmin": 250,
  "etag": "edacb59cfb66980bc4aabe521d89ab8e",
  "hascolor": true,
  "manufacturername": "IKEA of Sweden",
  "modelid": "TRADFRI bulb E27 WS opal 980lm",
  "name": "Kantoor Dressoir",
  "state": {
    "alert": "none",
    "bri": 255,
    "colormode": "ct",
    "ct": 454,
    "on": true,
    "reachable": true
  },
  "swversion": "1.2.217",
  "type": "Color temperature light",
  "uniqueid": "00:0b:57:ff:fe:f3:56:3c-01"
}

$ PUT /lights/47/state '{ "bri": 0, "transitiontime": 40 }' && GET /lights/47 | jq .

[{"success":{"/lights/47/state/bri":0}}]
{
  "ctmax": 454,
  "ctmin": 250,
  "etag": "f7a9d8dbe2caff44e568613a82d8aebd",
  "hascolor": true,
  "manufacturername": "IKEA of Sweden",
  "modelid": "TRADFRI bulb E27 WS opal 980lm",
  "name": "Kantoor Dressoir",
  "state": {
    "alert": "none",
    "bri": 0,
    "colormode": "ct",
    "ct": 454,
    "on": false,
    "reachable": true
  },
  "swversion": "1.2.217",
  "type": "Color temperature light",
  "uniqueid": "00:0b:57:ff:fe:f3:56:3c-01"
}

The behavior is the same regardless if I specify the transitiontime or not on the bri=0 state update.

@olemr
Copy link

olemr commented Jan 8, 2019

I have found the same. Also the API returns an error if you try to manipulate 'bri' in the 'off' state.
This means you have to control 'on'/'off'/'bri' 'manually' and be careful not to send commands too close in time. If you do, the bulb may hang.

@ebaauw
Copy link
Collaborator

ebaauw commented Jan 22, 2019

Did some sniffing how the Hue bridge handles state.on and state.bri. It would seem they changed it; at least it looks very different from what I think I remember. Square bridge on the lastest firmware, swversion: 1811120916 and apiversion: 1.29.0. Hue LTW010 color temperature light on the latest firmware, swversion: 1.46.13_r26312.

Body Command(s)
{"on": false} Off with Effect (0, 0)
{"on": false, "transitiontime": t} Move to Level (with On/Off) (0, t)
{"on": false", "bri": b} Move to Level (b, 4)
Off with Effect (0, 0)
{"on": false", "bri": b, "transitiontime": t}
(bri seems to be ignored)
Move to Level (with On/Off) (0, t)
{"on": true} On
{"on": true, "transitiontime": t }
(transitiontime seems to be ignored)
On
{"on": true, "bri": b}
(when state.on false)
Move to Level (with On/Off) (2, 0)
Move to Level (b, 4)
{"on": true, "bri": b}
(when state.on is true)
On
Move to Level (b, 4)
{"on": true", "bri": b, "transitiontime": t}
(when state.on false)
Move to Level (with On/Off) (2, 0)
Move to Level (b, t)
{"on": true", "bri": b, "transitiontime": t}
(when state.on is true)
On
Move to Level (b, t)
{"bri": b} Move to Level (b, 4)
{"bri": b, "transitiontime": t} Move to Level (b, t)

When "bri": 0 is specified, the Move to Level parameter Level is set to 0 (but the light remains on, and state.on remains true, and state.bri returns 1); when "bri": 255 is specified, Level is set to 254. An explicit transitiontime of 4 is handled differently than a missing transitiontime. I see no special handling of transitiontime of 0.

While not intuitive, this behaviour makes total sense to me, except when turning the light off with a transitiontime. The light comes back on at bri 1 instead of at the previous or specified bri. It might be better to ignore the transitiontime when bri is not specified (similar to setting the light on) and to send `Move to Level (b, t), followed by an Off with Effect (0, 1) when it is.

Interesting: when state.on is false when setting state.bri, the REST API returns error: 201 parameter, bri, is not modifiable. Device is set to off., but the bridge still sends the Move to Level command.

EDIT
The same commands seem to be sent when setting a group action, except that the group's state.all_on and state.any_on aren't considered. To turn the lights on, the On command is sent always, never the Move to Level (with On/Off) (2, 0).

@Thomas-Vos
Copy link
Contributor

I also noticed that when sending the following body to /lights/<lightid>/state, deCONZ will switch off the light instead of setting it to the lowest brightness.

{
  "on": true,
  "bri": 1
}

If you remove the on attribute or set bri to 2 instead, then it works correctly and the light is not switched off.

@stale
Copy link

stale bot commented May 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@ebaauw
Copy link
Collaborator

ebaauw commented May 25, 2019

@Thomas-Vos, Hue lights turn off when receiving a Move to Level (with On/Off) (1, t). From the ZCL spec (section 3.10.2.4.5):

If any command that decreases CurrentLevel reduces it to the minimum level allowed by the device, the OnOff attribute of the On/Off cluster on the same endpoint, if implemented, SHALL be set to Off.

I suppose Hue treats 1 as the minimum allowed level, even though CurrentLevel can take values 0 to 254.

The Hue bridge handles {"on": true, "bri": 1} by sending a Move to Level (with On/Off) (2, 0) to turn the light on at minimum brightness, and then a Move to Level (1, 4) to set the brightness to 1 (see above table). The deCONZ REST API plugin sends a single Move to Level (with On/Off) (1, t).

@manup I've been meaning to refactor setLightState() (and set GroupState()), but I'm not sure blindly copying the Hue bridge logic is the best way for non-Hue lights. We know there are lights that don't handle Move to Level (with On/Off) correctly (see e.g. #1205).

We could whitelist the different light types and/or manufacturers and adapt the logic accordingly (as is done for ct for the IKEA Trådfri colour bulb), but that will lead to issues with groups (see #1479). We could use the same whitelist for homogeneous groups (provided we keep track of the light types and/or manufacturers in a group), but there's no solution for heterogenous groups.

We definitely need to apply "on": true first (before setting brightness, colour (temperature), or effect) and "on": false last (so you can set the brightness and colour (temperature) the light should turn on to next time). We might provide a safety net, by sending Move to Level (with On/Off) (2, 0) followed by On, followed by Move to Level (b, t), but that is an additional ZigBee message.

In pseudo code (with whitelisting):

    const tt = "transitiontime": t ? t : 4;
    if ("on": true) {
        if (Hue-like && !isOn) {
            Move_to_Level_with_On_Off(2, 0);
        } else {
            On();
        }
        isOn = true;
    }
    if ("bri": b) {
        if (!isOn) {
            Error();
        }
        Move_to_Level(b, tt);
    }
    // handle `xy`, `ct`, `hs`, `_inc` variants, `effect`, `alert`, etc
    if ("on": false) {
        if (Hue-like) {
            Off_with_Effect(0, 0);
        } else {
            Off();
        }
    }

or without whitelisting:

    const tt = "transitiontime": t ? t : 4;
    if ("on": true) {
        Move_to_Level_with_On_Off(2, 0);
        On();
        isOn = true;
    }
    if ("bri": b) {
        if (!isOn) {
            Error();
        }
        Move_to_Level(b, tt);
    }
    // handle `xy`, `ct`, `hs`, `_inc` variants, `effect`, `alert`, etc
    if ("on": false) {
        Off_with_Effect(0, 0);
        Off();
    }

@manup
Copy link
Member

manup commented May 25, 2019

We could whitelist the different light types and/or manufacturers and adapt the logic accordingly (as is done for ct for the IKEA Trådfri colour bulb), but that will lead to issues with groups (see #1479). We could use the same whitelist for homogeneous groups (provided we keep track of the light types and/or manufacturers in a group), but there's no solution for heterogenous groups.

Yes this might turn out really tricky, albeit possible my main concern here would be poor scaling. With the ever crowing network sizes > 100, unicast messages won't be perceived well. I think we should explore the groupcast path first.

We definitely need to apply "on": true first (before setting brightness, colour (temperature), or effect) and "on": false last (so you can set the brightness and colour (temperature) the light should turn on to next time). We might provide a safety net, by sending Move to Level (with On/Off) (2, 0) followed by On, followed by Move to Level (b, t), but that is an additional ZigBee message.

I like that approach, with careful timing to not trigger the IKEA transition time bug this could actually work well. The extra messages should be fine and compared to dozens of unicast messages, they don't look too bad.

Since this is a sequence of group casts, this screams to be implemented as a simple state machine within the related group scope.

@ebaauw
Copy link
Collaborator

ebaauw commented May 25, 2019

Yes this might turn out really tricky, albeit possible my main concern here would be poor scaling. With the ever crowing network sizes > 100, unicast messages won't be perceived well. I think we should explore the groupcast path first.

Definitely - I sure as hell didn't mean to issue unicast messages for each member light when PUTting a group action. I was merely considering to keep track of the light types/manufacturers in a group, so we can apply the same whitelist as for PUTting light state to PUTting a group action for homegenous groups (with members of the same type/manufacturer). Still a pain in the neck, though.

We might provide a safety net, by sending Move to Level (with On/Off) (2, 0) followed by On, followed by Move to Level (b, t), but that is an additional ZigBee message.

I like that approach
The extra messages should be fine

Cool. I'll have a go at that, then.

with careful timing to not trigger the IKEA transition time bug this could actually work well.

As long as the first command(s) are sent with a transitiontime of 0, we should be OK.

Since this is a sequence of group casts, this screams to be implemented as a simple state machine within the related group scope.

Not sure I like the complexity of that. I haven't sniffed in detail how the Hue bridge handles bodies of {"on": true, "bri": 127, "ct": 300, "transitiontime": 10}. I would expect a sequence of:

  1. Move to Level (with On/Off) (2, 0)
  2. Move to Level (127, 10)
  3. Move to Color Temperature (127, 10)

or

  1. Move to Level (with On/Off) (2, 0)
  2. Move to Color Temperature (127, 10)
  3. Move to Level (127, 10)

For IKEA lights, we should either use a transitiontime of 0 for the second command, or issue the third command only after the transtion has completed. For the latter, we would need to keep a timer, or poll Remaining Time, neither of which seems like a particularly good idea. For either, the user experience would be different from Hue lights connected to the Hue bridge. I really don't want to sacrifice Hue functionality to work around IKEA issues. So we're back to whitelisting lights and homogenous groups, and not providing a solution for heterogenous groups.

For a next PUT, maybe we could issue a Stop command to actively cancel any running transitions. Haven't tried that, though, so I don't yet know if the IKEA lights would accept that. We should also test how the IKEA light deals with colour transitions.

@stale
Copy link

stale bot commented Sep 22, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

No branches or pull requests

5 participants