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

Add support for scenes. Other fixes and improvements. #8

Closed OleksandrBerchenko closed 5 years ago

OleksandrBerchenko commented 5 years ago

Resolving #7 .

OleksandrBerchenko commented 5 years ago
tfriedel commented 5 years ago

thanks for your work. I will try it out soon. did you see there are some tests in test_lightify.py. It requires some changes to your situation (names of lights and groups) and observing if the lights do what they should. this should be extended for scenes also.

OleksandrBerchenko commented 5 years ago

Thanks! Looking into tests right now.

OleksandrBerchenko commented 5 years ago

I have added scenes to the tests. Unfortunately, I'm not able to test RGB lights.

OleksandrBerchenko commented 5 years ago

More to come tomorrow.

OleksandrBerchenko commented 5 years ago

At the moment there is no way to detect that some light changed its name. I will fix that.

Much worse: keys in public lightify.groups() dict are mutable names (not immutable indices like lightify.lights() with immutable addrs as keys). Need to think how to deal with that.

OleksandrBerchenko commented 5 years ago

Also we need to handle a case when a light was deleted from gateway, but the library consumer keeps a link to the object.

OleksandrBerchenko commented 5 years ago

Well, I have decided to leave groups dict without changes (as well as scenes dict made by copying existing behavior of groups dict). All objects in the dict are recreated after each update anyway (unlike lights where we keep original objects when possible), so probably nobody keeps links to the objects themselves.

OleksandrBerchenko commented 5 years ago

I really hoped to obtain battery level of switches (it's displayed in the application + according to https://github.com/noctarius/lightify-binary-protocol, lum value works as battery level for motion sensors), but without luck :( I have two switches with different battery levels and both return exactly the same bytes.

OleksandrBerchenko commented 5 years ago
OleksandrBerchenko commented 5 years ago

Ooph... That's all for now, I think.

Looking forward for your review. Thanks!

OleksandrBerchenko commented 5 years ago

Well... I have already started reworking Home Assistant integration part, so new changes may follow if needed.

Thanks!

tfriedel commented 5 years ago

first of all, thanks again for your hard work! you improved this lib quite a bit. When I first tried to run the tests in my environment I experienced some problems due to this line

devicetype_raw = DeviceTypeRaw(devicetype_raw)

I have lamps from three different manufacturers currently: osram lightify, ikea and innr. And these devicetypes that are listed don't cover all of them. For example one lamp has 128 as devicetype. You have to have reasonable fallbacks for the case your hardcoded ids don't cover a device. That's why

ID_TO_DEVICETYPE = defaultdict(lambda: DeviceType.LIGHT)

maps any ID that is not in the dict to a light. I just removed the above line, since it doesn't matter if it's an int or an enum, when we look for it in the dict anyway. After this everything worked. I noticed that you fixed a bug with the groups. Very nice! Scenes also worked for me. Well there's one bug in the gateway where it doesn't set some of the ikea lamps in a scene to the correct temeprature. But this is also with the original osram lightify app. Because of things like this I'm going to use home assistant scenes instead of the lightify scenes.

I don't have the time to go through all your changes and am just going to trust that they are good. I can offer to test the lib with your modified home assistant lib when it's ready.