Fix schedule read and write timeouts#93
Open
eman wants to merge 19 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate schedule read/write timeouts by improving how queued schedule commands match incoming pump responses in the alpha_hwr transport layer.
Changes:
- Updates schedule operations to use wildcard response matching (
expect_obj_id=0, expect_sub_id=0) rather than incorrectly keying matching off type bytes like0xDE01/0xDA01/0xDC01. - Simplifies the short Class 10 ACK matching logic in
Transport::try_dispatch_response()by removing a hardcoded whitelist and relying on wildcard/flag-based matching.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| components/alpha_hwr/transport.cpp | Loosens/simplifies short Class 10 ACK matching logic for queued commands. |
| components/alpha_hwr/schedule_service.cpp | Changes schedule read/write calls to pass (0,0) for expected Obj/Sub to rely on wildcard response matching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
141
to
143
| this->transport_.send_apdu_command( | ||
| apdu, 5, 0xDA01, 0, | ||
| apdu, 5, 0, 0, | ||
| [this](bool success, const uint8_t *payload, size_t payload_len) { |
Comment on lines
313
to
316
| this->transport_.send_apdu_command( | ||
| apdu, 5, 0xDE01, 0, | ||
| apdu, 5, 0, 0, | ||
| [this, entries, layer](bool success, const uint8_t *payload, | ||
| size_t payload_len) { |
Comment on lines
439
to
442
| this->transport_.send_apdu_command( | ||
| apdu, 5, 0xDE01, 0, | ||
| apdu, 5, 0, 0, | ||
| [this, on_complete, layer](bool success, const uint8_t *payload, | ||
| size_t payload_len) { |
Comment on lines
922
to
925
| this->transport_.send_apdu_command( | ||
| apdu, 5, 0xDC01, 0, | ||
| apdu, 5, 0, 0, | ||
| [this, idx, events, on_complete, max_events, | ||
| read_next](bool success, const uint8_t *payload, size_t payload_len) { |
Comment on lines
569
to
571
| this->transport_.send_apdu_command( | ||
| apdu, sizeof(apdu), 0xDE01, 0, | ||
| apdu, sizeof(apdu), 0, 0, | ||
| [on_complete, layer](bool success, const uint8_t *data, size_t len) { |
Comment on lines
869
to
872
| this->transport_.send_apdu_command( | ||
| apdu, sizeof(apdu), 0xDE01, 0, | ||
| apdu, sizeof(apdu), 0, 0, | ||
| [this, on_complete, layer](bool success, const uint8_t *data, | ||
| size_t len) { |
Comment on lines
971
to
974
| this->transport_.send_apdu_command( | ||
| apdu, sizeof(apdu), 0xDC01, 0, | ||
| apdu, sizeof(apdu), 0, 0, | ||
| [this, on_complete, event](bool success, const uint8_t *data, | ||
| size_t len) { |
Comment on lines
374
to
376
| if (queued_class == 0x0A && len >= 6 && data[4] == 0x0A && (data[5] == 0x01 || data[5] == 0x81) && | ||
| cmd.expect_obj_id == 0x0000 && cmd.expect_sub_id == 0x0000 && | ||
| cmd.packet.size() > 9 && | ||
| (cmd.packet[5] == 0x97 || cmd.packet[5] == 0x96 || cmd.packet[5] == 0xB3 || | ||
| cmd.packet[5] == 0x95 || cmd.packet[5] == 0x91 || cmd.packet[5] == 0x90) && // queued OpSpec | ||
| ((cmd.packet[6] == 91 && cmd.packet[7] == 0x01 && cmd.packet[8] == 0xAE) || // old format | ||
| (cmd.packet[6] == 0x01 && cmd.packet[7] == 0xAE && cmd.packet[8] == 0x00 && cmd.packet[9] == 91) || // new format SubID 430 Obj 91 | ||
| (cmd.packet[6] == 0x56 && cmd.packet[7] == 0x00 && cmd.packet[8] == 0x06 && cmd.packet[9] == 0x01))) { // Sub 5600 Obj 0601 (mode write) | ||
| (cmd.expect_short_ack || (cmd.expect_obj_id == 0x0000 && cmd.expect_sub_id == 0x0000))) { | ||
| uint8_t err_code = (len >= 7) ? data[6] : 0xFF; |
eman
force-pushed
the
fix-schedule-timeouts
branch
from
July 16, 2026 20:06
9f33728 to
ae70018
Compare
eman
force-pushed
the
fix-schedule-timeouts
branch
from
July 16, 2026 20:18
e7f123e to
3d81e0e
Compare
Comment on lines
+145
to
+151
| ESP_LOGW(TAG, "Failed to poll schedule state (timeout). Assuming schedule is disabled by pump mode."); | ||
| this->schedule_enabled_ = false; | ||
| this->schedule_state_cached_ = true; | ||
| this->overview_cached_ = true; // MUST set this so is_fully_synced() can succeed! | ||
| if (this->state_change_callback_) { | ||
| this->state_change_callback_(false); | ||
| } |
Comment on lines
1013
to
+1016
| if (on_complete) | ||
| on_complete(true); | ||
| }, | ||
| 3000); | ||
| 3000, false, true); |
Comment on lines
+14
to
+37
|
|
||
| def set_mode(mode): | ||
| print(f"Setting mode to {mode}...") | ||
| url = f"{HOST}/api/services/select/select_option" | ||
| res = requests.post(url, headers=HEADERS, json={"entity_id": "select.alpha_hwr_pump_control_mode", "option": mode}, verify=False) | ||
| res.raise_for_status() | ||
|
|
||
| def set_flow(value): | ||
| print(f"Setting flow setpoint to {value}...") | ||
| url = f"{HOST}/api/services/number/set_value" | ||
| res = requests.post(url, headers=HEADERS, json={"entity_id": "number.alpha_hwr_constant_flow_setpoint", "value": value}, verify=False) | ||
| res.raise_for_status() | ||
|
|
||
| def set_switch(entity_id, enabled): | ||
| print(f"Setting {entity_id} to {enabled}...") | ||
| url = f"{HOST}/api/services/switch/turn_{'on' if enabled else 'off'}" | ||
| res = requests.post(url, headers=HEADERS, json={"entity_id": entity_id}, verify=False) | ||
| res.raise_for_status() | ||
|
|
||
| def get_state(entity_id): | ||
| url = f"{HOST}/api/states/{entity_id}" | ||
| res = requests.get(url, headers=HEADERS, verify=False) | ||
| res.raise_for_status() | ||
| return res.json().get("state") |
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.
Resolves the
Command timeout waiting for Obj 56833 Sub 0warning by ensuring schedule commands correctly match responses.schedule_service.cppto pass0, 0as the expected Object ID and Sub ID to enable wildcard matching, instead of incorrectly passing the payload type indicator (0xDE01,0xDA01,0xDC01).transport.cppto correctly respect wildcard matching and theexpect_short_ackflag, replacing the hardcoded Object ID whitelist.