feat: implement passcode management and cloud synchronization for SwitchBot Keypad

[deece]

Implement passcode management and cloud synchronization for SwitchBot Keypad

This Pull Request adds complete local and cloud passcode management and clock
synchronization capabilities for the non-vision SwitchBot Keypad (WoKeypad).

It also integrates the passive attempt_state advertisement property and tests proposed in PR #488 (by @italo-lombardi).

Why Cloud Sync is Needed:

• Keypad passcodes are stored and validated locally (allowing the Keypad to directly command the Lock offline without requiring internet connectivity).
• However, for security reasons, the Keypad's BLE protocol is write-only for passcodes. There is no command to read back or list saved codes over BLE.
• Since the SwitchBot app cannot query the Keypad to build its UI passcode list, it relies entirely on the SwitchBot Cloud database to track which codes exist.
• If a passcode is added locally over BLE but not synced to the cloud, it will successfully operate the Lock offline, but will be completely invisible in the official mobile app. Integrating the cloud sync option ensures the app UI stays in sync with the keypad's physical storage.

What Was Implemented:

1. Passcode Management:
   • add_password: Sends passcode bytes over BLE to register a new PIN code on the keypad. Parses response to retrieve the device-assigned index.
   • modify_password: Modifies an existing passcode by index. Overwrites values and configures active duration ranges.
   • delete_password: Instantly deletes a passcode from device memory by index.
   • get_password_count: Queries counts of stored PINs, NFC tags, fingerprints, and duress credentials.
2. Clock Synchronization (RTC):
   • sync_time: Synchronizes the internal device clock with millisecond precision (sending 8-byte big-endian milliseconds timestamp). Automatically scales second-level timestamps for backwards compatibility.
   • Note: Clock querying (get_time) was tested but found to return error 05 (unsupported) by the keypad hardware, indicating it is a write-only clock, so it has been dropped from the implementation.
3. SwitchBot Cloud Sync Option:
   • Added keyword-only parameters to add_password: session, token, region, name, and creator.
   • If provided, automatically triggers an HTTP POST request to API function 4245 to register the passcode in the SwitchBot Cloud database so it appears correctly in the official smartphone app.
