NSPManager / NSPanelManager

Sonoff NSPanel custom firmware for responsive and intuitive use
https://nspanelmanager.com/
126 stars 10 forks source link

Changing state_sat changes state_brightness to same value #147

Closed cablesandcoffee closed 2 months ago

cablesandcoffee commented 2 months ago

Describe the bug By coincidence I saw that the manager has the same state_sat value as brightness value on all the rgb lights. If I change saturation in Openhab, saturation is changed in NSPM but it also effects the brightness state for the same light. If you change the brightness of that light in Openhab the manager still keeps the wrong brightness value which is always the same as the state_sat value.

This is what it looks like in the mqtt log when restarting the manager:

nspanel/entities/light/48/state_brightness_pct 0 nspanel/entities/light/48/state_kelvin 24 nspanel/entities/light/48/state_brightness_pct 35 nspanel/entities/light/48/state_hue 25 nspanel/entities/light/48/state_sat 35 nspanel/entities/light/49/state_brightness_pct 0 nspanel/entities/light/49/state_kelvin 19 nspanel/entities/light/49/state_brightness_pct 28 nspanel/entities/light/49/state_hue 25 nspanel/entities/light/49/state_sat 28

There are actually two rows for state_brightness_pct. 0 is correct in this case but the 35 and 38 seems weird?

I can confirm that if I restart a panel in this state, the panel will update the screen and say that these lights are ON all though they are OFF.

To Reproduce Steps to reproduce the behavior:

  1. Change saturation on a RGB lights in Openhab
  2. Restart manager and see the list in mqtt log with all light entities.
  3. Manager has now set saturation and brightness on that light to the same value
  4. Change brightness value on same light in Openhab to 0
  5. Restart a panel
  6. Panel will set that light to be on with the value of state_sat all though its 0

Expected behavior state_sat shouldn't effect brightness

Environment info (please complete the following information):

tpanajott commented 2 months ago

The problem is that OpenHAB incorporates brightness in the "RGB" setting, in this case, HSB. I do not believe that the "brightness" item changes state when the HSB item changes state so we must use the brightness value from the HSB item. When starting the MQTTManager it can't know what state the light is in so it subscribes to the brightness, kelvin and HSB items. When it then receives brightness "0" from the brightness item it sends that state out but it later receives "28" from the HSB item it also sends that out as it cannot know what value is actually correct. I'm not quite sure how to go about fixing that aspect.

Given the above, the saturation still shouldn't change when changing brightness.

tpanajott commented 2 months ago

Please test with latest devel release (using the scripts and git as it's not yet released). This should be fixed.

cablesandcoffee commented 2 months ago

Hmm, doesn't seem to be fixed. My Brightness dimmer item and HSB item are always in synch. If I change the brightness dimmer item the brightness value in the HSB is also updated. Same the other way around.

If I set saturation to 20 on one light in Openhab, restart manager. That lights is shown as 20 brightness on the panel.

But is it really correct that there are two values with brightness sent out to the panel? This is mqtt log when restarting manager: nspanel/entities/light/48/state_brightness_pct 100 nspanel/entities/light/48/state_brightness_pct 20 nspanel/entities/light/48/state_hue 35 nspanel/entities/light/48/state_sat 20

100 is correct. 20 has some how come from the sat value?

tpanajott commented 2 months ago

Can you check what HSB value is set on the light in OpenHAB?

tpanajott commented 2 months ago

Also, is it only setting the brightness to the saturation value when you restart the manager or is it also when the manager is running and you simply change the HSB value of the light?

cablesandcoffee commented 2 months ago

Oh updated the text above a bit. In Openhab: HSB item is set to 35,20,100 Brightness Dimmer is set to 100

tpanajott commented 2 months ago

Strange. I belive I fixed that. Will have to take a closer look.

tpanajott commented 2 months ago

I can't reproduce it in the latest dev.

cablesandcoffee commented 2 months ago

This is fixed. I was on the wrong branch! :)