fix: skip cached state override when plug/humidifier ignore command

[bluetoothbot]

What

Gate _override_state on the result of _check_command_result in SwitchbotPlugMini.turn_on/turn_off and SwitchbotHumidifier._async_set_state/_set_level. Same shape as #502 (bot), extended to the other two device classes where the existing _check_command_result call was discarded.

Why

Audit follow-up to #502 (which fixed the bot half of #213). The same anti-pattern lives in plug.py and humidifier.py:

ret = self._check_command_result(result, 1, {0x80})  # protocol says "accepted?"
self._override_state({"isOn": True})                  # …but we override anyway

If the device returns anything other than the expected ACK byte, ret is False but the cached state still gets flipped, so the library lies about state to its caller. The bot version was observed in the wild (#213); the plug/humidifier versions are the same code shape and the existing _check_command_result calls show the original author already intended to validate — the gate was simply missing.

How

Two-line gate in each method, matching #502:

ret = self._check_command_result(result, ...)
if ret:
    self._override_state({...})
    self._fire_callbacks()
return ret

Behaviour for the accepted path is unchanged.

Testing

• New tests/test_plug.py (4 tests): accepted/rejected × turn_on/turn_off.
• New tests/test_humidifier.py (6 tests): accepted/rejected × turn_on/turn_off/set_level. Neither device had a test module before.
• Full suite: 1223 passed.

---

Quality Report

Changes: 18 files changed, 385 insertions(+), 2074 deletions(-)

Code scan: clean

Tests: passed (1311 passed)

Branch hygiene: clean

Generated by Kōan

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
switchbot/devices/humidifier.py	93.10% <100.00%> (+50.24%)	⬆️
switchbot/devices/plug.py	92.30% <100.00%> (+46.47%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR prevents cached state from being overridden when a Plug Mini or Humidifier command is rejected/ignored by gating _override_state() / _fire_callbacks() on _check_command_result(). It also adds new test coverage for the accepted vs rejected command paths for both device classes.

Changes:

• Gate cached state override + callback firing on ret in SwitchbotPlugMini.turn_on/turn_off.
• Gate cached state override + callback firing on SwitchbotHumidifier._async_set_state/_set_level.
• Add new unit tests for Plug Mini and Humidifier accepted/rejected command behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.

File	Description
switchbot/devices/plug.py	Only override cached isOn and fire callbacks when _check_command_result() succeeds.
switchbot/devices/humidifier.py	Only override cached isOn/level and fire callbacks when _check_command_result() succeeds.
tests/test_plug.py	Adds Plug Mini tests for accepted/rejected turn_on/turn_off.
tests/test_humidifier.py	Adds Humidifier tests for accepted/rejected turn_on/turn_off/set_level.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bluetoothbot]

PR Review — fix: skip cached state override when plug/humidifier ignore command

Correct, well-tested core fix; the humidifier's caller-visible return contract is left half-fixed.

The if ret: gate is exactly right and directly extends the merged #502/bot fix to plug and humidifier — the cached-state lie on a rejected command is closed for both. Adding two brand-new test modules (10 tests, accepted×rejected paths) for device classes that previously had zero coverage is solid, and the plug tests correctly assert the boolean return.

The gap is the humidifier: the PR description sells return ret as letting callers "detect rejected commands," but that only reaches the plug. The humidifier's public wrappers (turn_on/turn_off/set_level/async_set_auto/async_set_manual) still swallow the helper's return and yield None despite -> bool. The humidifier tests don't catch this because they never assert the return value.

• warning: humidifier public methods return None, not ret — cached state fixed, but caller-visible rejection contract still broken (agrees with Copilot 52/74)
• suggestion: humidifier tests should assert the boolean return (currently mask the bug above)
• suggestion: use assert_awaited_once_with for _send_command assertions to match repo convention

---

🟡 Important

1. Humidifier public methods swallow the return value — fix is only half-delivered
switchbot/devices/humidifier.py:57-68

The core fix (gating _override_state on ret) is correct, but for the humidifier it only fixes the cached-state lie, not the return-value contract the PR description claims to restore.

_async_set_state and _set_level now correctly return ret, but the public wrappers that callers actually invoke discard it:

async def turn_on(self) -> bool:
    await self._async_set_state(True)   # no return -> returns None

async def turn_off(self) -> bool:
    await self._async_set_state(False)  # returns None

async def set_level(self, level: int) -> bool:
    assert 1 <= level <= 100, ...
    await self._set_level(level)         # returns None

async def async_set_auto(self) -> bool:
    await self._set_level(128)           # returns None

async def async_set_manual(self) -> bool:
    await self._set_level(50)            # returns None

Why it matters: the PR's stated rationale is "return ret ... so callers can detect rejected commands." For the plug that holds — turn_on/turn_off return ret directly. For the humidifier, every public entry point is annotated -> bool but returns None at runtime, so a caller checking if await humidifier.turn_on(): to detect a rejected command gets None (falsy) on every call, accepted or not. The cached-state half is fixed; the caller-visible half is not.

This is a pre-existing bug (predates this PR — these wrappers haven't changed since #273), but the PR is exactly the change that makes it relevant, and it's a one-token fix per method:

async def turn_on(self) -> bool:
    return await self._async_set_state(True)

Apply return to all five wrappers (turn_on, turn_off, set_level, async_set_auto, async_set_manual). This matches Copilot's two comments on lines 52 and 74.

async def turn_on(self) -> bool:
    """Turn device on."""
    await self._async_set_state(True)

🟢 Suggestions

1. Humidifier tests don't assert the return value — they mask the swallowed-return bug
tests/test_humidifier.py:47-61

The plug tests assert await device.turn_on() is True / is False, which validates the caller-visible contract. The humidifier tests only check device.is_on() / get_level() and never assert the return value:

await device.turn_on()
assert device.is_on() is True

Why it matters: because the public wrappers return None (see the warning on humidifier.py), an assertion like assert await device.turn_on() is True would fail today. The tests pass precisely because they avoid asserting the return — so the suite gives false confidence that the humidifier's -> bool contract is honored when it isn't.

Once the wrappers are fixed to return, tighten these to assert both the return value and the cached state (mirroring the plug tests), e.g. assert await device.turn_on() is True. That converts these tests from "state changed" into a real regression guard for the rejected-command contract.

2. Prefer assert_awaited_once_with for AsyncMock command assertions
tests/test_plug.py:51

_send_command is an AsyncMock, and assert_called_with confirms it was called but not that it was awaited, nor that it was called exactly once. The rest of this test suite (test_base_cover.py, test_curtain.py, test_evaporative_humidifier.py, test_vacuum.py, etc.) uses assert_awaited_once_with for this pattern.

For consistency and a slightly stronger check, swap the four device._send_command.assert_called_with(plug.PLUG_*_KEY) calls to assert_awaited_once_with(...). Minor — the tests are correct as-is — but it matches the established convention. (Matches Copilot's four comments on this file.)

device._send_command.assert_awaited_once_with(plug.PLUG_ON_KEY)

---

Checklist

• Core fix (gate _override_state on result) is correct
• Public API honors its -> bool type contract — warning ＃1
• Tests cover the caller-visible contract — suggestion ＃2
• Tests follow repo async-assertion conventions — suggestion ＃3
• No security-sensitive code paths touched
• Accepted-path behavior unchanged

---

To rebase specific severity levels, mention me: @bluetoothbot rebase critical (fixes 🔴 only), @bluetoothbot rebase important (fixes 🔴 + 🟡), or just @bluetoothbot rebase for all.

---

Automated review by Kōan (Claude) HEAD=6f50dec 2 min 33s

[bluetoothbot]

Rebase with requested adjustments

Branch koan/plug-humidifier-state-gate was rebased onto master and review feedback was applied.

Changes applied

• Done. Summary:
• switchbot/devices/humidifier.py — Added return to turn_on, turn_off, set_level, async_set_auto, async_set_manual. They were annotated -> bool but discarded the helper result and returned None at runtime, so callers couldn't detect rejected commands. Addresses Copilot comments on lines 52 & 74 and conversation warning press #1.
• tests/test_plug.py — Swapped four assert_called_with to assert_awaited_once_with, matching suite convention and verifying the AsyncMock was awaited. Addresses Copilot's four comments + suggestion Beta #2.
• tests/test_humidifier.py — Each test now asserts the boolean return value (is True/is False) and _send_command.assert_awaited_once(), so a no-op or swallowed return fails. Addresses Copilot's six comments + suggestion press #1.

Stats

4 files changed, 214 insertions(+), 13 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=PR fix: skip cached state override when bot ignores command (#213) #502 fixed only the bot device; no commit on master gates plug.py or humidifier.py _override_stat)
• Rebased koan/plug-humidifier-state-gate onto upstream/master
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/plug-humidifier-state-gate to origin
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan