feat(server)!: multi-response handler, listener injection, UDP identification (0.4.0)

ARCHITECTURE.md

@@ -311,7 +311,7 @@ seam described above (section 3) usable.
 | `src/socket_manager.rs` | Owns the spawned socket task; bridges `FramedRead`/`FramedWrite` to two mpsc channels; enforces the general inactivity timeout |
 | `src/client_inner.rs` | The client state machine: a `ControlMessage` enum plus a select loop matching responses to pending requests |
 | `src/client.rs` | Public `Client<Conn>` — connect, routing activation, send/receive diagnostic messages |
-| `src/server.rs` | `Server<T>`, `ServerConnectionHandler`, `ClientConnectionInfo` |
+| `src/server.rs` | `Server<T>`, `ServerConnectionHandler`, `ResponseWriter`, `ClientConnectionInfo` |
 
 The client is a channel sandwich, described in one comment at the top of
 `src/client_inner.rs`:
@@ -366,6 +366,9 @@ future change can silently reintroduce a dropped-`Sender` bug.
   regenerate them**. Note they cover *bodies*, encoded via `Encode`; they do not
   independently pin header payload types (see section 7.2).
 - `tests/integration_test.rs` — real client-against-real-server over loopback TCP.
+- `tests/udp_identification.rs` — drives `Server::run_udp_responder` on a loopback
+  `UdpSocket`: an answered broadcast probe, silence for the directed EID/VIN
+  forms, and a responder that keeps serving after datagrams it cannot answer.
 - `tests/nested_encode.rs` — a permanent regression test for the encode hot path.
 - `examples/bare_metal_codec.rs` — encode into a `[u8; N]`, frame, decode; builds
   and runs with `--no-default-features`. This is the executable proof that the
@@ -377,42 +380,38 @@ future change can silently reintroduce a dropped-`Sender` bug.
 ## 7. Known issues and deferred work
 
 Everything in this section was found by review during the handoff cleanup. It is
-recorded here so the analysis does not have to be re-derived. None of it is
-scheduled; all of it is a decision for the crate's next owner.
+recorded here so the analysis does not have to be re-derived. Except where an
+entry is marked RESOLVED, none of it is scheduled; it is a decision for the
+crate's next owner. Resolved entries are kept because the analysis that led to
+the fix is still the fastest way to understand the shape the API ended up with.
 
-### 7.1 `ServerConnectionHandler::diagnostic_message` cannot express correct DoIP behavior
+### 7.1 `ServerConnectionHandler::diagnostic_message` — RESOLVED in 0.4.0
 
-This is the one place where the API shape prevents a correct implementation.
+DoIP prescribes that a DoIP entity receiving a diagnostic message first sends a
+`DiagnosticMessageAck`, and then — separately and later — sends any functional
+(e.g. UDS) response as its own `DiagnosticMessage`. Two messages, in order.
+
+The trait used to return a **single** `OwnedMessage`, so an implementer had to
+choose one of the two and no handler could drive a real UDS tester.
+
+It now takes a sink instead:
 
 ```rust
 async fn diagnostic_message(
     &self,
     message: &DiagnosticMessage<'_>,
-) -> Result<OwnedMessage, Error>;
+    responses: &mut dyn ResponseWriter,
+) -> Result<(), Error>;
 ```
 (`ServerConnectionHandler::diagnostic_message` in `src/server.rs`)
 
-DoIP prescribes that a DoIP entity receiving a diagnostic message first sends a
-`DiagnosticMessageAck`, and then — separately and later — sends any functional
-(e.g. UDS) response as its own `DiagnosticMessage`. Two messages, in order.
-
-The trait returns a **single** `OwnedMessage`, and the dispatch site
-(the `OwnedPayload::DiagnosticMessage` arm of `Server::handle_client_message`)
-maps that one value through `Some(..)` into `Server::handle_client_connection`,
-whose read loop writes at most one message per received message. There is no
-path by which a handler can emit both.
-
-So an implementer must choose: send the required acknowledgement, or send the
-functional response. `examples/echo_server.rs` picks the acknowledgement and
-smuggles the request bytes back inside the ack's `previous_message_data` field —
-which is an echo demo, not protocol-correct behavior, and the example says so in
-a comment (in its `diagnostic_message` implementation, `examples/echo_server.rs`).
-
-**Recommendation:** revisit the trait signature. Plausible shapes are returning a
-collection, taking a sink/writer the handler can push to, or splitting the ack
-decision (which the server could synthesize itself) from the response.
+Each `ResponseWriter::send` writes straight to the connection's framed write
+half, so a handler emits as many messages as the exchange needs — ack, any
+number of NRC `0x78` "response pending" messages, then the final answer — and
+may await arbitrary work between them. `examples/echo_server.rs` shows the
+two-message shape.
 
-Note that `routing_activation` has the same single-`OwnedMessage` return shape,
+Note that `routing_activation` still has the single-`OwnedMessage` return shape,
 but that is fine — routing activation genuinely is one request, one response.
 
 ### 7.2 `diagnostic_message_ack` hardcodes the positive acknowledgement payload type
