fix: call detect_bulb() in *_percentage() methods before reading value_max#714
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an initialization-order bug in BulbDevice where percentage-based setters could compute incorrect raw DPS values when called before bulb detection populated dpset['value_max'], leading to unintended full-brightness behavior (Issue #713).
Changes:
- Add a
bulb_configuredguard that callsdetect_bulb()at the start ofset_brightness_percentage(). - Add the same guard to
set_white_percentage()andset_colourtemp_percentage()to ensurevalue_maxis available before percentage conversion. - Align percentage setters’ behavior with existing lower-level setters that already ensure bulb detection occurs before using bulb-specific ranges.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not self.bulb_configured: | ||
| self.detect_bulb(nowait=nowait) | ||
|
|
| if not self.bulb_configured: | ||
| self.detect_bulb(nowait=nowait) |
| if not self.bulb_configured: | ||
| self.detect_bulb(nowait=nowait) |
| if not self.bulb_configured: | ||
| self.detect_bulb(nowait=nowait) |
|
@jasonacox-sam Please review and address the copilot code review items if they have merit. |
…thods When set_brightness_percentage(), set_white_percentage(), or set_colourtemp_percentage() is called before bulb detection has run, self.dpset['value_max'] is still -1 (the unconfigured default). The computed raw value is then wrong — always -1 — which triggers the fallback in set_brightness() and pins the device to full brightness regardless of the requested percentage. Fix: call detect_bulb() if bulb_configured is False before computing the raw value from value_max. This mirrors the guard already present in set_brightness(), set_white(), and other lower-level methods. Fixes jasonacox#713
Address Copilot review: detect_bulb() can exit without configuring when nowait=True with no cached status, leaving value_max=-1 and producing incorrect raw values. Now raises RuntimeError like state() and bulb_has_capability() already do. Also adds test_percentage_raises_when_unconfigured to verify the guard.
d088cc8 to
3841d1b
Compare
|
@jasonacox — Reviewed all four Copilot items. They have merit: Fixed in 3841d1b:
All 23 tests passing. — Sam ⚙️ |
|
Thanks, Sam! Added update to bundle with future release. |
Fixes #713
Problem
When
set_brightness_percentage(),set_white_percentage(), orset_colourtemp_percentage()is called before bulb detection has run,self.dpset['value_max']is still-1(the unconfigured default). The computed raw value is wrong, which causes the< 0fallback inset_brightness()to fire and pin the device to full brightness regardless of the requested percentage.Fix
Add a
if not self.bulb_configured: self.detect_bulb(nowait=nowait)guard at the top of each_percentage()method, before thevalue_maxmultiplication. This mirrors the guard already present inset_brightness(),set_white(), and other lower-level methods.Affected Methods
set_brightness_percentage()set_white_percentage()set_colourtemp_percentage()Testing
Workaround confirmed by @andreidiaconescu18 in #713 — calling
d.status()first (which triggersdetect_bulb()) makesset_brightness_percentage(75)work correctly. This PR makes that detection automatic.— Sam ⚙️