4. Passive Advertisement Property Integration (from Add SwitchbotKeypad device class for classic Keypad #488):
   • Exposes attempt_state property via _get_adv_value in SwitchbotKeypad.
   • Adds the missing KEYPAD_INFO test fixture to tests/__init__.py.

Testing Methodology:

1. Automated Tests (tests/test_keypad.py):
   • Created full coverage suite for add/modify/delete password, counts, and sync_time.
   • Implemented test_add_password_with_cloud_sync mocking api_request to verify payload structures, regional routing, and authorization headers.
   • Added advertisement parsing unit tests for battery percentage and attempt state.
   • Verified that all 14 unit tests pass cleanly.
2. Manual/Hardware Verification:
   • Tested using test_hardware.py on a physical WoKeypad and Lock.
   • Verified that syncing clock with millisecond timestamps sets the correct RTC, allowing time-limited (temporary) passcodes to evaluate successfully offline and unlock the lock.

Co-authored-by: Alastair D'Silva alastair@d-silva.org
Co-authored-by: Antigravity antigravity@google.com

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Blocking issues found.

• MagicMock used in annotations but never imported — test module fails to import
• Cloud sync silently skipped on partial credentials; BLE write already committed if cloud call then fails

[Copilot]

Pull request overview

Adds full BLE passcode management and RTC clock synchronization for the classic SwitchBot Keypad (WoKeypad), with an optional SwitchBot Cloud “passcode list” synchronization path, plus advertisement parsing coverage for attempt_state.

Changes:

• Introduces SwitchbotKeypad device implementation with add/modify/delete passcode APIs, passcode counts, and RTC sync.
• Adds a cloud-sync option to add_password that posts passcode metadata to SwitchBot Cloud (functionID 4245).
• Adds unit tests and an advertisement fixture for keypad battery and attempt_state.

Reviewed changes

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

File	Description
switchbot/devices/keypad.py	New Keypad device class implementing passcode + time sync APIs and optional cloud sync.
switchbot/__init__.py	Exports SwitchbotKeypad from the package public surface.
tests/test_keypad.py	Adds coverage for passcode ops, password counts, RTC sync, cloud sync payload shape, and adv parsing.
tests/__init__.py	Adds KEYPAD_INFO advertisement fixture used by keypad tests.

---

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

[codecov]

Codecov Report

❌ Patch coverage is 96.63462% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
switchbot/devices/keypad.py	96.61%	7 Missing ⚠️

Files with missing lines	Coverage Δ
switchbot/__init__.py	100.00% <100.00%> (ø)
switchbot/devices/keypad.py	96.63% <96.61%> (+96.63%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

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

[bluetoothbot]

❌ Permission denied. Only users with write access can trigger bot commands.

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Blocking issues found.

• Orphan passcode when time-window write fails after add succeeds

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

No blocking issues found.

[bdraco]

@bluetoothbot review

[bluetoothbot]

@bdraco: @bluetoothbot review

Reviewed at 42f2e80. Prior findings all fixed: MagicMock import, region fullmatch, type/timestamp validation, modify contract, rollback escalation, truncation guards. 32/32 tests pass.

Two new blockers, both on failure paths:

1. modify_password deletes the existing passcode on any failure — including rejected writes where nothing was committed, and out-of-range devices. Caller sees "Failed to modify password"; code silently gone. My earlier "match add_password" note caused this; retracting it. Modify has no prior value to restore.

2. add_password rollback covers only non-0x01 replies. _send_command re-raises bleak errors (device.py:413-446). Connection drop between passcode write and time-window write leaves a permanent passcode, index never returned.

Rest are nits — empty-string token skips cloud sync, no index range check, duplication with keypad_vision.py.

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Warning

Important issues found.

• modify_password deletes the caller's existing passcode on any failure — including failures where nothing was written
• add_password rollback only covers protocol rejection, not the BLE exception path — orphaned always-active passcode

[bluetoothbot]

PR Review — feat: implement passcode management and cloud synchronization for SwitchBot Keypad

Both prior blocking findings are genuinely fixed — no blockers remain. 33/33 tests pass at dde5ed1; merge-ready with nits.

What's solid, verified against the branch:

• The destructive modify_password rollback is gone. It no longer deletes the caller's existing credential on a failed write — the correct call, since the library never knew the previous passcode and had nothing to roll back to.
• add_password's rollback now covers the exception path. The time-window write is wrapped in try/except feeding _rollback_time_window(err), so a BLE drop between the passcode write and the window write no longer strands a permanently-active passcode. _send_command re-raising bleak errors after retries (device.py:407-451) was the exact gap; it's closed, and the escalation-on-failed-rollback path is tested both ways.
• Protocol work checks out. I hand-decoded the multi-chunk path: "987654321" → 12-byte payload → chunks of 11+1, packet_info nibbles 0x20/0x21, matching the asserted 570F52020220FF01090908070605040302 / 570F5202022101. KEYPAD_INFO matches process_wokeypad exactly (mfr_data[6]=0x8f→143, service_data[2]&0x7f→100) and its field order matches AdvTestCase.
• Truncation guards on get_basic_info (< 3) and get_password_count (< 6) are correctly sized for the indices each then reads — and are stricter than the keypad_vision.py original.

What's worth a follow-up (none blocking):

• token="" passes the all-or-nothing credential check but fails the truthiness gate on the sync branch — passcode written to the keypad, silently not to the cloud.
• end_time < start_time is accepted; passcode_type=1 with no window writes 0/0 while registering "timeLimit" in the cloud.
• Cloud-sync rollback fires on bare except Exception, so a transient timeout deletes a working code — and if the server did commit, produces the inverse orphan.
• A mid-sequence chunk failure in _add_passcode_to_device leaves a partial record with no index to clean up (unverified without hardware — the firmware may discard it).
• delete_password returns False silently on a failed revocation and doesn't range-check index; sync_time does the same and accepts negative/oversized timestamps.
• __init__ and verify_encryption_key are pure pass-throughs — set _model = SwitchbotModel.KEYPAD instead (PR refactor: replace model-default override boilerplate with _model classvar #476 convention).
• key_types.get(..., "permanent") is a dead default that would mask a future type/map mismatch.
• ~90 lines duplicated from keypad_vision.py, already divergent — this file's get_password_count has the truncation guard the vision copy still lacks.
• Stale comment at line 242 says the time window is conditional; it is always written.
• modify_password's error on a failed window write reads like nothing changed, but the PIN is already updated.

---

✅ Resolved since last review (5)

Previously-flagged issues verified fixed

• switchbot/devices/keypad.py:334 modify_password deletes the caller's existing passcode on any failure — including failures where nothing was written
• switchbot/devices/keypad.py:238 add_password rollback only covers protocol rejection, not the BLE exception path — orphaned always-active passcode
• switchbot/devices/keypad.py:263 Empty-string token or region passes validation, then silently skips cloud sync
• switchbot/devices/keypad.py:382 delete_password does not validate index range
• switchbot/devices/keypad.py:84 Large verbatim duplication with keypad_vision.py

---

🟢 Suggestions

1. Empty-string token passes the all-or-nothing check, then silently skips cloud sync
switchbot/devices/keypad.py:280

_validate_cloud_credentials (line 115-121) decides "all provided" with is not None, but the sync branch gates on truthiness:

if session and token and region:

token="" satisfies the validator — all three are non-None — and then falls through the if. add_password returns the index having written the passcode to the keypad but not to the cloud, with no warning. That is exactly the "code works on the lock but is invisible in the app" divergence this PR exists to prevent, arrived at silently.

(region="" can't reach here — the [a-z]{2,8} regex rejects it — and a ClientSession is always truthy, so token is the live case.)

Fix: make the two checks agree — either validate with truthiness (if not token: raise ValueError(...)) or gate the sync branch on is not None.

        if session and token and region:

2. Time window bounds are validated in isolation — an inverted or empty window is accepted as success
switchbot/devices/keypad.py:99-103

Each bound is range-checked independently, so end_time < start_time passes. add_password then writes the window, reports success, and returns an index for a passcode that can never authenticate — the caller has no signal that anything is wrong until a guest is standing at the door.

Related: add_password(passcode_type=1) with start_time/end_time left None writes a 0/0 window (always-active) while registering "4": "timeLimit" in the cloud payload (line 283-293). The app shows a time-limited credential; the keypad holds a permanent one.

Fix: in _validate_passcode_params, reject a non-zero end_time that precedes start_time, and consider requiring an explicit window when passcode_type == 1.

        if start_time is not None and not (0 <= start_time <= 0xFFFFFFFF):
            raise ValueError(f"Invalid start_time: {start_time}")

        if end_time is not None and not (0 <= end_time <= 0xFFFFFFFF):
            raise ValueError(f"Invalid end_time: {end_time}")

3. Cloud-sync rollback deletes a working passcode on any API exception, including a timeout the server may have honoured
switchbot/devices/keypad.py:316-333

The rollback fires on bare except Exception, which covers asyncio.TimeoutError, aiohttp.ClientError, and SwitchbotApiError raised for a non-100 statusCode (device.py:385-388). Two consequences:

• Transient network blip destroys a provisioned code. The BLE write already succeeded — the passcode is on the keypad and works offline. A 30s timeout on the cloud POST now deletes it, so a momentary internet outage turns a successful provisioning into nothing.
• Timeout after commit produces the inverse orphan. If the server accepted the record but the response was lost, the cloud lists a passcode that no longer exists on the device — the same app/device divergence the PR is trying to eliminate, just in the other direction.

This is a deliberate fail-closed trade-off, so it's a judgement call rather than a defect. But it's worth narrowing the rollback to failures that are unambiguously pre-commit (connection errors, 4xx) and leaving timeouts / 5xx to the caller with the index in the exception so they can retry the cloud leg instead of losing the credential.

            except Exception as err:
                _LOGGER.exception(
                    "SwitchBot Cloud sync failed for passcode at index %d. "
                    "Rolling back and deleting passcode from keypad memory.",
                    assigned_index,
                )

4. Mid-sequence chunk failure in _add_passcode_to_device leaves a partial record with no index to clean up
switchbot/devices/keypad.py:170-188

The chunk loop has no exception handling. _send_command re-raises BleakNotFoundError / CharacteristicMissingError / BLEAK_RETRY_EXCEPTIONS once retries are exhausted (device.py:407-451), so a connection drop between chunk 0 and chunk 1 propagates straight out.

At that point the keypad has received the first packet(s) of a multi-packet 570F520202 write. The index is only read from the last chunk's reply, so the caller gets a bleak error and no index — nothing can clean up whatever the device retained.

This is narrower than the time-window gap you just fixed (a partial record likely isn't a usable credential), and unverified without hardware — the firmware may well discard an incomplete sequence. Worth either confirming that on the physical device and noting it, or capturing the index from the first acknowledged reply so a mid-sequence failure is recoverable.

        result = None
        for cmd in cmds:
            result = await self._send_command(cmd)
            if not result or result[0] != 0x01:

5. delete_password returns False silently and does not range-check index
switchbot/devices/keypad.py:370-374

Two things on a credential-revocation call:

• Silent failure. A failed revocation returns False with no log line, while add_password and modify_password raise. A caller that ignores the return value believes a code was revoked when it is still active on the keypad. At minimum log at error level with the raw result; raising SwitchbotOperationError would match the rest of the class (the two rollback sites already treat the bool correctly, so they'd need a try instead).

• Unvalidated index. f"{index:02X}" with index=300 yields "12C" → an odd-length command string → an opaque ValueError: non-hexadecimal number from bytearray.fromhex in _send_command. Negative values produce a - in the hex. modify_password has the same exposure via _build_password_payload's payload.append(index) (line 131), which raises ValueError: byte must be in range(0, 256).

A 0 <= index <= 0xFF guard in both gives an actionable error instead.

    async def delete_password(self, index: int) -> bool:
        """Delete a passcode from the Keypad."""
        delete_cmd = f"570F520205{index:02X}"
        result = await self._send_command(delete_cmd)
        return bool(result and result[0] == 0x01)

6. sync_time swallows failure and accepts out-of-range timestamps
switchbot/devices/keypad.py:421-429

The seconds/milliseconds heuristic itself is sound — the 10**10 pivot sits at 1970-04-26 in ms and year 2286 in seconds, so real values land on the right side. Two rough edges around it:

• Silent False. A failed RTC write returns False with no log. A keypad left on a wrong clock makes time-limited passcodes activate or expire at the wrong times, and the only signal is a bool the caller may discard. Log at error level with the raw result.

• No range validation. start_time/end_time are range-checked but timestamp isn't. sync_time(-5) produces f"{-5000:016X}" → "-00000000001388" → an opaque fromhex ValueError; a value above 2**68 overflows past 16 hex chars and, when the total length stays even, sends a malformed 13-byte frame to the device. A 0 <= timestamp / 8-byte upper-bound check mirrors the validation the passcode window already gets.

        if timestamp is None:
            timestamp = int(time.time() * 1000)
        elif timestamp < 10000000000:
            timestamp *= 1000

        time_hex = f"{timestamp:016X}"
        cmd = f"57000501{time_hex}"
        result = await self._send_command(cmd)
        return bool(result and result[0] == 0x01)

7. Dead fallback in key_types.get masks a would-be mismatch
switchbot/devices/keypad.py:283

passcode_type is already constrained to (0, 1, 2, 3) by _validate_passcode_params, so the "permanent" default can never fire today. If the accepted set is ever widened without updating this map, a disposable or urgent code would be registered in the cloud as a permanent credential instead of failing loudly.

Index directly — key_types[passcode_type] — so the two lists cannot silently drift apart.

            key_type_str = key_types.get(passcode_type, "permanent")

8. modify_password's partial-failure state isn't communicated to the caller
switchbot/devices/keypad.py:337-368

Dropping the destructive rollback here was the right call — thanks for taking that. What's left is a reporting gap.

If the passcode chunks are accepted (line 351-357) but the time-window write then fails (line 364-368), the caller gets "Failed to set active time window for passcode (result=...)". The PIN has already been changed on the device; the old code no longer works and the new one is live with whatever window was previously configured. The message reads like the modify didn't take effect.

A one-line docstring addition or a message that distinguishes "no chunk acknowledged → nothing changed" from "passcode at index N was updated but its window was not" would let a caller decide whether to retry the window write or delete the credential.

        time_result = await self._send_command(time_cmd)
        if not time_result or time_result[0] != 0x01:
            result_hex = time_result.hex() if time_result else "None"
            raise SwitchbotOperationError(
                f"Failed to set active time window for passcode (result={result_hex})"
            )

9. ~90 lines duplicated from keypad_vision.py, already diverging
switchbot/devices/keypad.py:123-168

PASSWORD_RE, COMMAND_GET_PASSWORD_COUNT, _check_password_rules, _build_password_payload, _build_add_password_cmd and get_password_count are near-verbatim copies of switchbot/devices/keypad_vision.py:12-13, 81-127, 136-167.

The copies have already drifted, and this file's versions are the better ones:

• get_password_count here guards len(_data) < 6 (the PR fix: guard device get_basic_info parsers against short responses #500 truncated-response class); keypad_vision.py:137-146 still indexes _data[1].._data[5] unguarded and will IndexError on a short reply.
• _build_password_payload here is parameterised on index/passcode_type; the vision copy hardcodes 0xFF/0x00.

Since the wire format is identical, lifting the shared helpers into a common base (or having SwitchbotKeypadVision reuse them) would fix the vision truncation gap for free and stop the next protocol tweak from landing in only one of the two files.

    def _build_password_payload(
        self, password: str, passcode_type: int, index: int
    ) -> bytes:

---

Checklist

• Prior blocking findings resolved
• Input validation at boundaries — suggestion ＃1, suggestion ＃2, suggestion ＃5, suggestion ＃6
• Error handling and failure-path recovery — suggestion ＃3, suggestion ＃4, suggestion ＃8
• No hardcoded secrets or credential leakage
• Protocol encoding matches asserted command bytes
• Truncated/short BLE response guards
• Test coverage for new branches (33/33 pass)
• No duplicated logic across device modules — suggestion ＃9
• Diff matches PR description, no scope creep

---

Silent Failure Analysis

🟠 **HIGH** — silent no-op from truthiness/None mismatch
switchbot/devices/keypad.py:280

Risk: _validate_cloud_credentials gates on p is not None, but the sync block gates on truthiness, so an empty-string token (or any falsy-but-present credential) passes validation and then silently skips the cloud call — add_password returns an index as if the passcode was fully synced, leaving the app and device permanently out of sync with no log or error.

        # Sync to SwitchBot Cloud if credentials are provided
        if session and token and region:
            clean_mac = self._device.address.replace(":", "")...

Fix: Use the same predicate in both places (if session is not None and token is not None and region is not None:), and reject empty-string credentials in _validate_cloud_credentials.

🟡 **MEDIUM** — missing status-byte check / error frame parsed as data
switchbot/devices/keypad.py:376-392

Risk: Unlike every other command handler in this class (result[0] != 0x01), get_password_count never validates the status byte _data[0], so any ≥6-byte error/NACK frame is decoded as legitimate credential counts and returned to the caller as valid data.

        if len(_data) < 6:
            ...
            return None
        pin = _data[1]
        nfc = _data[2]
        fingerprint = _data[3]

Fix: Check _data[0] == 0x01 (or reuse _check_command_result) before parsing, and return None / raise on a non-success status.

🟡 **MEDIUM** — silent null return on error path
switchbot/devices/keypad.py:378-379

Risk: A failed or empty BLE response is converted to None with no log at all (the truncated branch two lines below does log), so a device that consistently fails this query is indistinguishable from one that simply has no data.

        if not (_data := await self._send_command(COMMAND_GET_PASSWORD_COUNT)):
            return None

Fix: Log at error/warning level on the empty-response path, mirroring the truncated-response branch.

🟡 **MEDIUM** — failure signalled only by a discardable bool
switchbot/devices/keypad.py:370-374

Risk: A failed deletion returns False with no log and no exception while sibling mutators (add_password, modify_password) raise, so a caller that ignores the return value believes a passcode was revoked when it still opens the door.

    async def delete_password(self, index: int) -> bool:
        delete_cmd = f"570F520205{index:02X}"
        result = await self._send_command(delete_cmd)
        return bool(result and result[0] == 0x01)

Fix: Log the failing response and either raise SwitchbotOperationError for consistency with the other mutators or document the bool contract prominently in the docstring.

🟡 **MEDIUM** — exception chain suppressed (`raise ... from None`)
switchbot/devices/keypad.py:255-264

Risk: On the non-exception path _rollback_time_window() is called with err=None, making this raise ... from None, which sets __suppress_context__ and strips the caught rollback exception from the traceback the caller sees.

            except Exception:
                _LOGGER.exception("Failed to delete passcode ... rollback ...")
            if not success:
                raise SwitchbotOperationError(...) from err

Fix: Capture the rollback exception and chain it (raise ... from rollback_err or err), or only pass from err when err is not None.

🟡 **MEDIUM** — partial failure reported as total failure
switchbot/devices/keypad.py:352-368

Risk: In modify_password the passcode value has already been overwritten on the device when the time-window write fails, but the error (unlike add_password's rollback path) neither rolls back nor states that the code was changed, so the caller assumes the old passcode is still in effect.

        time_result = await self._send_command(time_cmd)
        if not time_result or time_result[0] != 0x01:
            raise SwitchbotOperationError(
                f"Failed to set active time window for passcode (result={result_hex})"
            )

Fix: Either roll back / retry the window write, or make the error message state that the passcode at that index was already modified and its validity window is now indeterminate.

---

Automated review by Kōan (Claude) HEAD=dde5ed1 6 min 40s

[bluetoothbot]

Tip

No blocking issues found — ready to merge.