Log every UDP decline, report a client close in the commit wait, fall back when the UDP socket fails

esphome/components/esphome/ota/ota_esphome.cpp

@@ -532,8 +532,11 @@ void ESPHomeOTAComponent::handle_data_() {
       goto error;  // NOLINT(cppcoreguidelines-avoid-goto)
     }
     // 1 is this UDP format; other values are free for later ones, which we decline (a zero reply keeps TCP)
-    if (buf[0] == UDP_REQUEST_V1)
+    if (buf[0] == UDP_REQUEST_V1) {
       this->udp_open_(udp_link);
+      if (udp_link.reply[0] == 0)
+        ESP_LOGW(TAG, "UDP unavailable, using TCP");
+    }
     if (!this->data_write_(udp_link.reply, sizeof(udp_link.reply)))
       goto error;  // NOLINT(cppcoreguidelines-avoid-goto)
     if (udp_link.reply[0] != 0) {

esphome/components/esphome/ota/ota_esphome_udp.cpp

@@ -239,10 +239,8 @@ void ESPHomeOTAComponent::udp_open_(UdpLink &link) {
   link.pcb = udp_new();
 #endif
   // On failure the reply stays all zero and ~UdpLink removes the pcb
-  if (link.pcb == nullptr || udp_bind(link.pcb, IP_ANY_TYPE, this->port_) != ERR_OK) {
-    ESP_LOGW(TAG, "UDP unavailable, using TCP");
+  if (link.pcb == nullptr || udp_bind(link.pcb, IP_ANY_TYPE, this->port_) != ERR_OK)
     return;
-  }
   udp_recv(link.pcb, udp_recv_cb, &link);
 #else
   link.peer = peer;
@@ -254,10 +252,8 @@ void ESPHomeOTAComponent::udp_open_(UdpLink &link) {
       socket::set_sockaddr_any(reinterpret_cast<struct sockaddr *>(&local), sizeof(local), this->port_);
   if (link.sock == nullptr || local_len == 0 ||
       link.sock->bind(reinterpret_cast<struct sockaddr *>(&local), local_len) != 0 ||
-      link.sock->setblocking(false) != 0) {
-    ESP_LOGW(TAG, "UDP unavailable, using TCP");
+      link.sock->setblocking(false) != 0)
     return;
-  }
 #endif
   // Below the error codes (0x80 and up) the client checks the first byte against, and never all zero
   token[0] = (token[0] & 0x7F) | 0x01;
@@ -294,7 +290,11 @@ bool ESPHomeOTAComponent::udp_start_() {
       this->client_->setsockopt(SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
       return true;
     }
-    if (read == 0 || !this->would_block_(errno))
+    if (read == 0) {
+      this->remote_closed_ = true;
+      return false;
+    }
+    if (!this->would_block_(errno))
       return false;
     App.feed_wdt();
   }

esphome/espota2.py

@@ -832,13 +832,18 @@ def _start_udp(
         _LOGGER.debug("Device declined UDP")
         return None
     udp_sock = socket.socket(tcp_sock.family, socket.SOCK_DGRAM)
-    udp_sock.connect(tcp_sock.getpeername())
-    channel = UdpChannel(udp_sock, token)
-    if channel.probe():
-        _LOGGER.info("Using UDP")
-        return channel
-    channel.close()
-    _LOGGER.info("UDP blocked, using TCP")
+    try:
+        udp_sock.connect(tcp_sock.getpeername())
+    except OSError as err:
+        udp_sock.close()
+        _LOGGER.info("UDP unavailable (%s), using TCP", err)
+    else:
+        channel = UdpChannel(udp_sock, token)
+        if channel.probe():
+            _LOGGER.info("Using UDP")
+            return channel
+        channel.close()
+        _LOGGER.info("UDP blocked, using TCP")
     # Raw, outside any Noise frame: forging it only forces the TCP path
     send_check(tcp_sock, UDP_FALLBACK, "UDP fallback")
     return None

tests/unit_tests/test_espota2.py

@@ -1919,3 +1919,15 @@ def test_run_ota_impl_handshake_failure_prefers_udp(
         False,
         True,
     ]
+
+
+def test_start_udp_socket_error_falls_back(mock_socket: Mock) -> None:
+    """A UDP socket that cannot reach the peer stays on TCP."""
+    mock_socket.family = socket.AF_INET
+    mock_socket.getpeername.side_effect = OSError("not connected")
+    mock_socket.recv.side_effect = [b"\x05", b"\x06\x07\x08"]
+    assert espota2._start_udp(mock_socket, mock_socket, True) is None
+    assert mock_socket.sendall.call_args_list == [
+        call(espota2.UDP_REQUEST),
+        call(espota2.UDP_FALLBACK),
+    ]