search

tfriedel / python-lightify

Python module for Osram lightify. This is a work in progress.
Apache License 2.0
18 stars 9 forks source link

Fix crash when float is passed instead of int #13

Closed OleksandrBerchenko closed 5 years ago

OleksandrBerchenko commented 5 years ago

@tfriedel HA integration is ready. If you have time, it would be great if you could test it.

Just take https://github.com/OleksandrBerchenko/home-assistant/blob/osramlightify/homeassistant/components/light/osramlightify.py and put it instead of the original file (on my env it's located in /srv/homeassistant/lib/python3.5/site-packages/homeassistant/components/light/osramlightify.py). Additionally put the latest lightify/__init__.py instead of /srv/homeassistant/lib/python3.5/site-packages/lightify/__init__.py.

Thanks!

OleksandrBerchenko commented 5 years ago

@tfriedel P.S. See updated documentation at https://raw.githubusercontent.com/OleksandrBerchenko/home-assistant.io/next/source/_components/light.osramlightify.markdown

tfriedel commented 5 years ago

cool, looking forward to trying it out! maybe this weekend or later the week definitely.

btw regarding the motion sensors, there's people who use them in HA with this workaround: https://community.home-assistant.io/t/using-osram-lightify-motion-sensor-with-ha-triggered-by-change-in-rgb/55517

i.e. they require that they can read the rgb values in HA. not sure if we can still do this when we classify it not as a light. will have to check this.

Am Fr., 1. Feb. 2019 um 19:17 Uhr schrieb OleksandrBerchenko < notifications@github.com>:

@tfriedel https://github.com/tfriedel P.S. See updated documentation at https://github.com/OleksandrBerchenko/home-assistant.io/blob/next/source/_components/light.osramlightify.markdown

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tfriedel/python-lightify/pull/13#issuecomment-459817257, or mute the thread https://github.com/notifications/unsubscribe-auth/ABu6xn0FcoSWKznK5LXr1b9KnEXY6ORJks5vJITPgaJpZM4ae0ow .

OleksandrBerchenko commented 5 years ago

And my code essentially breaks their workaround :)

Well, I need to think how to fix/workaround that. Expect a new PR/update to osramlightify.py later today.

OleksandrBerchenko commented 5 years ago

I have updated this PR, osramlightify.py and documentation (refresh the links above).

What I changed:

  1. The library now exposes light.raw_values() with all the raw values as obtained from gateway. So, even if we parsed them 100% incorrectly, a user may still access original values and implement all the logic on his/her side.

  2. HA module checks whether the device type is "SENSOR" and exposes raw_values as sensor_values attribute. For now I have decided not to expose raw_values for all devices to avoid unneeded noise.

My update still breaks the code from https://community.home-assistant.io/t/using-osram-lightify-motion-sensor-with-ha-triggered-by-change-in-rgb/55517, but now it can be easily fixed by changing attributes.rgb_color[1] (green value) to attributes.sensor_values[4]. I will post there, once my changes are merged.

Does it make sense?

Thanks!

OleksandrBerchenko commented 5 years ago

Looking forward for your feedback about HA integration. Once everything is verified, could you please trigger a new release on Pypi?

OleksandrBerchenko commented 5 years ago

Oh, I see you have released it already. Thanks!

tfriedel commented 5 years ago

I started testing yesterday. Basic functionality was working fine. I should still add this device type for the one Ikea lamp with id 128. One thing I noticed, that with two RGB devices (an osram rgbw lamp and an osram lightstrip) blue was not working anymore. Which is probably a coincidence but still weird. So these devices are actually defective now. And for the lightstrip it was working during one of the tests recently. For the other lamp, it might have been defective longer and I just noticed it. If you go to Amazon and read the reviews for these things, there's quite a few people complaining about those lamps failing early. Bummer. Anyway this probably has nothing to do with the changes in the code I hope. I will test it a bit more the next days.

OleksandrBerchenko commented 5 years ago

Sounds strange. So, blue doesn't work anymore even from the Lightify app, correct?

I don't have RGB bulbs right now (my only one stopped working several months ago) and was not able to do 100% accurate testing. Anyway, I don't think I changed anything related to set_rgb/build_colour functionality.

Thanks!

tfriedel commented 5 years ago

Yes, blue doesn't even work from the lightify app anymore. Maybe the testing stressed it too much? I also am now getting started with zigbee2mqtt, so I recently paired this lightstrip with it, which means turning it off and on again quite often. Your light also stopped working, which is another datapoint for these lightify rgbw lamps being of bad quality. Will not buy them anymore I guess.

