fix: restores non-EXECUTE exchange retries, fixes execute-confirmation poll clearing, cleans up docs/comments

README.md

@@ -164,11 +164,10 @@ For window-type devices (`io_device_type: "window_opener"` or `"ventilation_poin
 
 `silent: true` makes the motor travel more slowly and quietly, matching the "silent operation"
 toggle in the manufacturer apps. It selects the protocol's slow travel profile on position moves
-and on the favorite ("My") command, both isolated by capturing the same command either side of that
-toggle. STOP is excluded because stopping has no travel speed, and ventilation and tilt because
-nothing has been captured for them yet — they keep their existing payloads rather than being
-guessed at. Nothing on the wire reports a device's current profile, so — like `invert_position` —
-this is a declared preference, not a readback.
+and on the favorite ("My") command. STOP is excluded because stopping has no travel speed, and
+ventilation and tilt because nothing has been captured for them yet — they keep their existing
+payloads rather than being guessed at. Nothing on the wire reports a device's current profile, so —
+like `invert_position` — this is a declared preference, not a readback.
 
 Declaring `silent:` on a cover also generates a `<Cover Name> Silent Operation` switch in Home
 Assistant, so the profile can be changed at runtime; the YAML value is the boot state. Omit the

components/home_io_control/exchange_engine.cpp

@@ -49,12 +49,11 @@ void ExchangeEngine::record_debug(const char *stage, uint8_t tries, bool saw_cha
   this->debug_.saw_challenge = this->debug_.saw_challenge || saw_challenge;
 
   const RadioCaptureInfo &capture = (*this->radio_ptr_)->get_last_capture();
-  // Every wait_for_packet() clears the radio's capture before it starts listening, so a plain
-  // "latest wins" here meant a failure report always described the *final*, timed-out wait — which
-  // by construction saw nothing. `cap_valid=0` on a timeout was therefore tautological rather than
-  // evidence, and it hid the only distinction that matters when a device goes quiet: whether the
-  // radio never detected a frame at all, or received one this layer then threw away. Keep the
-  // first informative capture of the exchange instead of letting a later empty one erase it.
+  // wait_for_packet() clears the radio's capture before it starts listening, so the *last* call
+  // recorded here is always the final, timed-out wait — which by construction saw nothing. Keep the
+  // first informative capture of the exchange instead: it's the only one that can distinguish "the
+  // radio never detected a frame" from "it received one and this layer discarded it", which is the
+  // question a failure report actually needs to answer.
   if (!capture.valid && this->debug_.capture_valid)
     return;
   this->debug_.capture_valid = capture.valid;
@@ -195,17 +194,17 @@ const char *inbound_stage_name(exchange::InboundAuthState state) {
 /// Check if frame is a 0x3D challenge response.
 bool frame_is_challenge_response(const IoFrame &frame) { return frame.cmd == CMD_CHALLENGE_RESP; }
 
-/// Log a frame that arrived but could not be parsed. These were recorded into the debug snapshot
-/// and never printed, which made "the radio heard nothing" and "we heard something and rejected
-/// it" look identical in a failure report — the two need opposite fixes. Redacted through the same
-/// helper as every other frame log, so an unparsable frame can't leak key material by being
-/// unrecognisable (see ADR 0011).
+/// Log a frame that arrived but could not be parsed. Printing it distinguishes "the radio heard
+/// nothing" from "we heard something and rejected it" in a failure report — the two need opposite
+/// fixes: a device that never transmitted needs a longer wait or a link check, while one that
+/// transmits noise this layer can't decode needs the RX bandwidth or framing looked at. Redacted
+/// through the same helper as every other frame log, so an unparsable frame can't leak key material
+/// by being unrecognisable (see ADR 0011).
 void log_unparsable_frame(const char *stage, int tries, const RadioRxPacket &packet) {
-  // The *fact* stays unconditional: it means the receiver is demodulating traffic it cannot
-  // decode, which is a real fault worth surfacing to someone who never enables a debug flag — a
-  // misconfigured RX bandwidth showed up as several of these a minute. The bytes only help someone
-  // already debugging the PHY, and dumping them at that rate is what makes a log unreadable, so
-  // they sit behind the frame-log flag with the rest of that detail.
+  // The *fact* that a frame failed to parse stays unconditional — it is a real fault worth
+  // surfacing to someone who never enables a debug flag. The raw bytes only help someone already
+  // debugging the PHY, and a noisy channel can produce several of these a minute, so they sit
+  // behind the frame-log flag with the rest of that detail.
   ESP_LOGW(TAG, "%s try=%d: %u bytes did not parse as a frame on %" PRIu32 " Hz", stage, tries, packet.len,
            packet.freq_hz);
 #ifdef IOHOME_FRAME_LOG
@@ -233,6 +232,7 @@ ExchangeOutcome ExchangeEngine::send_and_receive(const IoFrame &request, IoFrame
   this->reset_debug(request.cmd);
   const uint16_t request_preamble = is_start(request) ? LONG_PREAMBLE : (*this->radio_ptr_)->response_preamble();
   const uint32_t exchange_begin_ms = millis();
+  bool accepted_without_reply = false;
 
   for (uint8_t tries = 0; tries < EXCHANGE_RETRY_COUNT; tries++) {
     exchange::OutboundExchangeContext context;
@@ -280,13 +280,15 @@ ExchangeOutcome ExchangeEngine::send_and_receive(const IoFrame &request, IoFrame
     auto final_disp = this->wait_for_final_response_(request, context);
     if (final_disp != decisions::ExchangeFinalResponseDisposition::ACCEPT) {
       // The device challenged us and accepted our answer, so it demonstrably received the request.
-      // Not every device closes the exchange with a synchronous reply (see ExchangeOutcome), and
-      // retrying here is actively harmful: the command is already executing, so the two remaining
-      // tries re-send a movement command to a device that is mid-move and, having already acted,
-      // ignores them — which is what turned a working command into a reported failure.
+      // Not every device closes the exchange with a synchronous reply (see ExchangeOutcome). A
+      // retry is safe only for a request with no side effect to repeat — CMD_EXECUTE is already
+      // acting on the first copy, so it stops here; everything else spends its full retry budget.
       context.state = exchange::OutboundExchangeState::SUCCESS;
       this->record_debug("success_auth_unconfirmed", context.try_index, true);
-      return ExchangeOutcome::SUCCESS_UNCONFIRMED;
+      accepted_without_reply = true;
+      if (!decisions::retry_after_unconfirmed_accept_is_safe(request.cmd))
+        return ExchangeOutcome::SUCCESS_UNCONFIRMED;
+      continue;
     }
 
     context.state = exchange::OutboundExchangeState::SUCCESS;
@@ -295,7 +297,10 @@ ExchangeOutcome ExchangeEngine::send_and_receive(const IoFrame &request, IoFrame
     return ExchangeOutcome::SUCCESS_WITH_RESPONSE;
   }
 
-  return ExchangeOutcome::FAILED;
+  // An exchange that authenticated on some try but never got a reply is not the same as one the
+  // device never answered at all: callers that only need "the request landed" can act on it, and
+  // callers that need the payload still cannot.
+  return accepted_without_reply ? ExchangeOutcome::SUCCESS_UNCONFIRMED : ExchangeOutcome::FAILED;
 }
 
 // ============================================================================

components/home_io_control/exchange_engine.h

@@ -44,23 +44,25 @@ namespace home_io_control {
 /// Deliberately not a bool: "the device accepted the command" and "the device told us what
 /// happened" are different facts, and some devices only ever deliver the first.
 ///
-/// A Somfy RS100 challenges a command, authenticates it, executes it — and then transmits nothing
-/// for 3–12 seconds, reporting via an asynchronous status update instead (measured 2026-08-15
-/// across eight authenticated commands: next device frame at 3.4 s, 3.6 s, 5.3 s, 12.0 s, or
-/// never, against a 500 ms window). A Somfy awning on the same protocol closes the exchange
-/// properly with a synchronous 0x04 (tests/corpus/captures/somfy_awning/exchange_open_sx1276.yaml),
-/// so the four-frame exchange is real — just not universal.
+/// Some devices challenge a command, authenticate it, execute it, and then transmit nothing for
+/// several seconds — up to a dozen — reporting via an asynchronous status update later instead of
+/// closing the exchange with a synchronous reply, all well outside the exchange's own response
+/// window. Other devices on the same protocol close the exchange properly with a synchronous 0x04
+/// (see tests/corpus/captures/somfy_awning/exchange_open_sx1276.yaml), so the four-frame exchange
+/// is real — just not universal, and a caller cannot assume either shape from the command alone.
 ///
-/// Treating the RS100's silence as failure made every command it *did* execute report as failed,
-/// left the hub's position permanently stale, and re-sent two more copies of a movement command to
-/// a shutter that was already moving.
+/// SUCCESS_UNCONFIRMED exists so that silence after a real authentication is not treated the same
+/// as a request the device may never have heard at all: the two need different retry rules (see
+/// decisions::retry_after_unconfirmed_accept_is_safe()) and different reporting to the caller.
 enum class ExchangeOutcome : uint8_t {
   FAILED,                 ///< No usable reply; the device may never have heard the request.
   SUCCESS_WITH_RESPONSE,  ///< Device replied; the caller's `response` frame is populated.
   SUCCESS_UNCONFIRMED,    ///< Device authenticated the request — so it received and accepted it —
                           ///< but sent no final response. `response` is NOT populated. Callers that
                           ///< need payload (key exchange) must treat this as failure; callers that
-                          ///< only need "the command landed" should treat it as success.
+                          ///< only need "the command landed" should treat it as success. For every
+                          ///< command but CMD_EXECUTE, this outcome is only returned after the full
+                          ///< retry budget is spent — see retry_after_unconfirmed_accept_is_safe().
 };
 
 class ExchangeEngine {

components/home_io_control/hub_core.cpp

@@ -329,19 +329,20 @@ void IOHomeControlComponent::loop() {
   // A blocking exchange makes the radio deaf for 1–3 s. When a linked remote's press schedules a
   // status poll, dispatching it while that same remote is still transmitting would blind the hub
   // to the rest of the press — so background polls yield for a moment. Control operations never do.
-  if (!this->busy_ && !this->defer_background_poll_())
+  if (!this->busy_ && !this->defer_background_poll_()) {
     this->process_pending_operation_();
+  }
 
-    // Frequency hopping — protocol specifies 2.7ms per channel, but ESPHome calls
-    // loop() every ~16-30ms. This is acceptable for a controller: we initiate all
-    // exchanges with a long preamble (1024 bytes ≈ 330ms airtime) so the device has
-    // time to detect us regardless of channel alignment. Precise hopping would only
-    // matter for a passive receiver scanning for unsolicited frames.
-    // Diagnostics build flag: park the receiver on one channel instead of hopping. A hopping monitor
-    // is on any given channel roughly a third of the time, so "the capture never shows frame X" is
-    // weak evidence — locking to the channel under study makes an absence mean something. Define it
-    // to the channel in Hz, e.g. -DIOHOME_LOCK_CHANNEL_HZ=868950000 for CH2, the command channel.
-    // Only useful for a passive monitor: a hub that cannot hop will miss replies on other channels.
+  // Frequency hopping — protocol specifies 2.7ms per channel, but ESPHome calls
+  // loop() every ~16-30ms. This is acceptable for a controller: we initiate all
+  // exchanges with a long preamble (1024 bytes ≈ 330ms airtime) so the device has
+  // time to detect us regardless of channel alignment. Precise hopping would only
+  // matter for a passive receiver scanning for unsolicited frames.
+  // Diagnostics build flag: park the receiver on one channel instead of hopping. A hopping monitor
+  // is on any given channel roughly a third of the time, so "the capture never shows frame X" is
+  // weak evidence — locking to the channel under study makes an absence mean something. Define it
+  // to the channel in Hz, e.g. -DIOHOME_LOCK_CHANNEL_HZ=868950000 for CH2, the command channel.
+  // Only useful for a passive monitor: a hub that cannot hop will miss replies on other channels.
 #ifdef IOHOME_LOCK_CHANNEL_HZ
   if (!this->busy_ && this->radio_ != nullptr && this->radio_->get_current_freq() != IOHOME_LOCK_CHANNEL_HZ)
     this->radio_->change_frequency(IOHOME_LOCK_CHANNEL_HZ);

components/home_io_control/hub_core.h

@@ -538,6 +538,16 @@ class IOHomeControlComponent : public Component,
   /// @param device_id ID of the device to poll.
   /// @param initial_delay_ms Delay before the first follow-up poll.
   void begin_status_poll_tracking_(const std::string &device_id, uint32_t initial_delay_ms);
+  /// Arm the confirming poll that follows a command, because a CMD_EXECUTE reply is never trusted
+  /// for position (see update_device_status_()'s trust_position parameter) and therefore leaves the
+  /// hub with no idea where the device actually is. Re-arms the bounded tracking window rather than
+  /// only setting a due time: the same untrusted reply clears that window whenever it claims the
+  /// device is stopped, and pop_due_device() discards a due poll that has no active window. An
+  /// already-scheduled earlier poll wins.
+  /// @param device_id Device the command was sent to.
+  /// @param for_stop True for STOP (and position POS_STOP), which settles under
+  ///        STOP_SETTLE_POLL_CAP_MS instead of the normal settle cadence.
+  void arm_execute_confirmation_poll_(const std::string &device_id, bool for_stop);
   /// Schedule status polls for a fixed list of devices (shared by the id-linked and
   /// class-linked 1W paths, and by schedule_linked_remote_polls_()).
   /// @param device_ids Devices to poll.
@@ -583,22 +593,26 @@ class IOHomeControlComponent : public Component,
   /// @param linked True if the sender is linked to at least one registered device.
   /// @param src_id Sender's node ID as a string (already computed by the caller).
   void maybe_fire_sender_event_(const OneWayFrameInfo &info, bool linked, const std::string &src_id);
-  /// Shared request/response helper for high-level operations.
-  /// @param device_id Target device ID.
-  /// @param request Outbound request frame.
-  /// @param warn_on_no_response If true, logs a warning when no response is received.
-  /// @param retry_after_fail_ms If non-zero, schedules next status poll after this delay on failure.
-  /// @return true if the device acknowledged *or* accepted the request without replying — see
-  ///         @ref ExchangeOutcome, and the CMD_EXECUTE carve-out in the definition.
-  ///
   /// Handle an explicit CMD_ERROR_RESP refusal from the device: record the result code, stamp link
   /// health, and schedule the poll backoff. Split out of execute_request_and_update_() to keep that
   /// function's outcome dispatch readable — a refusal is a distinct concern from "what did the
   /// exchange achieve".
+  /// @param device_id Target device ID.
+  /// @param request Outbound request frame that drew the refusal.
+  /// @param response The CMD_ERROR_RESP frame.
+  /// @param retry_after_fail_ms If non-zero, schedules next status poll after this delay.
   /// @return Always false; a refusal is never a success.
   bool handle_error_response_(const std::string &device_id, const IoFrame &request, const IoFrame &response,
                               uint32_t retry_after_fail_ms);
 
+  /// Shared request/response helper for high-level operations.
+  /// @param device_id Target device ID.
+  /// @param request Outbound request frame.
+  /// @param warn_on_no_response If true, logs a warning when no response is received.
+  /// @param retry_after_fail_ms If non-zero, schedules next status poll after this delay on failure.
+  /// @return true when the device replied, or when a CMD_EXECUTE was accepted without a reply —
+  ///         every other command's unconfirmed acceptance is still a failure here (see
+  ///         @ref ExchangeOutcome and decisions::retry_after_unconfirmed_accept_is_safe()).
   bool execute_request_and_update_(const std::string &device_id, const IoFrame &request, bool warn_on_no_response,
                                    uint32_t retry_after_fail_ms = 0);
   /// Execute a named device command (STOP, FAVORITE, VENT, FORCE_OPEN) via the authenticated exchange.

components/home_io_control/hub_decisions.h

@@ -105,6 +105,16 @@ inline ExchangeFinalResponseDisposition classify_exchange_final_response(const I
                                                               : ExchangeFinalResponseDisposition::IGNORE_UNRELATED;
 }
 
+/// Whether an authenticated-but-unanswered request may be sent again.
+///
+/// CMD_EXECUTE is the only request the hub sends that moves something, so a retry there is a
+/// second side effect on a device already acting on the first copy. Every other request (status
+/// polls, name reads, management actions, config writes) is idempotent and keeps its full retry
+/// budget when the device authenticates but never closes the exchange.
+/// @param cmd Command byte of the outbound request.
+/// @return true when the remaining retries should still be spent.
+[[nodiscard]] inline bool retry_after_unconfirmed_accept_is_safe(uint8_t cmd) { return cmd != CMD_EXECUTE; }
+
 // == Pairing discovery & key-challenge classification ==
 
 /// Decide if a frame is a valid discovery response (0x29) during pairing.