Fix brightness dropped when color_temp is set alongside it in turn_on() - #167
Open
imwithsam wants to merge 1 commit into
Open
Fix brightness dropped when color_temp is set alongside it in turn_on()#167imwithsam wants to merge 1 commit into
imwithsam wants to merge 1 commit into
Conversation
turn_on()'s elif chain (in both CyncRoom and CyncSwitch) only calls combo_control() for RGB-mode changes (color_tone=254) or brightness-only changes with a "no color" marker (color_tone=255) - the color_tone byte is never actually populated with a real color-temperature value anywhere in the codebase, even though combo_control()'s own packet format already supports it (color_tone is sent as a raw byte, and the device firmware happily accepts 0-100 there instead of just the 254/255 mode markers). Whenever attr_ct was set, the code fell through to the ct-only branch, which sends two legacy single-attribute commands (hub.turn_on + hub.set_color_temp) and never looks at attr_br at all - so any call that set brightness and color_temp together (e.g. Adaptive Lighting's `apply`, or any client sending both attributes in one light.turn_on) silently lost the brightness change. Fix: add a combo_control() branch for the brightness+color_temp case, using the real color-temp percentage (the same 0-100 conversion already used by the existing color-temp-only branch) instead of a mode marker. ## Testing Verified live against a real Home Assistant instance with Adaptive Lighting temporarily disabled to isolate the call: sent light.turn_on(brightness=180, color_temp_kelvin=3200) and light.turn_on(brightness=90, color_temp_kelvin=5500) as single calls - both landed exactly (brightness within normal 0-100% round-trip rounding, color_temp exact) in one atomic packet, no errors in debug logs. This was previously reported (see nikshriv#122) with a documented `separate_turn_on_commands` workaround in Adaptive Lighting; this fix addresses the actual root cause so the workaround is no longer necessary (though harmless to leave enabled).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #122 ("Adaptive Lighting not working") - this fixes the actual root cause rather than working around it.
turn_on()'selifchain (in bothCyncRoomandCyncSwitch) only callscombo_control()for RGB-mode changes (color_tone=254) or brightness-only changes with a "no color" marker (color_tone=255). Thecolor_tonebyte is never actually populated with a real color-temperature value anywhere in the codebase, even thoughcombo_control()'s own packet format already supports it -color_toneis sent through as a raw byte, and (confirmed via live testing) the device firmware accepts a real 0-100 color-temp percentage there just as happily as the 254/255 mode markers.Whenever
attr_ctwas set, execution fell through to the color-temp-only branch, which sends two legacy single-attribute commands (hub.turn_on+hub.set_color_temp) and never looks atattr_brat all. So any call setting brightness and color_temp together - e.g. Adaptive Lighting'sapply, or any client sending both attributes in onelight.turn_on- silently lost the brightness change.Fix
Add a
combo_control()branch for the brightness+color_temp case, using the real color-temp percentage (the same 0-100 conversion already used by the existing color-temp-only branch) instead of a mode marker.Testing
Verified live against a real Home Assistant instance, with Adaptive Lighting temporarily disabled to isolate the call:
light.turn_on(brightness=180, color_temp_kelvin=3200)-> landed as brightness 181 (expected rounding from the 0-100% round-trip), color_temp exactly 3200Klight.turn_on(brightness=90, color_temp_kelvin=5500)-> landed as brightness 89 (same expected rounding), color_temp exactly 5500KBoth in a single atomic call, no errors in debug logs. This was previously investigated in #122 with a documented
separate_turn_on_commandsAdaptive Lighting workaround (splitting the call into two); this fix addresses the actual root cause, so that workaround is no longer necessary going forward (though harmless to leave enabled for anyone who already has it set).