@@ -568,11 +567,20 @@ returning `true` breaks it. Either delete the field or make the loop honor it.
 ### 7.6 Other rough edges
 
 These are documented in `README.md` under **Status** and are repeated here only
-as a pointer: no TLS; no UDP vehicle announcement or discovery; the server's
-accept loop serves one TCP connection at a time; entity status and vehicle
-identification requests over TCP are silently dropped;
-`ClientConnectionInfo::logical_address` is hard-coded to `0x0000` because the
-server tracks no per-connection state; a failed `accept()` panics the server task.
+as a pointer: no TLS; no unsolicited UDP vehicle announcement at power-on
+(identification requests over UDP *are* answered, but only by
+`Server::run_udp_responder` on a socket the caller binds and drives — `run_server`
+binds TCP alone — and only the broadcast `0x0001` form, since `Payload::decode`
+discards the EID/VIN the directed forms name); the server's accept loop serves
+one TCP connection at a time; entity status and vehicle identification requests
+over TCP are silently dropped; `ClientConnectionInfo::logical_address` is
+hard-coded to `0x0000` because the server tracks no per-connection state; the
+handler passed to `Server::new` is not validated.
+
+A failed `accept()` no longer panics the server task — as of 0.4.0 both the TCP
+accept loop and the UDP responder log the error, sleep briefly, and continue, so
+neither a transient peer reset nor a persistent condition such as `EMFILE` can
+take the entity down or spin a core.
 
 ---
 

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "simple_doip"
-version = "0.3.1"
+version = "0.4.0"
 edition = "2024"
 rust-version = "1.88"
 description = "An ISO 13400-2 (DoIP) implementation with a no_std, zero-copy protocol core and optional async client and server"
@@ -49,6 +49,10 @@ name = "golden_vectors"
 name = "integration_test"
 required-features = ["client", "server"]
 
+[[test]]
+name = "udp_identification"
+required-features = ["server"]
+
 [[example]]
 name = "simple_client"
 required-features = ["client"]

README.md

@@ -18,22 +18,27 @@ gaps a new integrator should know about before relying on them:
 - **No TLS.** Connections are established in the clear on `TCP_PORT`
   (`13400`); `TCP_TLS_PORT` (`3496`) is defined per ISO 13400-2 but nothing in
   this crate uses it.
-- **No UDP vehicle announcement / discovery.** The server does not send the
-  UDP vehicle-announcement broadcast on startup, nor answer vehicle
-  identification requests over UDP.
-- **The server accepts one TCP connection at a time.** `Server::run_server`'s
-  accept loop awaits each client's connection handling to completion before
-  calling `accept()` again, so a second client cannot connect while the first
-  is still being served.
+- **No unsolicited UDP vehicle announcement.** The server never sends the UDP
+  vehicle-announcement broadcast on startup, so a tester learns of an entity
+  only by asking. Vehicle identification requests over UDP *are* answered, but
+  only by `Server::run_udp_responder`, on a `UdpSocket` the caller bound and
+  drives: `run_server` binds TCP alone, so an entity that starts through it
+  and nothing else is invisible to a discovery probe. `run_udp_responder`
+  answers the broadcast request form (`0x0001`) only; the directed
+  with-EID (`0x0002`) and with-VIN (`0x0003`) forms are declined, because
+  `Payload::decode` discards the EID/VIN bytes and the responder cannot tell
+  whether it is the addressee.
+- **The server accepts one TCP connection at a time.** The accept loop in
+  `Server::run_server_with_listener` (which `Server::run_server` delegates to)
+  awaits each client's connection handling to completion before calling
+  `accept()` again, so a second client cannot connect while the first is still
+  being served — and one tester that connects and then stalls wedges that
+  entity until it disconnects.
 - **Entity status requests and vehicle identification requests over TCP are
   silently dropped.** `Server::handle_client_message` logs a warning and sends
   no reply for either, so a tester that asks gets silence rather than an error
-  or a negative response.
-- **A connection handler can only answer a diagnostic message with a single
-  message.** `ServerConnectionHandler::diagnostic_message` returns one
-  `OwnedMessage`, so a handler can send the required acknowledgement *or* a
-  functional response, not the acknowledgement followed by a separate
-  response as DoIP prescribes.
+  or a negative response. Identification requests are answered on the UDP path
+  only.
 - **A `DiagnosticMessage` arriving while the client is waiting for an ACK is
   silently discarded.** After `Client::send_diagnostic_message`, the inner
   client is in its `AwaitAck` state; a `DiagnosticMessage` that arrives before
@@ -46,11 +51,10 @@ gaps a new integrator should know about before relying on them:
 - **`ClientConnectionInfo::logical_address` is always `0x0000`.** The server
   does not yet track per-connection logical addresses, so this field is a
   placeholder rather than the client's real address.
-- The handler passed to `Server::new` is not validated, and a failed
-  `accept()` currently panics the server task rather than being handled.
+- The handler passed to `Server::new` is not validated.
 
 None of this blocks bare-metal or single-client use; it matters if you need
-concurrent clients, discovery, or TLS today.
+concurrent clients, unsolicited announcement, or TLS today.
 
 ## Quickstart
 