Am Di., 5. Feb. 2019 um 12:48 Uhr schrieb OleksandrBerchenko < notifications@github.com>:

Sounds strange. So, blue doesn't work anymore even from the Lightify app, correct?

I don't have RGB bulbs right now (my only one stopped working several months ago) and was not able to do 100% accurate testing. Anyway, I don't think I changed anything related to set_rgb/build_colour functionality.

Thanks!

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/tfriedel/python-lightify/pull/13#issuecomment-460609318, or mute the thread https://github.com/notifications/unsubscribe-auth/ABu6xv3FjSa0izc5gMRJQJ_xk7aSXHnRks5vKW99gaJpZM4ae0ow .

OleksandrBerchenko commented 5 years ago

Possibly, I'm not sure how many on/off cycles is expected for their lifetime.

By the way, you might have noticed that I slightly increased timeouts inside test_lightify.py. Without that my lights sometimes went into unreachable state for a few seconds.

rbdubz3 commented 5 years ago

Tested code from master on light strips and RGBW A19 bulbs since last night - all of them Lightify - I don't see any issues with Blue

tfriedel commented 5 years ago

Ok got a chance to test it a bit more. So far everything works pretty smooth! I noticed the colors of the icons in HA is now generally correct. Except with an RGBW lamp, there the displayed color is always the last RGB color. However the lamp can be in either of two modes: RGB or tunable white. Depending on what you changed last decides in which mode it is. For example you set the color to green, then you change the temperature to warm. The color of the icon in HA is green, but the lamp is orange.

OleksandrBerchenko commented 5 years ago

Hm... What is type_id of that lamp? Does it change dynamically?

OleksandrBerchenko commented 5 years ago

Or is it a "usual" RGB lamp?

tfriedel commented 5 years ago

type_id is 10. it's an RGBW. this is it: https://www.amazon.de/Osram-Lightify-Reflektorlampe-Warmwei%C3%9F-tageslicht/dp/B00YSXBMIO or in the official app it's called "PAR 16 50 RGBW - LIGHTIFY".

tfriedel commented 5 years ago

I also wonder if we are dealing with the minimum and maximum temperature correctly. For example the ikea lamp (tunable white) has min. temp 2200. So unless I added a special configuration for it, it was wrong. Would something bad happen if we set a lamp to a lower value than it is specified ? I would hope the lamp has these values as constants and would ignore values that are out of bounds. In this case maybe a good default would be lowest and highest value ever seen across all lamps?

OleksandrBerchenko commented 5 years ago

Ok, I start recollecting how my own RGB lamp worked... That makes sense: you can't change temperature of white in RGB mode.

Two questions:

  1. When you change the temperature and the lamp transforms from green into orange, the icon remains green because HA (as well as lightify library) still think it's in RGB mode and its color didn't change. But what happens after the next update from the gateway (in 5 seconds or what your scan_interval is)? Does it correctly update the colors to (255, 255, 255) and turn orange? In other words, does the next update from the gateway correct the colors?

  2. Are you aware about any RGB lamps without tunable white functionality?

Thanks!

OleksandrBerchenko commented 5 years ago

Regarding min/max temperature - need to think about it.

rbdubz3 commented 5 years ago

@OleksandrBerchenko - Sylvania Lightify has their Outdoor Garden Spots that are ONLY RGB .. No tunable white - actually bought some and realized after opening that they didn't support the white temperature.. returned them

tfriedel commented 5 years ago
  1. no it does not update the colors
  2. according to this list: https://koenkk.github.io/zigbee2mqtt/information/supported_devices.html there's at least this RGB lightstrip: https://de.paulmann.com/innenleuchten/smart-home/bluetooth/smarthome-yourled-rgb-controller-max.-60w/50039?fs=1799497283

Am Di., 5. Feb. 2019 um 23:52 Uhr schrieb OleksandrBerchenko < notifications@github.com>:

Ok, I start recollecting how my own RGB lamp worked... That makes sense: you can't change temperature of white in RGB mode.

Two questions:

1.

When you change the temperature and the lamp transforms from green into orange, the icon remains green because HA (as well as lightify library) still think it's in RGB mode and its color didn't change. But what happens after the next update from the gateway (in 5 seconds or what your scan_interval is)? Does it correctly update the colors to (255, 255, 255) and turn orange? In other words, does the next update from the gateway correct the colors? 2.

Are you aware about any RGB lamps without tunable white functionality?

