diff --git a/esphome/components/zigbee_proxy/zigbee_proxy.cpp b/esphome/components/zigbee_proxy/zigbee_proxy.cpp index 2dd87412f4..eb38d337db 100644 --- a/esphome/components/zigbee_proxy/zigbee_proxy.cpp +++ b/esphome/components/zigbee_proxy/zigbee_proxy.cpp @@ -77,7 +77,15 @@ void ZigbeeProxy::loop() { this->parent_->set_baud_rate(this->configured_baud_rate_); this->parent_->load_settings(false); } - this->reset_ash_protocol_(); + // A subscriber drives its own session: it opens with an RST and negotiates its own + // EZSP version. Harvesting here would put a second RST on the wire alongside the + // client's and renegotiate the NCP underneath it, so only harvest when there is + // something left to learn. + if (this->api_connection_ != nullptr && this->network_info_.valid) { + this->reset_ncp_link_(); + } else { + this->reset_ash_protocol_(); + } } // Process incoming UART data @@ -117,7 +125,11 @@ void ZigbeeProxy::loop() { millis() - this->last_recovery_attempt_ > RECOVERY_RETRY_INTERVAL_MS) { ESP_LOGI(TAG, "Attempting NCP recovery"); this->last_recovery_attempt_ = millis(); - this->reset_ash_protocol_(); + // Reset the link only. Re-running the harvest would renegotiate the NCP's EZSP + // version underneath the subscriber, which then keeps addressing a version the NCP + // no longer speaks -- and because the harvest consumes its own RSTACKs, it would + // never find out. Relaying the RSTACK instead lets it resynchronize deliberately. + this->reset_ncp_link_(); } } @@ -338,10 +350,31 @@ void ZigbeeProxy::reset_ash_protocol_() { this->boot_sequence_active_ = true; this->ezsp_sequence_ = 0; + // Abandon RSTACKs owed from a previous attempt: they can no longer arrive in a state + // where suppressing them is correct, and a stale count would swallow a real reset. + this->own_rst_outstanding_ = 0; + this->send_rst_frame_(); } -void ZigbeeProxy::send_rst_frame_() { +void ZigbeeProxy::reset_ncp_link_() { + this->ash_state_ = AshState::CONNECTING; + this->setup_time_ = millis(); // Reset timeout reference for the RSTACK wait + this->tx_sequence_ = 0; + this->rx_sequence_ = 0; + this->tx_buffer_pending_ = false; + this->tx_retry_count_ = 0; + this->ncp_tx_queue_count_ = 0; + this->parsing_state_ = ParsingState::WAIT_FLAG_START; + this->client_reset_session_(); + this->send_rst_frame_(false); +} + +void ZigbeeProxy::send_rst_frame_(bool own_reset) { + if (own_reset) { + this->own_rst_outstanding_++; + } + // Build a combined buffer: 32 CAN bytes followed immediately by the RST frame, // sent as a single write_array call. This ensures correct byte ordering and // minimizes the number of USB bulk transfers (all bytes fit in one USB FS packet). @@ -367,6 +400,14 @@ void ZigbeeProxy::handle_rstack_frame_(const uint8_t *data, size_t length) { this->rx_sequence_ = 0; this->clear_tx_buffer_(); + // Account for this RSTACK before deciding whether the client should see it. Only a + // reset we did not cause is news to the client; relaying one of ours makes bellows + // call enter_failed_state() and cancel every command it has in flight. + bool solicited_by_us = this->own_rst_outstanding_ > 0; + if (solicited_by_us) { + this->own_rst_outstanding_--; + } + if (this->boot_state_ == BootState::WAIT_RSTACK) { // Initial RSTACK - start boot sequence ESP_LOGV(TAG, "Received RSTACK, starting EZSP initialization"); @@ -395,6 +436,12 @@ void ZigbeeProxy::handle_rstack_frame_(const uint8_t *data, size_t length) { // Now check for WiFi/Zigbee channel conflicts this->check_wifi_zigbee_conflict_(); } else if (this->ash_state_ == AshState::CONNECTING) { + if (solicited_by_us) { + // One of our own resets answered while a client reset is still outstanding. Stay in + // CONNECTING and keep waiting for the RSTACK the client is actually owed. + ESP_LOGV(TAG, "Consumed own RSTACK while awaiting the client's"); + return; + } // RSTACK during connecting (triggered by client RST forwarding) ESP_LOGV(TAG, "Received RSTACK, NCP ready"); this->ash_state_ = AshState::CONNECTED; @@ -402,9 +449,15 @@ void ZigbeeProxy::handle_rstack_frame_(const uint8_t *data, size_t length) { if (this->api_connection_ != nullptr) { this->forward_ncp_rstack_to_client_(this->rx_buffer_.data() + 1, this->rx_buffer_index_ - 3); } + } else if (solicited_by_us) { + // Surplus RSTACK from one of our own resets, most often an RST retry racing a reply + // that was merely slow. The client never asked for it, so swallow it. + ESP_LOGV(TAG, "Consumed surplus RSTACK from own reset"); + this->ash_state_ = AshState::CONNECTED; } else if (this->api_connection_ != nullptr) { - // Client is subscribed, forward RSTACK - ESP_LOGV(TAG, "Forwarding RSTACK to client"); + // A reset we did not cause: the NCP rebooted on its own, which invalidates the + // client's session, so it has to hear about it. + ESP_LOGW(TAG, "NCP reset unexpectedly, notifying client"); this->ash_state_ = AshState::CONNECTED; this->forward_ncp_rstack_to_client_(this->rx_buffer_.data() + 1, this->rx_buffer_index_ - 3); } else { @@ -1342,16 +1395,7 @@ void ZigbeeProxy::client_parse_control_byte_(uint8_t control) { // Client wants to reset - forward RST to NCP // Don't use reset_ash_protocol_() as that enters boot sequence ESP_LOGV(TAG, "Client RST → forwarding to NCP"); - this->ash_state_ = AshState::CONNECTING; - this->setup_time_ = millis(); // Reset timeout reference for RSTACK wait - this->tx_sequence_ = 0; - this->rx_sequence_ = 0; - this->tx_buffer_pending_ = false; - this->tx_retry_count_ = 0; - this->ncp_tx_queue_count_ = 0; - this->parsing_state_ = ParsingState::WAIT_FLAG_START; - this->client_reset_session_(); - this->send_rst_frame_(); + this->reset_ncp_link_(); break; case AshFrameType::RSTACK: diff --git a/esphome/components/zigbee_proxy/zigbee_proxy.h b/esphome/components/zigbee_proxy/zigbee_proxy.h index 66ff4d8627..d888f3a6fe 100644 --- a/esphome/components/zigbee_proxy/zigbee_proxy.h +++ b/esphome/components/zigbee_proxy/zigbee_proxy.h @@ -108,7 +108,14 @@ class ZigbeeProxy : public uart::UARTDevice, public Component { protected: // ASH Protocol State Machine void reset_ash_protocol_(); - void send_rst_frame_(); + // Resets the NCP link without running the metadata harvest, leaving the client to + // negotiate its own session. The resulting RSTACK is relayed rather than consumed. + void reset_ncp_link_(); + // `own_reset` marks a reset we originate ourselves (harvest, retry, final RST) so its + // RSTACK is consumed here instead of being relayed to a client that never asked for + // one -- bellows treats an unsolicited RSTACK as fatal and cancels every pending + // command. Pass false when relaying a client's own RST. + void send_rst_frame_(bool own_reset = true); void handle_rstack_frame_(const uint8_t *data, size_t length); void handle_error_frame_(const uint8_t *data, size_t length); // Applies a frame's ackNum to the pending TX frame. Returns true if it @@ -307,6 +314,11 @@ class ZigbeeProxy : public uart::UARTDevice, public Component { uint32_t configured_baud_rate_{0}; // Line rate to restore after another device bool boot_sequence_active_{false}; // True during boot-time init + + // RSTACKs still owed to us for resets we sent ourselves. A retry can put two RSTs on + // the wire when the first RSTACK was only slow rather than lost, so the NCP answers + // with more RSTACKs than we asked for; the surplus must not reach a client. + uint8_t own_rst_outstanding_{0}; }; extern ZigbeeProxy *global_zigbee_proxy; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)