@@ -127,9 +131,14 @@ cargo test --features client,server
   `TCP_PORT` (`13400`), returning `Error::InvalidPort` — pointing a client at a
   non-standard port requires your own `Connector` implementation.
 
-  `echo_server` answers a diagnostic message with a positive acknowledgement
-  that carries the received bytes back in its previous-message-data field; see
-  the single-response handler limitation under [Status](#status).
+  `echo_server` answers a diagnostic message the way DoIP prescribes: first a
+  positive acknowledgement carrying the received bytes back in its
+  previous-message-data field, then the echo itself as a separate
+  `DiagnosticMessage`. Both are written into the `ResponseWriter` the handler
+  is given, which is the shape a real UDS response takes.
+
+  `echo_server` calls `run_server`, so it serves TCP only and does not answer
+  UDP discovery probes; see `Server::run_udp_responder` for that half.
 
 ## Relationship to `automotive-wire-codec`
 

examples/echo_server.rs

@@ -6,7 +6,7 @@ use simple_doip::{
         DiagnosticAckCode, DiagnosticMessage, OwnedMessage, RoutingActivationRequest,
         RoutingActivationResponseCode,
     },
-    server::{Server, ServerConnectionHandler},
+    server::{ResponseWriter, Server, ServerConnectionHandler},
 };
 use tracing::{debug, info};
 
@@ -53,25 +53,35 @@ impl ServerConnectionHandler for ServerHandler {
     async fn diagnostic_message(
         &self,
         message: &DiagnosticMessage<'_>,
-    ) -> Result<OwnedMessage, Error> {
+        responses: &mut dyn ResponseWriter,
+    ) -> Result<(), Error> {
         debug!(
             "Received diagnostic message from {:?} to {:?}",
             message.source_address, message.target_address
         );
         // A DoIP entity must acknowledge a diagnostic message first; any
-        // functional (UDS) response is a separate, later DiagnosticMessage.
-        // `ServerConnectionHandler::diagnostic_message` can only return a single
-        // message, so this example echoes the received bytes back inside the
-        // positive acknowledgement's previous-message-data field. A real entity
-        // that must also send a UDS response needs its own write path; the
-        // handler trait as it stands cannot emit two messages for one request.
-        Ok(OwnedMessage::diagnostic_message_ack(
-            self.protocol_version(),
-            message.target_address, // We are the target, so we answer as source
-            message.source_address, // ...back to the tester that asked
-            DiagnosticAckCode::RoutingConfirmationAck,
-            message.user_data.to_vec(),
-        ))
+        // functional (UDS) response is a separate, later DiagnosticMessage. Both
+        // go through `responses`, in that order, which is the sequence a UDS
+        // tester waits for.
+        responses
+            .send(OwnedMessage::diagnostic_message_ack(
+                self.protocol_version(),
+                message.target_address, // We are the target, so we answer as source
+                message.source_address, // ...back to the tester that asked
+                DiagnosticAckCode::RoutingConfirmationAck,
+                message.user_data.to_vec(),
+            ))
+            .await?;
+        // The echo itself: a real entity would put its UDS response here.
+        responses
+            .send(OwnedMessage::diagnostic_message(
+                self.protocol_version(),
+                message.target_address,
+                message.source_address,
+                message.user_data.to_vec(),
+            ))
+            .await?;
+        Ok(())
     }
 }
 

src/messages/mod.rs

@@ -464,6 +464,37 @@ impl OwnedMessage {
             payload: OwnedPayload::DiagnosticMessageAck(ack),
         }
     }
+
+    /// Construct a directed reply to a vehicle identification request.
+    ///
+    /// The header is stamped with [`PayloadType::VehicleAnnouncement`] (0x0004)
+    /// because ISO 13400-2 defines exactly one wire payload type for both the
+    /// unsolicited announcement and the directed reply. The payload nonetheless
+    /// uses [`OwnedPayload::VehicleIdentificationResponse`], which encodes
+    /// identically but records at construction time that this is an answer to a
+    /// request rather than a spontaneous announcement. A peer decoding these
+    /// bytes gets [`Payload::VehicleAnnouncement`] either way.
+    ///
+    /// # Panics
+    /// Panics if the payload's `encoded_size` errors, or if the resulting size
+    /// does not fit in a `u32`. Neither is reachable here: the payload is fixed
+    /// size (33 bytes) and `encoded_size` is pure arithmetic over the struct's
+    /// own fields, with no I/O to fail.
+    #[must_use]
+    pub fn vehicle_identification_response(
+        protocol_version: ProtocolVersion,
+        response: VehicleIdentificationResponse,
+    ) -> OwnedMessage {
+        let payload_size = payload_len(&Payload::VehicleIdentificationResponse(response));
+        OwnedMessage {
+            header: Header::new(
+                protocol_version,
+                PayloadType::VehicleAnnouncement,
+                payload_size,
+            ),
+            payload: OwnedPayload::VehicleIdentificationResponse(response),
+        }
+    }
 }
 
 /// Encode delegates through the borrowed view so there is exactly one wire