From b03716b97d552af2acbc3b0ceb9209ea0c2cf36e Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 1 May 2026 12:28:34 +0200 Subject: [PATCH 1/2] chore(ctw3): add diagnostics to pinpoint detect_status offset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #65: drink-events sensor never increments because the parser reads detect_status from CMD 210 byte 19, which appears to always be 0 on CTW3 firmware 111. The ground-truth log captured by the user (cat Milka drinking; HA UX showed 'detected' then back to 'clear') contains a single post-drink frame whose byte 26 jumps to 0xc5 vs 0x08 in idle frames — strongly suggesting the real detect signal lives in the currently-unparsed bytes 26..29. To gather conclusive evidence without risking a regression on W4/W5/CTW2, this commit ships diagnostic instrumentation only: * New raw_state and state_tail fields on PetkitFountainData populated by both CTW3 and generic CMD 210 parsers. * Coordinator logs a DEBUG-level byte-by-byte diff between consecutive CMD 210 polls, skipping the always-changing uptime tick at bytes 9..18 so semantically meaningful changes (e.g. pet-detection events) stand out. * New hidden DIAGNOSTIC sensor sensor._state_tail_hex exposes bytes 26..29 of CTW3 30-byte frames so users can graph their behaviour in HA history without grepping logs. Refs #65 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- custom_components/petkit_ble/ble_client.py | 19 ++++++ custom_components/petkit_ble/coordinator.py | 50 +++++++++++++++ custom_components/petkit_ble/manifest.json | 2 +- custom_components/petkit_ble/sensor.py | 8 +++ .../petkit_ble/translations/en.json | 3 +- .../petkit_ble/translations/nl.json | 3 +- .../petkit_ble/translations/uk.json | 3 +- tests/test_data_model.py | 33 ++++++++++ tests/test_state_diff.py | 62 +++++++++++++++++++ 9 files changed, 179 insertions(+), 4 deletions(-) create mode 100644 tests/test_state_diff.py diff --git a/custom_components/petkit_ble/ble_client.py b/custom_components/petkit_ble/ble_client.py index 8bc0b51..8ea4484 100644 --- a/custom_components/petkit_ble/ble_client.py +++ b/custom_components/petkit_ble/ble_client.py @@ -113,6 +113,19 @@ class PetkitFountainData: battery_work_time: int = 0 battery_sleep_time: int = 0 + # Raw CMD 210 payload as last received. Kept so the coordinator can log + # a byte-by-byte diff between consecutive polls — a diagnostic aid for + # reverse-engineering CTW3 fields whose offsets are not yet known + # (notably ``detect_status``; see issue #65). + raw_state: bytes = b"" + + # Bytes 26..29 of the CTW3 30-byte CMD 210 payload. Currently unparsed + # — exposed via a hidden diagnostic sensor so users can graph their + # behaviour while we narrow down which byte carries the real + # ``detect_status``. Always empty for non-CTW3 devices and for older + # CTW3 firmware that returns only 26 bytes. + state_tail: bytes = b"" + # True once a CMD 211 (read settings) response has been parsed at least # once for this entry. Some firmware revisions never reply to CMD 211 # (observed on CTW3 fw 111). When this flag is False, the cached @@ -358,6 +371,11 @@ def _parse_state_ctw3(data: PetkitFountainData, payload: bytes) -> None: if len(payload) < 26: _LOGGER.warning("CTW3 state payload too short: %d bytes", len(payload)) return + data.raw_state = bytes(payload) + # Bytes 26..29 are not yet decoded by this parser — see issue #65. + # Captured raw so the user (and future maintainers) can correlate + # them with observed pet-detection events via the diagnostic sensor. + data.state_tail = bytes(payload[26:30]) if len(payload) >= 30 else b"" data.power_status = payload[0] data.suspend_status = payload[1] # The CTW3 firmware has been observed to transiently report mode=0 during @@ -412,6 +430,7 @@ def _parse_state_generic(data: PetkitFountainData, payload: bytes) -> None: if len(payload) < 12: _LOGGER.warning("State payload too short: %d bytes", len(payload)) return + data.raw_state = bytes(payload) data.power_status = payload[0] data.mode = payload[1] data.dnd_state = payload[2] diff --git a/custom_components/petkit_ble/coordinator.py b/custom_components/petkit_ble/coordinator.py index 8bbdf81..8a363ba 100644 --- a/custom_components/petkit_ble/coordinator.py +++ b/custom_components/petkit_ble/coordinator.py @@ -100,6 +100,37 @@ def _reconcile_settings_into( return warned +# Byte indices in the CMD 210 payload that are known to change every poll +# (running-uptime ms tick / sequence counter on CTW3 firmware 111). Excluded +# from the diff log so the diff highlights only semantically-meaningful +# changes — making it easy to spot which byte carries an event signal such +# as pet-detection (see issue #65). +_CTW3_NOISE_BYTES: frozenset[int] = frozenset(range(9, 19)) + + +def _diff_state_bytes( + prev: bytes, + curr: bytes, + *, + noisy: frozenset[int] = _CTW3_NOISE_BYTES, +) -> list[tuple[int, int, int]]: + """Return ``(index, old, new)`` triples for bytes that changed. + + Indices listed in ``noisy`` are skipped. If the two payloads have + different lengths the longer one is treated as ground truth and the + appended bytes are reported as additions from ``0x00``. + """ + if not prev: + return [] + out: list[tuple[int, int, int]] = [] + for i in range(min(len(prev), len(curr))): + if i in noisy: + continue + if prev[i] != curr[i]: + out.append((i, prev[i], curr[i])) + return out + + @dataclass class _DrinkCountState: """Mutable holder for the daily drink-event counter. @@ -221,6 +252,11 @@ def __init__(self, hass: HomeAssistant, config_entry: ConfigEntry) -> None: self._settings_cache: dict[str, int] = {} self._warned_no_config: bool = False + # Previous CMD 210 raw payload, kept so we can log a byte-by-byte + # diff between consecutive polls (see _diff_state_bytes). Used as a + # diagnostic aid for issue #65 (CTW3 detect_status offset unknown). + self._prev_raw_state: bytes = b"" + super().__init__( hass, _LOGGER, @@ -375,6 +411,20 @@ async def _async_update_data(self) -> PetkitFountainData: "Polled %s: power=%s mode=%s firmware=%s", self._name, data.power_status, data.mode, data.firmware ) + # Diagnostic: log changed bytes between consecutive CMD 210 polls. + # Only emitted when DEBUG logging is enabled. Skips the noisy + # uptime/tick bytes 9..18 so the diff highlights semantic changes + # such as pet-detection events (issue #65). + if _LOGGER.isEnabledFor(logging.DEBUG) and data.raw_state: + diff = _diff_state_bytes(self._prev_raw_state, data.raw_state) + if diff: + _LOGGER.debug( + "CMD 210 state diff for %s: %s", + self._name, + " ".join(f"byte[{i}]=0x{old:02x}->0x{new:02x}" for i, old, new in diff), + ) + self._prev_raw_state = data.raw_state + self._reconcile_settings(data) # Self-heal persistence: if the BLE client inferred a corrected alias diff --git a/custom_components/petkit_ble/manifest.json b/custom_components/petkit_ble/manifest.json index 02d9b0b..6e54d52 100644 --- a/custom_components/petkit_ble/manifest.json +++ b/custom_components/petkit_ble/manifest.json @@ -1,7 +1,7 @@ { "domain": "petkit_ble", "name": "Petkit BLE", - "version": "1.1.12", + "version": "1.1.13", "config_flow": true, "documentation": "https://github.com/aavdberg/ha-petkit", "issue_tracker": "https://github.com/aavdberg/ha-petkit/issues", diff --git a/custom_components/petkit_ble/sensor.py b/custom_components/petkit_ble/sensor.py index 9454d70..10baaf2 100644 --- a/custom_components/petkit_ble/sensor.py +++ b/custom_components/petkit_ble/sensor.py @@ -135,6 +135,14 @@ class PetkitSensorEntityDescription(SensorEntityDescription): value_fn=lambda d: d.drink_event_count, available_fn=lambda d: d.is_ctw3, ), + PetkitSensorEntityDescription( + key="state_tail_hex", + translation_key="state_tail_hex", + entity_category=EntityCategory.DIAGNOSTIC, + entity_registry_enabled_default=False, + value_fn=lambda d: d.state_tail.hex() if d.state_tail else None, + available_fn=lambda d: d.is_ctw3 and bool(d.state_tail), + ), ) diff --git a/custom_components/petkit_ble/translations/en.json b/custom_components/petkit_ble/translations/en.json index c59c9f9..377349b 100644 --- a/custom_components/petkit_ble/translations/en.json +++ b/custom_components/petkit_ble/translations/en.json @@ -57,7 +57,8 @@ "firmware": {"name": "Firmware"}, "hardware_version": {"name": "Hardware Version"}, "rssi": {"name": "Signal Strength"}, - "drink_count": {"name": "Drink Events Today"} + "drink_count": {"name": "Drink Events Today"}, + "state_tail_hex": {"name": "CTW3 State Tail (hex)"} }, "binary_sensor": { "pump_running": {"name": "Pump Running"}, diff --git a/custom_components/petkit_ble/translations/nl.json b/custom_components/petkit_ble/translations/nl.json index 6395002..ecaa650 100644 --- a/custom_components/petkit_ble/translations/nl.json +++ b/custom_components/petkit_ble/translations/nl.json @@ -57,7 +57,8 @@ "firmware": {"name": "Firmware"}, "hardware_version": {"name": "Hardwareversie"}, "rssi": {"name": "Signaalsterkte"}, - "drink_count": {"name": "Drinkgebeurtenissen vandaag"} + "drink_count": {"name": "Drinkgebeurtenissen vandaag"}, + "state_tail_hex": {"name": "CTW3 statestaart (hex)"} }, "binary_sensor": { "pump_running": {"name": "Pomp actief"}, diff --git a/custom_components/petkit_ble/translations/uk.json b/custom_components/petkit_ble/translations/uk.json index 0057fb6..5257675 100644 --- a/custom_components/petkit_ble/translations/uk.json +++ b/custom_components/petkit_ble/translations/uk.json @@ -57,7 +57,8 @@ "firmware": {"name": "Прошивка"}, "hardware_version": {"name": "Версія апаратного забезпечення"}, "rssi": {"name": "Рівень сигналу"}, - "drink_count": {"name": "Випадки пиття сьогодні"} + "drink_count": {"name": "Випадки пиття сьогодні"}, + "state_tail_hex": {"name": "Хвіст стану CTW3 (hex)"} }, "binary_sensor": { "pump_running": {"name": "Насос працює"}, diff --git a/tests/test_data_model.py b/tests/test_data_model.py index dbf9dd2..ed4dc25 100644 --- a/tests/test_data_model.py +++ b/tests/test_data_model.py @@ -154,6 +154,39 @@ def test_parse_state_ctw3_accepts_valid_mode_change(self, sample_ctw3_state_payl PetkitBleClient._parse_state_ctw3(data, bytes(buf)) assert data.mode == 1 + def test_parse_state_ctw3_captures_state_tail_when_30_bytes(self) -> None: + """30-byte CTW3 payload populates state_tail and raw_state. + + The ground-truth log captured for issue #65 contains 30-byte frames + whose trailing four bytes (26..29) are not yet decoded but vary + between idle and post-drink samples — see plan.md. + """ + # Synthesised from real CTW3 fw 111 frame at 12:14:57: + # raw=01010102000000000000245787080100009b3d00141a10736400c506be06 + raw = bytes.fromhex("01010102000000000000245787080100009b3d00141a10736400c506be06") + data = PetkitFountainData(alias=ALIAS_CTW3) + PetkitBleClient._parse_state_ctw3(data, raw) + assert data.raw_state == raw + assert data.state_tail == bytes.fromhex("c506be06") + # All previously-known fields keep their meaning + assert data.power_status == 1 + assert data.battery_percent == 100 + assert data.filter_percent == 8 + + def test_parse_state_ctw3_state_tail_empty_for_26_byte_payload(self, sample_ctw3_state_payload: bytes) -> None: + """Older CTW3 firmware returns 26 bytes with no trailing tail.""" + data = PetkitFountainData(alias=ALIAS_CTW3) + PetkitBleClient._parse_state_ctw3(data, sample_ctw3_state_payload) + assert data.raw_state == sample_ctw3_state_payload + assert data.state_tail == b"" + + def test_parse_state_generic_does_not_set_state_tail(self, sample_generic_state_payload: bytes) -> None: + """Non-CTW3 payloads never populate the CTW3-specific tail.""" + data = PetkitFountainData(alias=ALIAS_W5) + PetkitBleClient._parse_state_generic(data, sample_generic_state_payload) + assert data.raw_state == sample_generic_state_payload + assert data.state_tail == b"" + def test_parse_state_generic(self, sample_generic_state_payload: bytes) -> None: """Parse a generic state payload and verify fields.""" data = PetkitFountainData(alias=ALIAS_W5) diff --git a/tests/test_state_diff.py b/tests/test_state_diff.py new file mode 100644 index 0000000..ed83dac --- /dev/null +++ b/tests/test_state_diff.py @@ -0,0 +1,62 @@ +"""Tests for the CMD 210 byte-diff diagnostic helper.""" + +from __future__ import annotations + +from custom_components.petkit_ble.coordinator import _diff_state_bytes + + +class TestDiffStateBytes: + """Tests for the byte-diff helper used by the diagnostic poll log.""" + + def test_returns_empty_when_prev_is_empty(self) -> None: + """Initial poll has no previous frame to diff against.""" + assert _diff_state_bytes(b"", b"\x01\x02") == [] + + def test_returns_empty_when_payloads_identical(self) -> None: + """No bytes changed → empty diff.""" + payload = bytes.fromhex("01020304") + assert _diff_state_bytes(payload, payload) == [] + + def test_reports_changed_byte_with_old_and_new(self) -> None: + """Each diff entry is (index, old, new).""" + prev = bytes.fromhex("0008072307") + curr = bytes.fromhex("c506be0607") + # Indices 0..3 differ; index 4 is the same. + assert _diff_state_bytes(prev, curr) == [ + (0, 0x00, 0xC5), + (1, 0x08, 0x06), + (2, 0x07, 0xBE), + (3, 0x23, 0x06), + ] + + def test_skips_noisy_byte_indices(self) -> None: + """Bytes 9..18 (CTW3 uptime tick) are excluded from the diff.""" + prev = bytearray(30) + curr = bytearray(30) + # Change bytes inside the noisy window — must NOT appear. + prev[10] = 0x01 + curr[10] = 0xFF + prev[18] = 0x00 + curr[18] = 0x42 + # Change a byte outside the noisy window — MUST appear. + prev[26] = 0x08 + curr[26] = 0xC5 + diff = _diff_state_bytes(bytes(prev), bytes(curr)) + assert diff == [(26, 0x08, 0xC5)] + + def test_real_ctw3_frame_pair_highlights_byte_26(self) -> None: + """Real captured frames before/after a drink event. + + Frames sourced from + ``Logs/home-assistant_petkit_ble_2026-05-01T10-15-53.221Z.log``. + With the noisy uptime bytes (9..18) suppressed, byte 26 jumping + from 0x08 to 0xc5 stands out as the strongest pet-detection + candidate (see plan.md / issue #65). + """ + prev = bytes.fromhex("01010102000000000000242bfe080100006fb40014171076640008072307") + curr = bytes.fromhex("01010102000000000000245787080100009b3d00141a10736400c506be06") + diff = _diff_state_bytes(prev, curr) + # Diff must include byte 26 (0x08 -> 0xc5). + assert (26, 0x08, 0xC5) in diff + # And must exclude every byte inside the noisy uptime window. + assert all(i not in range(9, 19) for i, _, _ in diff) From e948fa04f7bb15f3e21ee36898cb60b886f2cf6e Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 1 May 2026 12:34:07 +0200 Subject: [PATCH 2/2] review: handle length-mismatched payloads and skip noise filter outside CTW3 Address Copilot review comments on PR #68: * _diff_state_bytes now reports appended/truncated bytes against 0x00 for both growing and shrinking payloads, matching the behaviour the docstring promised. Two new unit tests pin the contract. * Coordinator only applies the CTW3 uptime-noise filter (bytes 9..18) for CTW3 devices. On 12-byte W4/W5/CTW2 payloads those indices carry pump_runtime tail / filter_percent / running_status, so suppressing them would have made the diagnostic misleading. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- custom_components/petkit_ble/coordinator.py | 30 ++++++++++++++------- tests/test_state_diff.py | 22 +++++++++++++++ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/custom_components/petkit_ble/coordinator.py b/custom_components/petkit_ble/coordinator.py index 8a363ba..aece35b 100644 --- a/custom_components/petkit_ble/coordinator.py +++ b/custom_components/petkit_ble/coordinator.py @@ -116,18 +116,21 @@ def _diff_state_bytes( ) -> list[tuple[int, int, int]]: """Return ``(index, old, new)`` triples for bytes that changed. - Indices listed in ``noisy`` are skipped. If the two payloads have - different lengths the longer one is treated as ground truth and the - appended bytes are reported as additions from ``0x00``. + Indices listed in ``noisy`` are skipped. When the two payloads have + different lengths the missing positions are treated as ``0x00`` so + appended/truncated bytes (e.g. the CTW3 tail at indices 26..29 that + appears only in 30-byte frames) are still reported. """ if not prev: return [] out: list[tuple[int, int, int]] = [] - for i in range(min(len(prev), len(curr))): + for i in range(max(len(prev), len(curr))): if i in noisy: continue - if prev[i] != curr[i]: - out.append((i, prev[i], curr[i])) + old = prev[i] if i < len(prev) else 0 + new = curr[i] if i < len(curr) else 0 + if old != new: + out.append((i, old, new)) return out @@ -412,11 +415,18 @@ async def _async_update_data(self) -> PetkitFountainData: ) # Diagnostic: log changed bytes between consecutive CMD 210 polls. - # Only emitted when DEBUG logging is enabled. Skips the noisy - # uptime/tick bytes 9..18 so the diff highlights semantic changes - # such as pet-detection events (issue #65). + # Only emitted when DEBUG logging is enabled. For CTW3, skip the + # noisy uptime/tick bytes 9..18 so the diff highlights semantic + # changes such as pet-detection events (issue #65). For W4/W5/CTW2 + # the payload is only 12 bytes long and indices 9..11 carry + # meaningful state (pump_runtime tail, filter_percent, + # running_status) so we must not suppress them. if _LOGGER.isEnabledFor(logging.DEBUG) and data.raw_state: - diff = _diff_state_bytes(self._prev_raw_state, data.raw_state) + diff = _diff_state_bytes( + self._prev_raw_state, + data.raw_state, + noisy=_CTW3_NOISE_BYTES if data.is_ctw3 else frozenset(), + ) if diff: _LOGGER.debug( "CMD 210 state diff for %s: %s", diff --git a/tests/test_state_diff.py b/tests/test_state_diff.py index ed83dac..2285f9f 100644 --- a/tests/test_state_diff.py +++ b/tests/test_state_diff.py @@ -60,3 +60,25 @@ def test_real_ctw3_frame_pair_highlights_byte_26(self) -> None: assert (26, 0x08, 0xC5) in diff # And must exclude every byte inside the noisy uptime window. assert all(i not in range(9, 19) for i, _, _ in diff) + + def test_reports_appended_tail_bytes_when_payload_grows(self) -> None: + """If a poll returns more bytes than the previous one, the + appended indices must surface in the diff (treated as 0x00 -> new). + + Guards the documented behaviour against a regression on CTW3 + firmware revisions that may switch between 26- and 30-byte CMD 210 + responses. + """ + prev = bytes(range(20)) # 20 bytes + curr = prev + b"\xc5\x06\xbe\x06" # 24 bytes + # Disable the noisy filter so this test exercises only the + # length-mismatch behaviour. + diff = _diff_state_bytes(prev, curr, noisy=frozenset()) + assert diff == [(20, 0x00, 0xC5), (21, 0x00, 0x06), (22, 0x00, 0xBE), (23, 0x00, 0x06)] + + def test_reports_truncated_tail_bytes_when_payload_shrinks(self) -> None: + """Symmetric guard for shrinking payloads — old bytes vs 0x00.""" + prev = bytes([0x10, 0x20, 0x30, 0x40]) + curr = bytes([0x10, 0x20]) + diff = _diff_state_bytes(prev, curr, noisy=frozenset()) + assert diff == [(2, 0x30, 0x00), (3, 0x40, 0x00)]