[espnow][udp] Fixes from code review

- Add point_to_point to the UDP link, to override the guess that an
  address ending in .255 is a broadcast address, for subnets other than
  /24
- Warn, at most once a minute, when a point to point UDP link drops a
  packet from another sender
- Say that on ESP8266 an ESP-NOW peer link also receives the peer's
  broadcasts

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

esphome/components/espnow/packet_interface/espnow_packet_interface.h

@@ -18,7 +18,7 @@ namespace esphome::espnow {
  * it are received, so the link provides the addressing capability itself. With the broadcast address, packets go to
  * every device on the channel and every broadcast packet is received, from known and unknown senders; an addressing
  * layer then tells the devices apart. ESP8266 does not report a packet's destination, so there a broadcast link also
- * receives packets sent to this device.
+ * receives packets sent to this device, and a peer link also receives the peer's broadcasts.
  *
  * ESP-NOW queues each packet and reports later whether it was delivered; a reliability layer above resends what is
  * lost.

esphome/components/udp/packet_interface/__init__.py

@@ -23,6 +23,8 @@ CODEOWNERS = ["@clydebarrow"]
 
 DEPENDENCIES = ["udp"]
 
+CONF_POINT_TO_POINT = "point_to_point"
+
 # Size of the receive buffer of the udp component
 MAX_PACKET_SIZE = 508
 
@@ -31,21 +33,35 @@ UdpPacketInterface = udp_ns.class_(
     packet_interface.PacketInterface,
 )
 
-CONFIG_SCHEMA = packet_interface_schema(
-    UdpPacketInterface, MAX_PACKET_SIZE, MAX_PACKET_SIZE
-).extend(UDP_SCHEMA)
+CONFIG_SCHEMA = (
+    packet_interface_schema(UdpPacketInterface, MAX_PACKET_SIZE, MAX_PACKET_SIZE)
+    .extend(UDP_SCHEMA)
+    .extend(
+        {
+            # Overrides the guess from the udp component's addresses, which cannot see the subnet
+            cv.Optional(CONF_POINT_TO_POINT): cv.boolean,
+        }
+    )
+)
 
 
-def _peer(udp_config: ConfigType) -> IPv4Address | None:
+def _peer(udp_config: ConfigType, config: ConfigType) -> IPv4Address | None:
     """The one peer, when the UDP component sends to a single unicast IPv4 address.
 
-    An address ending in .255 is taken as a subnet broadcast, as the subnet is not known here.
+    Unless point_to_point says otherwise, an address ending in .255 is taken as a subnet broadcast, as the subnet is
+    not known here.
     """
+    point_to_point = config.get(CONF_POINT_TO_POINT)
     match udp_config[CONF_ADDRESSES]:
-        case [IPv4Address() as address] if not (
-            address.is_multicast or address.packed[3] == 255
-        ):
-            return address
+        case [IPv4Address() as address] if not address.is_multicast:
+            if point_to_point is None:
+                point_to_point = address.packed[3] != 255
+            return address if point_to_point else None
+    if point_to_point:
+        raise cv.Invalid(
+            f"'{CONF_POINT_TO_POINT}' needs the UDP component to send to one unicast IPv4 address",
+            path=[CONF_POINT_TO_POINT],
+        )
     return None
 
 
@@ -56,7 +72,7 @@ def _udp_config(full_config: ConfigType, config: ConfigType) -> ConfigType:
 
 
 def _final_validate(config: ConfigType) -> ConfigType:
-    if (peer := _peer(_udp_config(fv.full_config.get(), config))) is None:
+    if (peer := _peer(_udp_config(fv.full_config.get(), config), config)) is None:
         check_broadcast_link(config)
     elif CONF_ADDRESSING in config:
         raise cv.Invalid(
@@ -75,7 +91,7 @@ async def to_code(config: ConfigType) -> None:
     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())
-    peer = _peer(_udp_config(CORE.config, config))
+    peer = _peer(_udp_config(CORE.config, config), config)
     peer_bytes = peer.packed if peer is not None else bytes(4)
     # The copy buffer's size is known here, so it is a static array rather than allocated at run time
     tx_buffer = f"{config[CONF_ID].id}__tx_buffer"

esphome/components/udp/packet_interface/udp_packet_interface.cpp

@@ -2,6 +2,7 @@
 #ifdef USE_NETWORK
 #include <cstring>
 #include "esphome/components/network/util.h"
+#include "esphome/core/application.h"
 #include "esphome/core/log.h"
 
 namespace esphome::udp {
@@ -24,8 +25,10 @@ void UdpPacketInterface::setup() {
 }
 
 void UdpPacketInterface::receive_(std::span<const uint8_t> data, const sockaddr *source) {
-  if (!this->accepts_from_(source))
+  if (!this->accepts_from_(source)) {
+    this->warn_dropped_(source);
     return;
+  }
   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_);
@@ -41,6 +44,24 @@ bool UdpPacketInterface::accepts_from_(const sockaddr *source) const {
   return socket::sockaddr_to_ipv4(source, &address) && memcmp(&address, this->peer_.data(), sizeof(address)) == 0;
 }
 
+void UdpPacketInterface::warn_dropped_(const sockaddr *source) {
+  static constexpr uint32_t WARNING_INTERVAL_MS = 60000;
+  uint32_t now = App.get_loop_component_start_time();
+  if (this->drop_warned_ && now - this->last_drop_warning_ms_ < WARNING_INTERVAL_MS)
+    return;
+  this->drop_warned_ = true;
+  this->last_drop_warning_ms_ = now;
+  uint32_t address = 0;
+  socket::sockaddr_to_ipv4(source, &address);
+  uint8_t from[4];
+  memcpy(from, &address, sizeof(from));
+  ESP_LOGW(TAG,
+           "%s: Packet from %u.%u.%u.%u dropped, as only %u.%u.%u.%u is accepted; set point_to_point: false if that "
+           "is a broadcast address",
+           this->name_, from[0], from[1], from[2], from[3], this->peer_[0], this->peer_[1], this->peer_[2],
+           this->peer_[3]);
+}
+
 bool UdpPacketInterface::send_to_interface(const PacketBuffer &data, const PacketMetaData &meta_data) {
   size_t size = data.size();
   if (size > this->max_packet_size_) {

esphome/components/udp/packet_interface/udp_packet_interface.h

@@ -18,7 +18,8 @@ namespace esphome::udp {
  *
  * When the UDP component sends to one unicast IPv4 address, the link joins exactly two devices: it accepts packets
  * only from that address, and provides the addressing capability itself. Otherwise every packet is accepted, and an
- * addressing layer can tell the devices apart.
+ * addressing layer can tell the devices apart. Packets from other senders are dropped with a warning at most once a
+ * minute, as they suggest that the address is a broadcast address the link took for a device.
  */
 class UdpPacketInterface : public packet_interface::PacketInterface {
  public:
@@ -34,10 +35,13 @@ class UdpPacketInterface : public packet_interface::PacketInterface {
   void receive_(std::span<const uint8_t> data, const sockaddr *source);
   /// True if packets from \p source are accepted: from anywhere, or with a peer, only from the peer.
   bool accepts_from_(const sockaddr *source) const;
+  void warn_dropped_(const sockaddr *source);
 
   UDPComponent *parent_;
   const std::span<uint8_t> tx_buffer_;
   std::array<uint8_t, 4> peer_;
+  uint32_t last_drop_warning_ms_{0};
+  bool drop_warned_{false};
 };
 }  // namespace esphome::udp
 #endif

tests/unit_tests/test_udp_packet_interface.py

@@ -14,19 +14,40 @@ import esphome.final_validate as fv
 
 
 @pytest.mark.parametrize(
-    ("addresses", "peer"),
+    ("addresses", "point_to_point", "peer"),
     [
-        ([IPv4Address("192.168.1.60")], IPv4Address("192.168.1.60")),
-        ([IPv4Address("255.255.255.255")], None),
-        ([IPv4Address("192.168.1.255")], None),
-        ([IPv4Address("239.0.60.53")], None),
-        ([IPv4Address("192.168.1.60"), IPv4Address("192.168.1.61")], None),
-        ([IPv6Address("2001:db8::1")], None),
+        ([IPv4Address("192.168.1.60")], None, IPv4Address("192.168.1.60")),
+        ([IPv4Address("255.255.255.255")], None, None),
+        ([IPv4Address("192.168.1.255")], None, None),
+        ([IPv4Address("239.0.60.53")], None, None),
+        ([IPv4Address("192.168.1.60"), IPv4Address("192.168.1.61")], None, None),
+        ([IPv6Address("2001:db8::1")], None, None),
+        # A directed broadcast on a /25, and a host ending in .255 on a /23
+        ([IPv4Address("192.168.1.127")], False, None),
+        ([IPv4Address("192.168.0.255")], True, IPv4Address("192.168.0.255")),
     ],
 )
-def test_peer(addresses: list, peer: IPv4Address | None) -> None:
-    """Only one unicast IPv4 destination makes the link point to point."""
-    assert udp_packet_interface._peer({CONF_ADDRESSES: addresses}) == peer
+def test_peer(
+    addresses: list, point_to_point: bool | None, peer: IPv4Address | None
+) -> None:
+    """Only one unicast IPv4 destination makes the link point to point, unless point_to_point says otherwise."""
+    config = {} if point_to_point is None else {"point_to_point": point_to_point}
+    assert udp_packet_interface._peer({CONF_ADDRESSES: addresses}, config) == peer
+
+
+@pytest.mark.parametrize(
+    "addresses",
+    [
+        [IPv4Address("239.0.60.53")],
+        [IPv4Address("192.168.1.60"), IPv4Address("192.168.1.61")],
+    ],
+)
+def test_point_to_point_needs_one_unicast_address(addresses: list) -> None:
+    """point_to_point cannot make a multicast or several addresses into one peer."""
+    with pytest.raises(cv.Invalid, match="one unicast IPv4 address"):
+        udp_packet_interface._peer(
+            {CONF_ADDRESSES: addresses}, {"point_to_point": True}
+        )
 
 
 def _final_validate(addresses: list, layers: dict) -> None:
@@ -46,6 +67,9 @@ def test_point_to_point_rejects_addressing(setup_core) -> None:
     with pytest.raises(cv.Invalid, match="already addressed"):
         _final_validate([IPv4Address("192.168.1.60")], addressing)
     _final_validate([IPv4Address("192.168.1.255")], addressing)
+    _final_validate(
+        [IPv4Address("192.168.1.127")], {**addressing, "point_to_point": False}
+    )
 
 
 def test_broadcast_needs_addressing_for_two_device_layers(setup_core) -> None: