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

Distinguish RGB and RGBW lamps #17

Closed OleksandrBerchenko closed 5 years ago

OleksandrBerchenko commented 5 years ago

All unknown lamps are still treated as RGBW.

I have also reordered DeviceSubType (while it's not too late): it's more likely to get new sensor types than new switches.

OleksandrBerchenko commented 5 years ago

@tfriedel FYI: https://github.com/home-assistant/home-assistant/pull/21176

OleksandrBerchenko commented 5 years ago

@tfriedel Hi! Could you please comment in https://github.com/home-assistant/home-assistant/pull/21176 that you also tested those changes? Maybe that could speed up the reviewing process :)

Thanks!

tfriedel commented 5 years ago

right, that seems stuck atm unfortunately. maybe I have to check what's going on with the log messages first

Am Sa., 9. März 2019 um 10:28 Uhr schrieb OleksandrBerchenko < notifications@github.com>:

@tfriedel https://github.com/tfriedel Hi! Could you please comment in home-assistant/home-assistant#21176 https://github.com/home-assistant/home-assistant/pull/21176 that you also tested those changes? Maybe that could speed up the reviewing process :)

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/17#issuecomment-471161521, or mute the thread https://github.com/notifications/unsubscribe-auth/ABu6xnmEi-MeMGYsaLas3Zs88jijZLioks5vU37HgaJpZM4a_FTe .

OleksandrBerchenko commented 5 years ago

@tfriedel It looks like the issue exists only if it's installed as a custom component + there is no logging configuration in configuration.yaml (and we rely on some default values). Well, I didn't try it as a custom component (because it's not going to be a "custom component" :) ), but I suspect that custom components ignore Home Assistant default (WARNING) and use their own defaults (DEBUG in our case). To be honest, I don't see a problem here. Maybe I'm wrong.

Thanks!

tfriedel commented 5 years ago

ok I don't understand this completely yet, but I wonder why we have DEBUG as default. can we fix this easily by changing the default to WARNING as well?

Am Sa., 9. März 2019 um 11:46 Uhr schrieb OleksandrBerchenko < notifications@github.com>:

@tfriedel https://github.com/tfriedel It looks like the issue exists only if it's installed as a custom component + there is no logging configuration in configuration.yaml (and we rely on some default values). Well, I didn't try it as a custom component (because it's not going to be a "custom component" :) ), but I suspect that custom components ignore Home Assistant default (WARNING) and use their own defaults (DEBUG in our case). To be honest, I don't see a problem here. Maybe I'm wrong.

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/17#issuecomment-471166328, or mute the thread https://github.com/notifications/unsubscribe-auth/ABu6xuJt7oCitxRzPE9mmcMROaVTXJvgks5vU5D1gaJpZM4a_FTe .

OleksandrBerchenko commented 5 years ago

@tfriedel As far as I observe, if I set it to WARNING, it won't be possible to change it to INFO or DEBUG in Home Assistant (only to ERROR). If I set it to DEBUG, in Home Assistant I can now set any level like:

logger:
  default: info

or

logger:
  default: warning
  logs:
    lightify: debug

Also see https://github.com/home-assistant/home-assistant/pull/21176#issuecomment-466485696 .

Thanks!

tfriedel commented 5 years ago

I did copy over osramlightify.py the old and manually upgraded lightify and logging worked with the snippets you posted. Also when no "logger:" config was present, it was not defaulting to debug. I'm not sure if it is good that we configure "lightify" and not something like homeassistant.components.light or homeassistant.components.light.osramlightify. I have no idea how this is like in other components.

I could reproduce @SukramJ 's problem, that with a fresh home assistant installation and osramlightify as a custom component the default logging level for lightify was debug. Not only that, after I reproduced this problem I moved it from custom_components over the old one. I expected the problem would not appear then. But it appeared there too. So in summary, logging is broken and needs to be fixed. I'm not sure how. Is there a guide or requirements? Or some other custom component that uses an external library with logging for inspiration?

tfriedel commented 5 years ago

turns out it is easier to fix then I thought. in osramlightify change logging.DEBUG to logging.NOTSET . it fixed all the problems.

OleksandrBerchenko commented 5 years ago

@tfriedel Wow, never tried that. I will double check and apply a fix.

Thanks!

OleksandrBerchenko commented 5 years ago

@tfriedel https://github.com/home-assistant/home-assistant/pull/21176