[socket] Refactor socket implementations for memory efficiency and code quality (#12550)

esphome/components/socket/bsd_sockets_impl.cpp

@@ -12,8 +12,7 @@
 #include <lwip/sockets.h>
 #endif
 
-namespace esphome {
+namespace esphome::socket {
-namespace socket {
 
 std::string format_sockaddr(const struct sockaddr_storage &storage) {
   if (storage.ss_family == AF_INET) {
@@ -44,11 +43,11 @@ class BSDSocketImpl : public Socket {
   BSDSocketImpl(int fd, bool monitor_loop = false) : fd_(fd) {
 #ifdef USE_SOCKET_SELECT_SUPPORT
     // Register new socket with the application for select() if monitoring requested
-    if (monitor_loop && fd_ >= 0) {
+    if (monitor_loop && this->fd_ >= 0) {
       // Only set loop_monitored_ to true if registration succeeds
-      loop_monitored_ = App.register_socket_fd(fd_);
+      this->loop_monitored_ = App.register_socket_fd(this->fd_);
     } else {
-      loop_monitored_ = false;
+      this->loop_monitored_ = false;
     }
 #else
     // Without select support, ignore monitor_loop parameter
@@ -56,70 +55,69 @@ class BSDSocketImpl : public Socket {
 #endif
   }
   ~BSDSocketImpl() override {
-    if (!closed_) {
+    if (!this->closed_) {
-      close();  // NOLINT(clang-analyzer-optin.cplusplus.VirtualCall)
+      this->close();  // NOLINT(clang-analyzer-optin.cplusplus.VirtualCall)
     }
   }
-  int connect(const struct sockaddr *addr, socklen_t addrlen) override { return ::connect(fd_, addr, addrlen); }
+  int connect(const struct sockaddr *addr, socklen_t addrlen) override { return ::connect(this->fd_, addr, addrlen); }
   std::unique_ptr<Socket> accept(struct sockaddr *addr, socklen_t *addrlen) override {
-    return accept_impl_(addr, addrlen, false);
+    int fd = ::accept(this->fd_, addr, addrlen);
-  }
-  std::unique_ptr<Socket> accept_loop_monitored(struct sockaddr *addr, socklen_t *addrlen) override {
-    return accept_impl_(addr, addrlen, true);
-  }
-
- private:
-  std::unique_ptr<Socket> accept_impl_(struct sockaddr *addr, socklen_t *addrlen, bool loop_monitored) {
-    int fd = ::accept(fd_, addr, addrlen);
     if (fd == -1)
       return {};
-    return make_unique<BSDSocketImpl>(fd, loop_monitored);
+    return make_unique<BSDSocketImpl>(fd, false);
+  }
+  std::unique_ptr<Socket> accept_loop_monitored(struct sockaddr *addr, socklen_t *addrlen) override {
+    int fd = ::accept(this->fd_, addr, addrlen);
+    if (fd == -1)
+      return {};
+    return make_unique<BSDSocketImpl>(fd, true);
   }
 
- public:
+  int bind(const struct sockaddr *addr, socklen_t addrlen) override { return ::bind(this->fd_, addr, addrlen); }
-  int bind(const struct sockaddr *addr, socklen_t addrlen) override { return ::bind(fd_, addr, addrlen); }
   int close() override {
-    if (!closed_) {
+    if (!this->closed_) {
 #ifdef USE_SOCKET_SELECT_SUPPORT
       // Unregister from select() before closing if monitored
-      if (loop_monitored_) {
+      if (this->loop_monitored_) {
-        App.unregister_socket_fd(fd_);
+        App.unregister_socket_fd(this->fd_);
       }
 #endif
-      int ret = ::close(fd_);
+      int ret = ::close(this->fd_);
-      closed_ = true;
+      this->closed_ = true;
       return ret;
     }
     return 0;
   }
-  int shutdown(int how) override { return ::shutdown(fd_, how); }
+  int shutdown(int how) override { return ::shutdown(this->fd_, how); }
 
-  int getpeername(struct sockaddr *addr, socklen_t *addrlen) override { return ::getpeername(fd_, addr, addrlen); }
+  int getpeername(struct sockaddr *addr, socklen_t *addrlen) override {
+    return ::getpeername(this->fd_, addr, addrlen);
+  }
   std::string getpeername() override {
     struct sockaddr_storage storage;
     socklen_t len = sizeof(storage);
-    int err = this->getpeername((struct sockaddr *) &storage, &len);
+    if (::getpeername(this->fd_, (struct sockaddr *) &storage, &len) != 0)
-    if (err != 0)
       return {};
     return format_sockaddr(storage);
   }
-  int getsockname(struct sockaddr *addr, socklen_t *addrlen) override { return ::getsockname(fd_, addr, addrlen); }
+  int getsockname(struct sockaddr *addr, socklen_t *addrlen) override {
+    return ::getsockname(this->fd_, addr, addrlen);
+  }
   std::string getsockname() override {
     struct sockaddr_storage storage;
     socklen_t len = sizeof(storage);
-    int err = this->getsockname((struct sockaddr *) &storage, &len);
+    if (::getsockname(this->fd_, (struct sockaddr *) &storage, &len) != 0)
-    if (err != 0)
       return {};
     return format_sockaddr(storage);
   }
   int getsockopt(int level, int optname, void *optval, socklen_t *optlen) override {
-    return ::getsockopt(fd_, level, optname, optval, optlen);
+    return ::getsockopt(this->fd_, level, optname, optval, optlen);
   }
   int setsockopt(int level, int optname, const void *optval, socklen_t optlen) override {
-    return ::setsockopt(fd_, level, optname, optval, optlen);
+    return ::setsockopt(this->fd_, level, optname, optval, optlen);
   }
-  int listen(int backlog) override { return ::listen(fd_, backlog); }
+  int listen(int backlog) override { return ::listen(this->fd_, backlog); }
-  ssize_t read(void *buf, size_t len) override { return ::read(fd_, buf, len); }
+  ssize_t read(void *buf, size_t len) override { return ::read(this->fd_, buf, len); }
   ssize_t recvfrom(void *buf, size_t len, sockaddr *addr, socklen_t *addr_len) override {
 #if defined(USE_ESP32) || defined(USE_HOST)
     return ::recvfrom(this->fd_, buf, len, 0, addr, addr_len);
@@ -129,41 +127,52 @@ class BSDSocketImpl : public Socket {
   }
   ssize_t readv(const struct iovec *iov, int iovcnt) override {
 #if defined(USE_ESP32)
-    return ::lwip_readv(fd_, iov, iovcnt);
+    return ::lwip_readv(this->fd_, iov, iovcnt);
 #else
-    return ::readv(fd_, iov, iovcnt);
+    return ::readv(this->fd_, iov, iovcnt);
 #endif
   }
-  ssize_t write(const void *buf, size_t len) override { return ::write(fd_, buf, len); }
+  ssize_t write(const void *buf, size_t len) override { return ::write(this->fd_, buf, len); }
-  ssize_t send(void *buf, size_t len, int flags) { return ::send(fd_, buf, len, flags); }
+  ssize_t send(void *buf, size_t len, int flags) { return ::send(this->fd_, buf, len, flags); }
   ssize_t writev(const struct iovec *iov, int iovcnt) override {
 #if defined(USE_ESP32)
-    return ::lwip_writev(fd_, iov, iovcnt);
+    return ::lwip_writev(this->fd_, iov, iovcnt);
 #else
-    return ::writev(fd_, iov, iovcnt);
+    return ::writev(this->fd_, iov, iovcnt);
 #endif
   }
 
   ssize_t sendto(const void *buf, size_t len, int flags, const struct sockaddr *to, socklen_t tolen) override {
-    return ::sendto(fd_, buf, len, flags, to, tolen);  // NOLINT(readability-suspicious-call-argument)
+    return ::sendto(this->fd_, buf, len, flags, to, tolen);  // NOLINT(readability-suspicious-call-argument)
   }
 
   int setblocking(bool blocking) override {
-    int fl = ::fcntl(fd_, F_GETFL, 0);
+    int fl = ::fcntl(this->fd_, F_GETFL, 0);
     if (blocking) {
       fl &= ~O_NONBLOCK;
     } else {
       fl |= O_NONBLOCK;
     }
-    ::fcntl(fd_, F_SETFL, fl);
+    ::fcntl(this->fd_, F_SETFL, fl);
     return 0;
   }
 
-  int get_fd() const override { return fd_; }
+  int get_fd() const override { return this->fd_; }
+
+#ifdef USE_SOCKET_SELECT_SUPPORT
+  bool ready() const override {
+    if (!this->loop_monitored_)
+      return true;
+    return App.is_socket_ready(this->fd_);
+  }
+#endif
 
  protected:
   int fd_;
-  bool closed_ = false;
+  bool closed_{false};
+#ifdef USE_SOCKET_SELECT_SUPPORT
+  bool loop_monitored_{false};
+#endif
 };
 
 // Helper to create a socket with optional monitoring
@@ -182,7 +191,6 @@ std::unique_ptr<Socket> socket_loop_monitored(int domain, int type, int protocol
   return create_socket(domain, type, protocol, true);
 }
 
-}  // namespace socket
+}  // namespace esphome::socket
-}  // namespace esphome
 
 #endif  // USE_SOCKET_IMPL_BSD_SOCKETS

esphome/components/socket/lwip_raw_tcp_impl.cpp

@@ -18,8 +18,7 @@
 #include <coredecls.h>  // For esp_schedule()
 #endif
 
-namespace esphome {
+namespace esphome::socket {
-namespace socket {
 
 #ifdef USE_ESP8266
 // Flag to signal socket activity - checked by socket_delay() to exit early
@@ -711,7 +710,6 @@ std::unique_ptr<Socket> socket_loop_monitored(int domain, int type, int protocol
   return socket(domain, type, protocol);
 }
 
-}  // namespace socket
+}  // namespace esphome::socket
-}  // namespace esphome
 
 #endif  // USE_SOCKET_IMPL_LWIP_TCP

esphome/components/socket/lwip_sockets_impl.cpp

@@ -7,8 +7,7 @@
 #include <cstring>
 #include "esphome/core/application.h"
 
-namespace esphome {
+namespace esphome::socket {
-namespace socket {
 
 std::string format_sockaddr(const struct sockaddr_storage &storage) {
   if (storage.ss_family == AF_INET) {
@@ -37,11 +36,11 @@ class LwIPSocketImpl : public Socket {
   LwIPSocketImpl(int fd, bool monitor_loop = false) : fd_(fd) {
 #ifdef USE_SOCKET_SELECT_SUPPORT
     // Register new socket with the application for select() if monitoring requested
-    if (monitor_loop && fd_ >= 0) {
+    if (monitor_loop && this->fd_ >= 0) {
       // Only set loop_monitored_ to true if registration succeeds
-      loop_monitored_ = App.register_socket_fd(fd_);
+      this->loop_monitored_ = App.register_socket_fd(this->fd_);
     } else {
-      loop_monitored_ = false;
+      this->loop_monitored_ = false;
     }
 #else
     // Without select support, ignore monitor_loop parameter
@@ -49,96 +48,108 @@ class LwIPSocketImpl : public Socket {
 #endif
   }
   ~LwIPSocketImpl() override {
-    if (!closed_) {
+    if (!this->closed_) {
-      close();  // NOLINT(clang-analyzer-optin.cplusplus.VirtualCall)
+      this->close();  // NOLINT(clang-analyzer-optin.cplusplus.VirtualCall)
     }
   }
-  int connect(const struct sockaddr *addr, socklen_t addrlen) override { return lwip_connect(fd_, addr, addrlen); }
+  int connect(const struct sockaddr *addr, socklen_t addrlen) override {
+    return lwip_connect(this->fd_, addr, addrlen);
+  }
   std::unique_ptr<Socket> accept(struct sockaddr *addr, socklen_t *addrlen) override {
-    return accept_impl_(addr, addrlen, false);
+    int fd = lwip_accept(this->fd_, addr, addrlen);
-  }
-  std::unique_ptr<Socket> accept_loop_monitored(struct sockaddr *addr, socklen_t *addrlen) override {
-    return accept_impl_(addr, addrlen, true);
-  }
-
- private:
-  std::unique_ptr<Socket> accept_impl_(struct sockaddr *addr, socklen_t *addrlen, bool loop_monitored) {
-    int fd = lwip_accept(fd_, addr, addrlen);
     if (fd == -1)
       return {};
-    return make_unique<LwIPSocketImpl>(fd, loop_monitored);
+    return make_unique<LwIPSocketImpl>(fd, false);
+  }
+  std::unique_ptr<Socket> accept_loop_monitored(struct sockaddr *addr, socklen_t *addrlen) override {
+    int fd = lwip_accept(this->fd_, addr, addrlen);
+    if (fd == -1)
+      return {};
+    return make_unique<LwIPSocketImpl>(fd, true);
   }
 
- public:
+  int bind(const struct sockaddr *addr, socklen_t addrlen) override { return lwip_bind(this->fd_, addr, addrlen); }
-  int bind(const struct sockaddr *addr, socklen_t addrlen) override { return lwip_bind(fd_, addr, addrlen); }
   int close() override {
-    if (!closed_) {
+    if (!this->closed_) {
 #ifdef USE_SOCKET_SELECT_SUPPORT
       // Unregister from select() before closing if monitored
-      if (loop_monitored_) {
+      if (this->loop_monitored_) {
-        App.unregister_socket_fd(fd_);
+        App.unregister_socket_fd(this->fd_);
       }
 #endif
-      int ret = lwip_close(fd_);
+      int ret = lwip_close(this->fd_);
-      closed_ = true;
+      this->closed_ = true;
       return ret;
     }
     return 0;
   }
-  int shutdown(int how) override { return lwip_shutdown(fd_, how); }
+  int shutdown(int how) override { return lwip_shutdown(this->fd_, how); }
 
-  int getpeername(struct sockaddr *addr, socklen_t *addrlen) override { return lwip_getpeername(fd_, addr, addrlen); }
+  int getpeername(struct sockaddr *addr, socklen_t *addrlen) override {
+    return lwip_getpeername(this->fd_, addr, addrlen);
+  }
   std::string getpeername() override {
     struct sockaddr_storage storage;
     socklen_t len = sizeof(storage);
-    int err = this->getpeername((struct sockaddr *) &storage, &len);
+    if (lwip_getpeername(this->fd_, (struct sockaddr *) &storage, &len) != 0)
-    if (err != 0)
       return {};
     return format_sockaddr(storage);
   }
-  int getsockname(struct sockaddr *addr, socklen_t *addrlen) override { return lwip_getsockname(fd_, addr, addrlen); }
+  int getsockname(struct sockaddr *addr, socklen_t *addrlen) override {
+    return lwip_getsockname(this->fd_, addr, addrlen);
+  }
   std::string getsockname() override {
     struct sockaddr_storage storage;
     socklen_t len = sizeof(storage);
-    int err = this->getsockname((struct sockaddr *) &storage, &len);
+    if (lwip_getsockname(this->fd_, (struct sockaddr *) &storage, &len) != 0)
-    if (err != 0)
       return {};
     return format_sockaddr(storage);
   }
   int getsockopt(int level, int optname, void *optval, socklen_t *optlen) override {
-    return lwip_getsockopt(fd_, level, optname, optval, optlen);
+    return lwip_getsockopt(this->fd_, level, optname, optval, optlen);
   }
   int setsockopt(int level, int optname, const void *optval, socklen_t optlen) override {
-    return lwip_setsockopt(fd_, level, optname, optval, optlen);
+    return lwip_setsockopt(this->fd_, level, optname, optval, optlen);
   }
-  int listen(int backlog) override { return lwip_listen(fd_, backlog); }
+  int listen(int backlog) override { return lwip_listen(this->fd_, backlog); }
-  ssize_t read(void *buf, size_t len) override { return lwip_read(fd_, buf, len); }
+  ssize_t read(void *buf, size_t len) override { return lwip_read(this->fd_, buf, len); }
   ssize_t recvfrom(void *buf, size_t len, sockaddr *addr, socklen_t *addr_len) override {
-    return lwip_recvfrom(fd_, buf, len, 0, addr, addr_len);
+    return lwip_recvfrom(this->fd_, buf, len, 0, addr, addr_len);
   }
-  ssize_t readv(const struct iovec *iov, int iovcnt) override { return lwip_readv(fd_, iov, iovcnt); }
+  ssize_t readv(const struct iovec *iov, int iovcnt) override { return lwip_readv(this->fd_, iov, iovcnt); }
-  ssize_t write(const void *buf, size_t len) override { return lwip_write(fd_, buf, len); }
+  ssize_t write(const void *buf, size_t len) override { return lwip_write(this->fd_, buf, len); }
-  ssize_t send(void *buf, size_t len, int flags) { return lwip_send(fd_, buf, len, flags); }
+  ssize_t send(void *buf, size_t len, int flags) { return lwip_send(this->fd_, buf, len, flags); }
-  ssize_t writev(const struct iovec *iov, int iovcnt) override { return lwip_writev(fd_, iov, iovcnt); }
+  ssize_t writev(const struct iovec *iov, int iovcnt) override { return lwip_writev(this->fd_, iov, iovcnt); }
   ssize_t sendto(const void *buf, size_t len, int flags, const struct sockaddr *to, socklen_t tolen) override {
-    return lwip_sendto(fd_, buf, len, flags, to, tolen);
+    return lwip_sendto(this->fd_, buf, len, flags, to, tolen);
   }
   int setblocking(bool blocking) override {
-    int fl = lwip_fcntl(fd_, F_GETFL, 0);
+    int fl = lwip_fcntl(this->fd_, F_GETFL, 0);
     if (blocking) {
       fl &= ~O_NONBLOCK;
     } else {
       fl |= O_NONBLOCK;
     }
-    lwip_fcntl(fd_, F_SETFL, fl);
+    lwip_fcntl(this->fd_, F_SETFL, fl);
     return 0;
   }
 
-  int get_fd() const override { return fd_; }
+  int get_fd() const override { return this->fd_; }
+
+#ifdef USE_SOCKET_SELECT_SUPPORT
+  bool ready() const override {
+    if (!this->loop_monitored_)
+      return true;
+    return App.is_socket_ready(this->fd_);
+  }
+#endif
 
  protected:
   int fd_;
-  bool closed_ = false;
+  bool closed_{false};
+#ifdef USE_SOCKET_SELECT_SUPPORT
+  bool loop_monitored_{false};
+#endif
 };
 
 // Helper to create a socket with optional monitoring
@@ -157,7 +168,6 @@ std::unique_ptr<Socket> socket_loop_monitored(int domain, int type, int protocol
   return create_socket(domain, type, protocol, true);
 }
 
-}  // namespace socket
+}  // namespace esphome::socket
-}  // namespace esphome
 
 #endif  // USE_SOCKET_IMPL_LWIP_SOCKETS

esphome/components/socket/socket.cpp

@@ -6,33 +6,10 @@
 #include "esphome/core/log.h"
 #include "esphome/core/application.h"
 
-namespace esphome {
+namespace esphome::socket {
-namespace socket {
 
 Socket::~Socket() {}
 
-bool Socket::ready() const {
-#ifdef USE_SOCKET_SELECT_SUPPORT
-  if (!loop_monitored_) {
-    // Non-monitored sockets always return true (assume data may be available)
-    return true;
-  }
-
-  // For loop-monitored sockets, check with the Application's select() results
-  int fd = this->get_fd();
-  if (fd < 0) {
-    // No valid file descriptor, assume ready (fallback behavior)
-    return true;
-  }
-
-  return App.is_socket_ready(fd);
-#else
-  // Without select() support, we can't monitor sockets in the loop
-  // Always return true (assume data may be available)
-  return true;
-#endif
-}
-
 std::unique_ptr<Socket> socket_ip(int type, int protocol) {
 #if USE_NETWORK_IPV6
   return socket(AF_INET6, type, protocol);
@@ -113,6 +90,5 @@ socklen_t set_sockaddr_any(struct sockaddr *addr, socklen_t addrlen, uint16_t po
   return sizeof(sockaddr_in);
 #endif /* USE_NETWORK_IPV6 */
 }
-}  // namespace socket
+}  // namespace esphome::socket
-}  // namespace esphome
 #endif

esphome/components/socket/socket.h

@@ -6,8 +6,7 @@
 #include "headers.h"
 
 #if defined(USE_SOCKET_IMPL_LWIP_TCP) || defined(USE_SOCKET_IMPL_LWIP_SOCKETS) || defined(USE_SOCKET_IMPL_BSD_SOCKETS)
-namespace esphome {
+namespace esphome::socket {
-namespace socket {
 
 class Socket {
  public:
@@ -54,12 +53,7 @@ class Socket {
   /// Check if socket has data ready to read
   /// For loop-monitored sockets, checks with the Application's select() results
   /// For non-monitored sockets, always returns true (assumes data may be available)
-  bool ready() const;
+  virtual bool ready() const { return true; }
-
- protected:
-#ifdef USE_SOCKET_SELECT_SUPPORT
-  bool loop_monitored_{false};  ///< Whether this socket is monitored by the event loop
-#endif
 };
 
 /// Create a socket of the given domain, type and protocol.
@@ -91,6 +85,5 @@ void socket_delay(uint32_t ms);
 void socket_wake();
 #endif
 
-}  // namespace socket
+}  // namespace esphome::socket
-}  // namespace esphome
 #endif