[udp] Fixes from code review

- Enforce max_packet_size on send and receive
- Copy split packets into a buffer allocated in setup, not on the stack
- Add unit tests for the link's send and receive paths
- Explain why the broadcast socket is always enabled

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

esphome/components/udp/packet_interface/__init__.py

@@ -12,7 +12,7 @@ CODEOWNERS = ["@clydebarrow"]
 
 DEPENDENCIES = ["udp"]
 
-# Size of the receive buffer in udp_component.h
+# Size of the receive buffer of the udp component
 MAX_PACKET_SIZE = 508
 
 UdpPacketInterface = udp_ns.class_(
@@ -28,5 +28,6 @@ CONFIG_SCHEMA = packet_interface_schema(
 async def to_code(config: ConfigType) -> None:
     udp_var = await cg.get_variable(config[CONF_UDP_ID])
     cg.add(udp_var.set_should_listen())
+    # The udp component sends to its addresses through its broadcast socket
     cg.add(udp_var.set_should_broadcast())
     await new_packet_interface(config, udp_var)

esphome/components/udp/packet_interface/udp_packet_interface.cpp

@@ -1,6 +1,5 @@
 #include "udp_packet_interface.h"
 #ifdef USE_NETWORK
-#include <array>
 #include "esphome/components/network/util.h"
 #include "esphome/core/log.h"
 
@@ -12,25 +11,33 @@ using packet_interface::PacketMetaData;
 ESPHOME_LOG_TAG(TAG, "udp.packet_interface");
 
 void UdpPacketInterface::setup() {
-  this->parent_->add_listener([this](std::span<const uint8_t> buf) {
-    this->on_receive_from_interface_(PacketBuffer(buf.data(), buf.size()), {});
-  });
+  this->tx_buffer_ = std::make_unique<uint8_t[]>(this->max_packet_size_);
+  this->parent_->add_listener([this](std::span<const uint8_t> data) { this->receive_(data); });
+}
+
+void UdpPacketInterface::receive_(std::span<const uint8_t> data) {
+  if (data.size() > this->max_packet_size_) {
+    ESP_LOGV(TAG, "%s: Packet of %zu bytes is larger than %zu, dropped", this->name_, data.size(),
+             this->max_packet_size_);
+    return;
+  }
+  this->on_receive_from_interface_(PacketBuffer(data.data(), data.size()), {});
 }
 
 bool UdpPacketInterface::send_to_interface(const PacketBuffer &data, const PacketMetaData &meta_data) {
+  size_t size = data.size();
+  if (size > this->max_packet_size_) {
+    ESP_LOGW(TAG, "%s: Packet of %zu bytes is larger than %zu", this->name_, size, this->max_packet_size_);
+    return false;
+  }
   if (!network::is_connected())
     return false;
   if (data.is_contiguous()) {
     auto span = data.as_span();
     return this->parent_->send_packet(span.data(), span.size());
   }
-  if (data.size() > MAX_PACKET_SIZE) {
-    ESP_LOGW(TAG, "Packet of %zu bytes is larger than %zu", data.size(), MAX_PACKET_SIZE);
-    return false;
-  }
-  std::array<uint8_t, MAX_PACKET_SIZE> buf;
-  size_t len = data.copy_to(buf.data(), buf.size());
-  return this->parent_->send_packet(buf.data(), len);
+  data.copy_to(this->tx_buffer_.get(), size);
+  return this->parent_->send_packet(this->tx_buffer_.get(), size);
 }
 
 }  // namespace esphome::udp

esphome/components/udp/packet_interface/udp_packet_interface.h

@@ -4,11 +4,20 @@
 
 // udp_component.h declares nothing without USE_NETWORK
 #ifdef USE_NETWORK
+#include <memory>
+#include <span>
+
 #include "esphome/components/packet_interface/packet_interface.h"
 #include "../udp_component.h"
 
 namespace esphome::udp {
 
+/**
+ * A packet interface link over UDP: each packet is one datagram, sent to every address of the UDP component.
+ *
+ * Every datagram received on the listen port is passed up, from any sender, so on a shared network an addressing
+ * or encryption layer is needed to tell the peer's packets from others.
+ */
 class UdpPacketInterface : public packet_interface::PacketInterface {
  public:
   explicit UdpPacketInterface(UDPComponent *udp_component) : parent_(udp_component) {}
@@ -18,7 +27,11 @@ class UdpPacketInterface : public packet_interface::PacketInterface {
                          const packet_interface::PacketMetaData &meta_data) override;
 
  protected:
+  void receive_(std::span<const uint8_t> data);
+
   UDPComponent *parent_;
+  // For packets split across buffers, allocated in setup()
+  std::unique_ptr<uint8_t[]> tx_buffer_;
 };
 }  // namespace esphome::udp
 #endif

tests/components/udp/__init__.py

@@ -0,0 +1,6 @@
+from tests.testing_helpers import ComponentManifestOverride
+
+
+def override_manifest(manifest: ComponentManifestOverride) -> None:
+    # benchmark.yaml configures a udp packet_interface, whose to_code waits on the udp component variable.
+    manifest.enable_codegen()

tests/components/udp/benchmark.yaml

@@ -0,0 +1,5 @@
+udp:
+  addresses: ["192.168.1.255"]
+
+packet_interface:
+  - platform: udp

tests/components/udp/udp_packet_interface.cpp

@@ -0,0 +1,64 @@
+#include <gtest/gtest.h>
+
+#include <cstdint>
+#include <vector>
+
+#include "esphome/components/udp/packet_interface/udp_packet_interface.h"
+
+#ifdef USE_NETWORK
+
+namespace esphome::udp::testing {
+
+using packet_interface::PacketBuffer;
+using packet_interface::PacketMetaData;
+using Packet = std::vector<uint8_t>;
+
+class TestUdp : public UdpPacketInterface {
+ public:
+  explicit TestUdp(UDPComponent *udp) : UdpPacketInterface(udp) {
+    this->set_max_packet_size(8);
+    this->add_packet_interface_listener(
+        [this](const PacketBuffer &data, const PacketMetaData &) { this->received.push_back(data.to_vector()); });
+  }
+  using UdpPacketInterface::receive_;
+
+  std::vector<Packet> received;
+};
+
+// The udp component is not set up, so it has no destinations and a send does nothing
+class UdpPacketInterfaceTest : public ::testing::Test {
+ protected:
+  UDPComponent udp;
+  TestUdp link{&udp};
+};
+
+TEST_F(UdpPacketInterfaceTest, ReceivedPacketsUpToMaxPassedUp) {
+  Packet packet{1, 2, 3, 4, 5, 6, 7, 8};
+  this->link.receive_(packet);
+  Packet oversized{1, 2, 3, 4, 5, 6, 7, 8, 9};
+  this->link.receive_(oversized);
+  EXPECT_EQ(this->link.received, std::vector<Packet>{packet});
+}
+
+TEST_F(UdpPacketInterfaceTest, SendsContiguousAndSplitPackets) {
+  this->link.setup();
+  Packet packet{1, 2, 3, 4};
+  EXPECT_TRUE(this->link.send_to_interface(PacketBuffer(packet), {}));
+  uint8_t header = 0;
+  PacketBuffer body(packet);
+  EXPECT_TRUE(this->link.send_to_interface(PacketBuffer(&header, 1, &body, nullptr, 0), {}));
+}
+
+TEST_F(UdpPacketInterfaceTest, OversizedPacketNotSent) {
+  this->link.setup();
+  Packet packet{1, 2, 3, 4, 5, 6, 7, 8, 9};
+  EXPECT_FALSE(this->link.send_to_interface(PacketBuffer(packet), {}));
+  uint8_t header = 0;
+  Packet body_data{1, 2, 3, 4, 5, 6, 7, 8};
+  PacketBuffer body(body_data);
+  EXPECT_FALSE(this->link.send_to_interface(PacketBuffer(&header, 1, &body, nullptr, 0), {}));
+}
+
+}  // namespace esphome::udp::testing
+
+#endif