Capture the topic most device replies arrive on - #126
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Two descriptions incorrectly state that energy replies require the new wildcard.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds the missing device-scoped MQTT wildcard so firmware captures include device responses.
Changes:
- Subscribes to
cmd/{type}/navilink-{mac}/#. - Documents the capture improvement in the changelog.
- Validation not run during review.
File summaries
| File | Description |
|---|---|
examples/advanced/firmware_payload_capture.py |
Captures device-path MQTT traffic. |
CHANGELOG.rst |
Records the wildcard fix. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Everything under the device's own command path. This is where the | ||
| # device acknowledges control and query commands, on | ||
| # cmd/{type}/navilink-{mac}/{client_id}/res - status, device info and | ||
| # energy replies all land here, so a capture without this subscription | ||
| # misses most of the traffic. It also picks up requests and replies | ||
| # belonging to other clients on the same device (a Home Assistant | ||
| # integration, the vendor app), which is exactly what a capture wants. |
There was a problem hiding this comment.
Correct — fixed in de68c81. I checked it against the capture rather than the code, and the routing agrees with you:
cmd/52/navien-client-XXXX/res/energy-usage-daily-query/rd already covered
cmd/52/navilink-XXXX/st/energy-usage-daily-query/rd request echo, device path
cmd/52/navilink-XXXX/navien-client-XXXX/res status + device info, device path
Only the energy request echo is on the device path; the reply was already matched by the response wildcard, same as reservations. Both descriptions now name what the device path actually adds — the control/query acknowledgement carrying status and device info, the published requests, and other clients' traffic — and say explicitly that client-keyed query results were already covered.
| acknowledges control and query commands on | ||
| ``cmd/{type}/navilink-{mac}/{client_id}/res`` - which neither pattern | ||
| matches - so device status, device info and energy replies were all missed. |
There was a problem hiding this comment.
Correct — fixed in de68c81. I checked it against the capture rather than the code, and the routing agrees with you:
cmd/52/navien-client-XXXX/res/energy-usage-daily-query/rd already covered
cmd/52/navilink-XXXX/st/energy-usage-daily-query/rd request echo, device path
cmd/52/navilink-XXXX/navien-client-XXXX/res status + device info, device path
Only the energy request echo is on the device path; the reply was already matched by the response wildcard, same as reservations. Both descriptions now name what the device path actually adds — the control/query acknowledgement carrying status and device info, the published requests, and other clients' traffic — and say explicitly that client-keyed query results were already covered.
6d7fd96 to
4279a09
Compare
de68c81 to
b443ec8
Compare
4279a09 to
4d96cd5
Compare
b443ec8 to
4014414
Compare
firmware_payload_capture.py subscribed to cmd/{type}/{client_id}/res/#
and the event topic. Neither matches
cmd/{type}/navilink-{mac}/{client_id}/res, which is where the device
acknowledges control and query commands - so device status, device info
and energy replies never reached the capture file, and the tool recorded
2 payloads where it should have recorded 11.
Subscribing to cmd/{type}/navilink-{mac}/# as well fixes it, and has the
side benefit of picking up requests and replies belonging to other
clients on the same device, which is what a capture tool wants.
Verified against the device: the same run now records the feature and
status responses it was silently dropping.
Claude-Session: https://claude.ai/code/session_01XVj9BYvuLj7Th3iFUVoeCn
The energy reply is routed to the client-keyed
cmd/{type}/{client_id}/res/energy-usage-daily-query/rd, which the
existing response wildcard already matched; only its request echo is on
the device path. What the device path actually adds is the control/query
acknowledgement carrying status and device info, the published requests,
and other clients' traffic.
Claude-Session: https://claude.ai/code/session_01XVj9BYvuLj7Th3iFUVoeCn
4014414 to
7dd334b
Compare
Problem
examples/advanced/firmware_payload_capture.pysubscribed to two wildcards:cmd/{type}/{client_id}/res/#— query results the device routes back to this clientevt/{type}/navilink-{mac}/#— device eventsNeither matches
cmd/{type}/navilink-{mac}/{client_id}/res, the topic where the device acknowledges control and query commands — the status and device-info responses. Nothing published on the device path was captured either, requests included.A real run recorded 2 payloads: the two reservation responses, which use the client-keyed topic. Everything else was dropped silently, including the device-info response the script itself prints the firmware version from.
Fix
Also subscribe to
cmd/{type}/navilink-{mac}/#. Same run now records 11 payloads, including the feature and status responses. As a bonus it picks up requests and replies belonging to other clients on the same device (a Home Assistant integration, the vendor app), which is what you want from a capture tool.Query results the device routes to a client-keyed topic — reservations, energy usage — were already covered by the existing response wildcard and are unaffected.
Verified against a live device.