Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions homeassistant/components/blebox/light.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ async def async_turn_on(self, **kwargs: Any) -> None:
else:
value = feature.apply_brightness(value, brightness)

if isinstance(value, list) and not any(value):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If apply_brightness ever returns all zeros for a non-zero brightness, that's a bug in the library that should be caught and fixed there. Adding a test that mocks that behavior would only be testing the mock, not protecting against a real regression.

await self._feature.async_off()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async_turn_off calls async_off() without any try/except either, so the error handling is already consistent. Neither path wraps it.

return

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root cause is that sensible_on_value can already return all-zero channel values, and in that case async_on([0, 0, 0, 0]) always raises a ValueError regardless of what the user requested. So checking the computed value is the right place to catch this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix it and add checking against a tuple.

Comment on lines +204 to +206

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard is intentionally on the post-processed value. Apply_brightness with brightness=0 always produces all-zero list/tuple for every color mode we handle, and no code path here produces a scalar. Checking brightness == 0 early would also miss cases where brightness comes implicitly from rgb/rgbww.


try:
await self._feature.async_on(value)
except ValueError as exc:
Expand Down
42 changes: 42 additions & 0 deletions tests/components/blebox/test_light.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,48 @@ def turn_on(value):
assert state.state == STATE_ON


async def test_wlightbox_turn_on_with_zero_brightness_turns_off(
wlightbox, hass: HomeAssistant
) -> None:
"""Test that setting brightness to 0 turns the light off instead of raising ValueError."""

feature_mock, entity_id = wlightbox

def initial_update():
feature_mock.is_on = True
feature_mock.rgbw_hex = "c1d2f3c7"
feature_mock.white_value = 0xC7

feature_mock.async_update = AsyncMock(side_effect=initial_update)
await async_setup_entity(hass, entity_id)
feature_mock.async_update = AsyncMock()

state = hass.states.get(entity_id)
assert state.state == STATE_ON

feature_mock.apply_brightness = MagicMock(return_value=[0, 0, 0, 0])

def turn_off():
feature_mock.is_on = False
feature_mock.white_value = 0x0
feature_mock.rgbw_hex = "00000000"

feature_mock.async_off = AsyncMock(side_effect=turn_off)

await hass.services.async_call(
"light",
SERVICE_TURN_ON,
{"entity_id": entity_id, ATTR_BRIGHTNESS: 0},
blocking=True,
)

feature_mock.async_off.assert_called_once()
feature_mock.async_on.assert_not_called()

state = hass.states.get(entity_id)
assert state.state == STATE_OFF


async def test_wlightbox_off(wlightbox, hass: HomeAssistant) -> None:
"""Test light off."""

Expand Down