Thanks!

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/tfriedel/python-lightify/pull/13#issuecomment-460836409, or mute the thread https://github.com/notifications/unsubscribe-auth/ABu6xhpFDWkYwJv0-UGq9U-5wAcHe1Ftks5vKgstgaJpZM4ae0ow .

OleksandrBerchenko commented 5 years ago

Ok, so here we have 3 separate issues:

  1. There are RGBW lamps (probably, a majority), which allow to switch to "tunable white" mode and simple RGB lamps, which don't. For simple RGB lamps we would need to disable changing color temperature (to remove temp from supported_features).

    The question is how to distinguish them? I believe it's possible only if RGB lamps have a different type ID (not 10 like RGBW lamps have). Do I understand correctly that none of you have a RGB lamp right now and can check its type ID?

    If my changes to HA are merged, original type ID will be exposed on UI (for unknown devices it will look like 11 (unknown device)) and it should be easier to ask people what device they have. If RGB and RGBW lamps share the same type ID, then we probably can't help here. If RGB lamps have a different type ID, then we will be able to fix that.

  2. RGBW lamps can switch between "color" and "tunable white" modes. The question is how to find what mode is currently used? It looks like the gateway always provides both RGB and temperature values. If we just changed a color or a temperature using the library, then we can probably assume which mode we have right now. But what if it was changed using the mobile app? Or we are initializing a light for the first time? Currently I don't see any way how to do that. Do I miss anything?

    Possibly our last chance is to check alpha value: it's not used at the moment, but it's exposed as the last element of raw_values list. @tfriedel could you please check whether it's being changed when you switch between "color" and "tunable white" modes? For my tunable white lamps and plugs it's always 255.

  3. (to be continued)

OleksandrBerchenko commented 5 years ago

  1. Some 3rd party lamps (like made by Ikea) have a temperature range different from original Osram lamps (2700-6500 for TW and 1900-6500 for RGB).

    I see two issues/risks with setting the widest possible range for all lamps: 1. "Would something bad happen if we set a lamp to a lower value than it is specified?" - I have no idea :) Hopefully not, but I wouldn't introduce additional risks without a real need. Especially after your suspicions that extensive testing might kill your RGB lamps. 2. That would affect user experience with original Osram lamps. Imagine that on HA UI a user sees a temperature slider with current value somewhere in the middle. The user wants to set the coldest temperature and moves the slider to the left end. Nothing happens and in a moment the slider returns to its original position. Looks like a bug, isn't it?

    Maybe we should return to the idea of adding known 3rd party type IDs to the default list? In that case we will fix at least Ikea lamps (possibly the most common ones among 3rd party lamps). Our assumption that different vendors may use the same type IDs is not based on any precedents yet. Probably we can just add all known types (and wait for bug reports). As I mentioned, now it should be easier to report unknown type IDs.

    @tfriedel, what do you think?

Thanks!

tfriedel commented 5 years ago

@OleksandrBerchenko

  1. I don't have a simple RGB lamp. I wouldn't disable changing color temperature for them, because I can imagine that the lamp just approximates the orange / bluish colors with it's three R,G,B colours. Since we don't know this yet, we potentially would remove functionality. I also think if there is a temperature slider but it has no effect, it's not so bad. The osram lightify app doesn't allow changing the temperature of the ikea bulbs because it makes wrong assumptions and I would prefer if it didn't do this.
  2. The official lightify app recognizes the two states (temperature vs rgb) even if it is not used to control those parameters. I.e. I use HA to change color, then change color temperature and I observe in the lightify app that it updates the color in the UI accordingly. I don't know if there's a direct way to find out what state is in, temperatur or color. But while HA is running we can always keep the values from the last update and compare them with the current update. Then we look what changed, temperature or color and change the mode it's currently in depending on which of those variables changed. This will work almost always and even when the color/temp is changed for example with the official lightify app. I did look at the alpha value. No matter what I do, it always stays at 255.
  3. Re: Using type ids to detect temperature range. I'm wondering if not even bulbs from the same manufacturer with the same type id have different temperature ranges. Here is another implementation where the author gets the capabilities from the type id by using different bits as feature flags: 
        $deviceRGB  = ($type & 8) ? true: false;
    $deviceCCT  = ($type & 2) ? true: false;
    $deviceCLR  = ($type & 4) ? true: false;

    CCT means it has color temperatur. Not sure what CLR means. However the ikea type id 128 doesn't fit this, which explains why the official lightify app can't control it's temperature. I think we should add known type ids to the default list. For this ikea bulb I have I already added it. Unfortunately I don't have more bulbs / typeids together with temperature ranges, so this would be the only one for now.

