fix: correct LocalCommsStatus field semantics and add missing proto fields#18
Merged
Conversation
…oller - WifiInfo (QSM) and Controller now surface bssid and frequency_mhz from the shared WifiState wire message (both fields already present on the wire for QSMs, not just Controllers) via parse_wifi_state(). Adds a WifiInfo.band property mirroring Controller.wifi_band. - QuiltSmartModule and Controller now expose raw local_comms_link_state, local_comms_connection_state, and local_comms_version alongside the existing local_comms_health enum, for richer local-mesh diagnostics. Found while investigating why an over-the-air 2.4 GHz packet capture showed zero local-mesh traffic: every device with a healthy local-comms link is actually associated on 5 GHz. Having bssid/frequency in the model would have surfaced this without a packet capture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ields Verified the library against the Android app (com.quilt.android 1.0.29) by decoding the compiled protobuf newMessageInfo metadata. LocalCommsStatus fields 3/4/6 were mislabeled from limited capture guesses: - f3 link_state -> visible_devices_count (mesh peers visible) - f4 version -> expected_devices_count (mesh peers expected) - f6 connection_state -> reason (LocalCommsHealthReason enum, APK-confirmed) - f5 health_changed_ts -> last_session_change_ts - f2 health -> status (proto field name) Also: - Add LocalCommsHealthReason enum (12 diagnostic values; the local mesh is Eclipse Zenoh-based, not NATS). - Add IndoorUnitConditions.compressor_minimum_run_time (proto field 13, APK-confirmed 1.0.29). - Mark SpaceSettings.safety_heating=9 as APK-confirmed (was "unconfirmed"). - Regenerate proto stubs; update QSM/Controller models, docstrings, and tests.
There was a problem hiding this comment.
Pull request overview
Updates the client’s protobuf and model layer to match APK-confirmed field numbers/semantics for local communications health, and exposes the newly verified diagnostics and missing proto fields in the public Python models.
Changes:
- Correct
LocalCommsStatussubfield names/semantics and add the newLocalCommsHealthReasonenum end-to-end (proto → generated stubs → models → tests). - Expose LocalComms mesh diagnostics on
QuiltSmartModuleandController(visible/expected device counts, diagnostic reason, last session change timestamp). - Add the missing
IndoorUnitConditions.compressor_minimum_run_timefield and update tests/stubs accordingly.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_models_from_proto.py | Updates proto stubs and assertions for corrected LocalComms fields + new compressor condition field. |
| src/quilt_hp/models/qsm.py | Adds local comms diagnostic fields and augments WiFi parsing to include BSSID/frequency. |
| src/quilt_hp/models/controller.py | Adds local comms diagnostic fields and surfaces WiFi BSSID/frequency from WifiState. |
| src/quilt_hp/models/indoor_unit.py | Adds compressor_minimum_run_time to conditions model and activation checks. |
| src/quilt_hp/models/_helpers.py | Adds local_comms_last_session_change() and expands parse_wifi_state() return shape. |
| src/quilt_hp/models/enums.py | Introduces LocalCommsHealthReason enum. |
| src/quilt_hp/models/init.py | Exports LocalCommsHealthReason from the models package. |
| proto/cleaned/quilt_hds.proto | Adds LocalCommsHealthReason; corrects LocalCommsStatus field definitions; confirms SafetyHeatingMode f9; adds compressor runtime condition. |
| src/quilt_hp/_proto/quilt_hds_pb2.pyi | Regenerated typing stubs reflecting proto changes. |
| src/quilt_hp/_proto/quilt_hds_pb2.py | Regenerated protobuf module reflecting proto changes. |
…comms-audit # Conflicts: # src/quilt_hp/models/_helpers.py # src/quilt_hp/models/controller.py # src/quilt_hp/models/qsm.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cross-checked the library against the Android app (com.quilt.android 1.0.29, versionCode 242) by decompiling with jadx and decoding the compiled protobuf
newMessageInfometadata for exact field numbers. All enum values and control-write field numbers were verified correct; this PR fixes the semantic errors and missing fields that verification surfaced.Fixes
LocalCommsStatusfields were mislabeled (guessed from limited captures). The compiled message (classl62) proves:healthstatuslink_state(int32)visible_devices_count(int32)version(int32)expected_devices_count(int32)health_changed_tslast_session_change_tsconnection_state(int32)reason(LocalCommsHealthReasonenum)The "HEALTHY=9/DEGRADED=8" and "always 9" observations were device counts; the "connection_state" int was actually a diagnostic reason enum (HEALTHY=1, DEGRADED=5=PARTIAL_VISIBILITY). Decoding still worked by field number, but the model exposed wrong names/semantics and discarded the reason diagnostics.
Also:
LocalCommsHealthReasonenum (12 APK-confirmed values).ZENOH_SESSION_DOWNidentifies the local mesh transport as Eclipse Zenoh, not NATS (port 7447 is Zenoh's default).IndoorUnitConditions.compressor_minimum_run_time(proto field 13, APK-confirmed 1.0.29).SpaceSettings.safety_heating=9APK-confirmed (was "unconfirmed").local_comms_visible_devices,local_comms_expected_devices,local_comms_reason,local_comms_last_session_change), docstrings, and tests.Validation