Trim comments

esphome/components/async_tcp/async_tcp_socket.cpp

@@ -42,8 +42,7 @@ bool AsyncClient::connect(const char *host, uint16_t port) {
   }
 
   if (socket_->setblocking(false) != 0) {
-    // Capture before the log and reset() below can clobber errno; a blocking
-    // connect()/read() would otherwise stall the whole loop
+    // Capture before the log and close() clobber errno
     const int saved_errno = errno;
     ESP_LOGE(TAG, "Failed to set nonblocking: errno %d", saved_errno);
     close();

esphome/components/socket/lwip_raw_tcp_impl.cpp

@@ -87,9 +87,8 @@ static int lwip_err_to_errno(err_t err) {
 // Must be called before destroying the object that tcp_arg points to —
 // tcp_abort() triggers the err callback synchronously, which would
 // otherwise call back into a partially-destroyed object.
-// tcp_sent/tcp_poll are not cleared because this implementation
-// never registers them, and the tcp_connect callback only fires on
-// SYN_SENT -> ESTABLISHED, which cannot follow an abort or close.
+// tcp_sent/tcp_poll are never registered and the connect callback cannot
+// fire after abort or close, so neither is cleared.
 static void pcb_detach_abort(struct tcp_pcb *pcb) {
   tcp_arg(pcb, nullptr);
   tcp_recv(pcb, nullptr);
@@ -102,8 +101,7 @@ static void pcb_detach_abort(struct tcp_pcb *pcb) {
 // After tcp_close(), the PCB remains alive during the TCP close handshake
 // (FIN_WAIT, TIME_WAIT states). Without clearing callbacks first, LWIP
 // would call recv/err on a destroyed socket object, corrupting the heap.
-// tcp_sent/tcp_poll and the tcp_connect callback are not cleared for the
-// same reasons as in pcb_detach_abort().
+// Callbacks are left as in pcb_detach_abort().
 // Returns ERR_OK on success; on failure the PCB is aborted instead.
 static err_t pcb_detach_close(struct tcp_pcb *pcb) {
   tcp_arg(pcb, nullptr);
@@ -436,9 +434,8 @@ void LWIPRawImpl::s_err_fn(void *arg, err_t err) {
   auto *arg_this = reinterpret_cast<LWIPRawImpl *>(arg);
   ESP_LOGVV(TAG, "socket %p: err(err=%d)", arg_this, err);
   if (arg_this->connect_err_ == EINPROGRESS) {
-    // A connect that never established: RST is a refusal, anything else is
-    // the SYN retransmits giving up. Written before pcb_ so poll_connect()
-    // never sees a dead pcb without its reason.
+    // Refused (RST) or SYN retries exhausted; written before pcb_ so
+    // poll_connect() never sees a dead pcb without its reason
     arg_this->connect_err_ = err == ERR_RST ? ECONNREFUSED : ETIMEDOUT;
   }
   arg_this->pcb_ = nullptr;
@@ -446,8 +443,7 @@ void LWIPRawImpl::s_err_fn(void *arg, err_t err) {
 }
 
 err_t LWIPRawImpl::s_connected_fn(void *arg, struct tcp_pcb *pcb, err_t err) {
-  // LWIP CALLBACK — same constraints as s_err_fn. err is always ERR_OK; a
-  // failed connect arrives through s_err_fn instead.
+  // LWIP CALLBACK, same constraints as s_err_fn; err is always ERR_OK
   auto *arg_this = reinterpret_cast<LWIPRawImpl *>(arg);
   arg_this->connect_err_ = EISCONN;
   esphome::wake_loop_any_context();
@@ -470,8 +466,7 @@ int LWIPRawImpl::connect(const struct sockaddr *addr, socklen_t addrlen) {
     return -1;
   }
 #if LWIP_IPV6
-  // tcp_connect needs a concrete address type. A remembered IPv4 peer on an
-  // IPv6 build arrives as a v4-mapped address and must be dialed as IPv4.
+  // tcp_connect needs a concrete type; a remembered IPv4 peer arrives v4-mapped
   if (IP_IS_ANY_TYPE_VAL(ip)) {
     if (ip6_addr_isipv4mappedipv6(ip_2_ip6(&ip))) {
       unmap_ipv4_mapped_ipv6(ip_2_ip4(&ip), ip_2_ip6(&ip));
@@ -628,9 +623,8 @@ ssize_t LWIPRawImpl::read_locked_(void *buf, size_t len) {
 }
 
 ssize_t LWIPRawImpl::read(void *buf, size_t len) {
-  // Would block: let queued WiFi RX reach lwip first so this read may
-  // succeed; otherwise inbound segments can sit unprocessed for seconds
-  // while the main loop polls
+  // Let queued WiFi RX reach lwip first; otherwise inbound segments can
+  // sit unprocessed for seconds while the main loop polls
   if (this->waiting_for_data_()) {
     yield_to_sys();
   }

esphome/components/socket/lwip_raw_tcp_impl.h

@@ -50,8 +50,7 @@ class LWIPRawCommon {
 
  protected:
   int ip2sockaddr_(ip_addr_t *ip, uint16_t port, struct sockaddr *name, socklen_t *addrlen);
-  /// Convert a sockaddr of this socket's family to an lwip address and port.
-  /// Returns false with errno set on a family or length mismatch.
+  /// sockaddr of this socket's family to lwip address and port; false with errno on mismatch
   bool sockaddr2ip_(const struct sockaddr *name, socklen_t addrlen, ip_addr_t *ip, uint16_t *port) const;
 
   // Member ordering optimized to minimize padding on 32-bit systems
@@ -61,15 +60,13 @@ class LWIPRawCommon {
   bool nodelay_ = false;
   sa_family_t family_ = 0;
   uint8_t recv_timeout_cs_ = 0;  // SO_RCVTIMEO in centiseconds (0 = no timeout, max 2.55s)
-  // State of connect() on this socket: 0 before one was started, EINPROGRESS
-  // while the SYN is out, EISCONN once established, otherwise the errno the
-  // lwip callbacks recorded for its failure. Fits the padding byte here.
+  // 0 before connect(), EINPROGRESS while pending, EISCONN once established,
+  // else the failure errno the callbacks recorded; fills the padding byte
   uint8_t connect_err_ = 0;
   static_assert(EINPROGRESS < 256 && EISCONN < 256 && ECONNREFUSED < 256 && ECONNRESET < 256 && ETIMEDOUT < 256,
                 "connect_err_ stores errno values in a byte");
 };
-// The connect state must stay inside the padding: no socket, listening or
-// accepted, pays RAM for it
+// The connect state must stay in the padding so no socket pays RAM for it
 static_assert(sizeof(LWIPRawCommon) == sizeof(struct tcp_pcb *) + 4, "LWIPRawCommon grew past one word of flags");
 
 /// Connected socket implementation for LWIP raw TCP.
@@ -95,14 +92,11 @@ class LWIPRawImpl : public LWIPRawCommon {
     errno = EOPNOTSUPP;
     return -1;
   }
-  /// Start a non-blocking connect. Always returns -1 with errno EINPROGRESS
-  /// when the SYN was queued; completion is reported by poll_connect().
-  /// addr must be of the family the socket was created with; an IPv4 peer
-  /// on an AF_INET6 socket arrives as a v4-mapped sockaddr_in6.
+  /// Non-blocking: returns -1/EINPROGRESS once the SYN is queued, see poll_connect().
+  /// addr must match the socket family; an IPv4 peer on AF_INET6 arrives v4-mapped.
   int connect(const struct sockaddr *addr, socklen_t addrlen);
-  // Intentionally unlocked like ready(): reads one pointer and one byte that
-  // the callbacks write in the order the checks depend on (error byte first,
-  // then pcb_), so a torn read only costs one extra poll.
+  // Unlocked like ready(): the callbacks write the error byte before pcb_,
+  // so a torn read only costs one extra poll
   ConnectPollResult poll_connect(int &err_out) const;
   ssize_t read(void *buf, size_t len);
   ssize_t readv(const struct iovec *iov, int iovcnt);

esphome/components/socket/socket.h

@@ -145,11 +145,8 @@ inline socklen_t set_sockaddr(struct sockaddr *addr, socklen_t addrlen, const st
 /// Set a sockaddr to the any address and specified port for the IP version used by socket_ip().
 socklen_t set_sockaddr_any(struct sockaddr *addr, socklen_t addrlen, uint16_t port);
 
-/// Check a non-blocking connect() for completion without blocking. Only
-/// meaningful after connect() returned -1 with errno EINPROGRESS. On
-/// CONNECT_POLL_RESULT_ERROR, err_out holds the socket's SO_ERROR (or errno when the
-/// poll itself failed) on fd based implementations, and the failure recorded
-/// by the lwip callbacks on the raw lwip implementation.
+/// Poll a connect() that returned EINPROGRESS. On error, err_out is SO_ERROR (or
+/// errno) on fd implementations and the failure the callbacks recorded on raw lwip.
 #ifdef USE_SOCKET_IMPL_LWIP_TCP
 inline ConnectPollResult poll_connect(Socket &sock, int &err_out) { return sock.poll_connect(err_out); }
 #else

tests/integration/fixtures/socket_set_sockaddr.yaml

@@ -3,8 +3,7 @@ esphome:
   on_boot:
     then:
       - lambda: |-
-          // Exercise the contract callers rely on: 0 for text that is not an
-          // address, the sockaddr length otherwise, broadcast included
+          // 0 for text that is not an address, the length otherwise, broadcast included
           struct sockaddr_storage addr;
           auto *sa = reinterpret_cast<struct sockaddr *>(&addr);
           ESP_LOGI("test", "SET_SOCKADDR invalid=%u valid=%u broadcast=%u",

tests/integration/test_socket_set_sockaddr.py

@@ -1,9 +1,4 @@
-"""Integration test for the socket::set_sockaddr failure contract.
-
-Callers skip an address when set_sockaddr returns 0, so text that is not an
-address must return 0 while a real address, including the broadcast address
-that inet_addr() would have reported as a failure, returns its length.
-"""
+"""Integration test for the socket::set_sockaddr failure contract."""
 
 import asyncio
 import re