OleksandrBerchenko commented 5 years ago

@tfriedel

  1. Agree. And until we have some real type IDs that don't support changing color temperature for sure, there is nothing that we can do here.

  2. What if you don't update colors/temperature neither in HA nor in the app and just turn it on/off? Which state to choose when we initialize a light for the first time? Honestly, I don't see any good fix here.

    In theory, the app may get additional information from the cloud. Cloud communicates with gateway using different protocol that may contain additional bits of data. For example, I was not able to find where the app gets information about battery level from.

    Well... Could you turn on logging.DEBUG and check raw onoff value (before we convert it to boolean)? May be it has more possible values, not only 0/1?

  3. Wow, another Lightify library :) I need to take a look.

OleksandrBerchenko commented 5 years ago

@tfriedel

  1. I can't find what exactly CLR means, but it refers to "fixed white" lamps. And those bitwise operations don't look reliable.

  2. The library is able to connect not only to a local gateway, but also to the cloud. But I don't see anything super useful that may be obtained there.

  3. The only interesting thing that I found is a new type id for RGB lamps without tunable white capabilities:

  const TYPE_LIGHT_COLOR        = 8;  //Fixed White and RGB
  const TYPE_LIGHT_EXT_COLOR    = 10; //Tuneable White and RGBW

  const LABEL_LIGHT_COLOR       = "On|Off Level Colour";
  const LABEL_LIGHT_EXT_COLOR   = "On|Off Level Colour Temperature";

So, probably we can add it to the library (that would address my p. 1 above). What do you think?

Thanks!

OleksandrBerchenko commented 5 years ago

@tfriedel I have created a new PR for RGB/RGBW stuff: https://github.com/tfriedel/python-lightify/pull/17 . Let me know what you think.

P.S. By the way, that PHP library is not able to recognize temperature/RGB states too. I really doubt that we could leverage onoff, but let me know if you checked that.

Thanks!

tfriedel commented 5 years ago

I think we can safely add this device type 8, even though we have no ability to test it.

Am Sa., 16. Feb. 2019 um 18:11 Uhr schrieb OleksandrBerchenko < notifications@github.com>:

@tfriedel https://github.com/tfriedel I have created a new PR for RGB/RGBW stuff: #17 https://github.com/tfriedel/python-lightify/pull/17 . Let me know what you think.

P.S. By the way, that PHP library is not able to recognize temperature/RGB states too. I really doubt that we could leverage onoff, but let me know if you checked that.

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tfriedel/python-lightify/pull/13#issuecomment-464363760, or mute the thread https://github.com/notifications/unsubscribe-auth/ABu6xh_7W2hYV8RaEdpr-v2qZrYgZrINks5vODvCgaJpZM4ae0ow .

tfriedel commented 5 years ago

the onoff val is only 0 or 1 in the debug print. I don't know how the app gets the state. I guess you could try to sniff the traffic, but it's probably not worth the effort.

Am Sa., 16. Feb. 2019 um 23:38 Uhr schrieb Thomas Friedel < thomas.friedel@gmail.com>:

I think we can safely add this device type 8, even though we have no ability to test it.

Am Sa., 16. Feb. 2019 um 18:11 Uhr schrieb OleksandrBerchenko < notifications@github.com>:

@tfriedel https://github.com/tfriedel I have created a new PR for RGB/RGBW stuff: #17 https://github.com/tfriedel/python-lightify/pull/17 . Let me know what you think.

P.S. By the way, that PHP library is not able to recognize temperature/RGB states too. I really doubt that we could leverage onoff, but let me know if you checked that.

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tfriedel/python-lightify/pull/13#issuecomment-464363760, or mute the thread https://github.com/notifications/unsubscribe-auth/ABu6xh_7W2hYV8RaEdpr-v2qZrYgZrINks5vODvCgaJpZM4ae0ow .

OleksandrBerchenko commented 5 years ago

Agree. Thanks!

OleksandrBerchenko commented 5 years ago

Ok, then we are probably done for now. Could you please create a new build on Pypi? 1.0.7.1

tfriedel commented 5 years ago

done!

Am So., 17. Feb. 2019 um 08:23 Uhr schrieb OleksandrBerchenko < notifications@github.com>:

Ok, then we are probably done for now. Could you please create a new build on Pypi? 1.0.7.1

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tfriedel/python-lightify/pull/13#issuecomment-464424845, or mute the thread https://github.com/notifications/unsubscribe-auth/ABu6xobT3IWtODNS7JClAAGpMww4oNbBks5vOQNhgaJpZM4ae0ow .