phase 19b: EmbassyNetFactory + SocketPool storage by JustinKovacich · Pull Request #99 · luminartech/simple_someip

JustinKovacich · 2026-04-29T12:49:45Z

Real implementation of TransportFactory over embassy-net 0.4. The adapter now claims a buffer slot from a caller-declared &'static SocketPool<POOL, RX_BUF, TX_BUF> on each bind(), constructs an embassy_net::udp::UdpSocket borrowing the slot's RX/TX buffers, and reclaims the slot when the returned EmbassyNetSocket drops.

SocketPool

pub struct SocketPool<const POOL: usize, const RX_BUF: usize, const TX_BUF: usize>
Holds [Slot<RX_BUF, TX_BUF>; POOL] of UnsafeCell-wrapped buffers
- [AtomicBool; POOL] of in-use flags.
const fn new() so the pool can live in a plain static declaration.
unsafe impl Sync justified by the AcqRel CAS handshake serializing per-slot UnsafeCell access.
4-entry PacketMetadata arrays per direction (constant PACKET_METADATA_LEN = 4) — sized for SOME/IP-SD's announcement + occasional Subscribe burst pattern.

EmbassyNetFactory

pub struct EmbassyNetFactory<'pool, D, POOL, RX_BUF, TX_BUF> generic over the embassy-net Driver and the pool dimensions.
impl TransportFactory only for 'pool = 'static (the trait needs F::Socket: 'static); an unsafe lifetime lift in bind() carries the pool reference into the socket. SAFETY argument: the lift is identity at the impl-bound 'static, and the per-slot CAS handshake gives the same exclusion guarantees as a Mutex would.
bind() returns Err(TransportError::AddressInUse) on pool exhaustion (closest existing variant; a future TransportError::PoolExhausted would be a small additive change).

EmbassyNetSocket

Wraps UdpSocket<'static> with the slot index + a &'static dyn SlotReclaim for free-list release on Drop.
The SlotReclaim trait erases the pool's three const generics from the socket type signature, keeping EmbassyNetSocket declaration-clean.
Drop calls inner.close() (releases the smoltcp slot) and then reclaim.release(slot_index).

TransportSocket impl (stub)

The TransportFactory::Socket: TransportSocket bound forces a trait impl on EmbassyNetSocket for the factory to typecheck. 19b ships a minimum-viable stub:

send_to / recv_from futures resolve to Err(TransportError::Unsupported) (real poll_send_to / poll_recv_from-driven named futures land in 19c).
local_addr returns the bind-time SocketAddrV4.
join_multicast_v4 / leave_multicast_v4 return Ok(()) because embassy-net's multicast-group join lives on Stack (async) — the user is expected to call stack.join_multicast_group(...) before constructing the factory. Documented prominently on EmbassyNetFactory.

Until 19c lands, attempting actual I/O through a bound socket fails with Unsupported. The 19b commit verifies the pool-claim / pool-release / Drop wiring without requiring the full embassy-net I/O bring-up.

Verification:
cargo build -p simple-someip-embassy-net ✓
cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf ✓
cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic ✓
cargo fmt --all --check ✓

Real implementation of `TransportFactory` over embassy-net 0.4. The adapter now claims a buffer slot from a caller-declared `&'static SocketPool<POOL, RX_BUF, TX_BUF>` on each `bind()`, constructs an `embassy_net::udp::UdpSocket` borrowing the slot's RX/TX buffers, and reclaims the slot when the returned `EmbassyNetSocket` drops. ## SocketPool - `pub struct SocketPool<const POOL: usize, const RX_BUF: usize, const TX_BUF: usize>` - Holds `[Slot<RX_BUF, TX_BUF>; POOL]` of `UnsafeCell`-wrapped buffers + `[AtomicBool; POOL]` of in-use flags. - `const fn new()` so the pool can live in a plain `static` declaration. - `unsafe impl Sync` justified by the AcqRel CAS handshake serializing per-slot UnsafeCell access. - 4-entry `PacketMetadata` arrays per direction (constant `PACKET_METADATA_LEN = 4`) — sized for SOME/IP-SD's announcement + occasional Subscribe burst pattern. ## EmbassyNetFactory - `pub struct EmbassyNetFactory<'pool, D, POOL, RX_BUF, TX_BUF>` generic over the embassy-net `Driver` and the pool dimensions. - `impl TransportFactory` only for `'pool = 'static` (the trait needs `F::Socket: 'static`); an unsafe lifetime lift in `bind()` carries the pool reference into the socket. SAFETY argument: the lift is identity at the impl-bound `'static`, and the per-slot CAS handshake gives the same exclusion guarantees as a Mutex would. - `bind()` returns `Err(TransportError::AddressInUse)` on pool exhaustion (closest existing variant; a future `TransportError::PoolExhausted` would be a small additive change). ## EmbassyNetSocket - Wraps `UdpSocket<'static>` with the slot index + a `&'static dyn SlotReclaim` for free-list release on `Drop`. - The `SlotReclaim` trait erases the pool's three const generics from the socket type signature, keeping `EmbassyNetSocket` declaration-clean. - `Drop` calls `inner.close()` (releases the smoltcp slot) and then `reclaim.release(slot_index)`. ## TransportSocket impl (stub) The `TransportFactory::Socket: TransportSocket` bound forces a trait impl on `EmbassyNetSocket` for the factory to typecheck. 19b ships a minimum-viable stub: - `send_to` / `recv_from` futures resolve to `Err(TransportError::Unsupported)` (real `poll_send_to` / `poll_recv_from`-driven named futures land in 19c). - `local_addr` returns the bind-time SocketAddrV4. - `join_multicast_v4` / `leave_multicast_v4` return `Ok(())` because embassy-net's multicast-group join lives on `Stack` (async) — the user is expected to call `stack.join_multicast_group(...)` before constructing the factory. Documented prominently on `EmbassyNetFactory`. Until 19c lands, attempting actual I/O through a bound socket fails with `Unsupported`. The 19b commit verifies the pool-claim / pool-release / Drop wiring without requiring the full embassy-net I/O bring-up. Verification: cargo build -p simple-someip-embassy-net ✓ cargo build -p simple-someip-embassy-net --target thumbv7em-none-eabihf ✓ cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic ✓ cargo fmt --all --check ✓ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Implements a real embassy-net 0.4 transport backend for simple-someip by introducing a static SocketPool for UDP buffer storage and a TransportFactory that binds UdpSockets by claiming/reclaiming pool slots (with drop-based reclamation via EmbassyNetSocket).

Changes:

Added SocketPool with fixed-capacity RX/TX buffers + metadata and atomic slot-claim/release.
Implemented EmbassyNetFactory as a TransportFactory that binds sockets using pool slots and reclaims on drop.
Expanded EmbassyNetSocket to wrap UdpSocket<'static> and added a stub TransportSocket impl (I/O returns Unsupported for now).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
simple-someip-embassy-net/src/socket.rs	Defines `EmbassyNetSocket` + drop-based slot reclamation and a stub `TransportSocket` impl.
simple-someip-embassy-net/src/factory.rs	Adds `SocketPool` storage + atomic slot management and implements `EmbassyNetFactory::bind()` over embassy-net UDP.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T12:54:42Z

+    fn local_addr(&self) -> Result<SocketAddrV4, TransportError> {
+        Ok(self.local)
+    }


local_addr() currently just returns the local field recorded at bind time. If the caller binds with port 0 (ephemeral), this will keep reporting port 0, which violates TransportSocket::local_addr’s contract (used to discover the assigned ephemeral port). Consider reading back the actual bound port from the underlying UdpSocket after bind() (or computing it lazily in local_addr()) and storing/returning that instead.

Copilot · 2026-04-29T12:54:43Z

+        // 3. bind() to the requested port. Port 0 means
+        //    "ephemeral, let the stack pick" — embassy-net
+        //    interprets bind on a `port: 0` IpListenEndpoint as
+        //    "any port". The actual local addr is read back via
+        //    EmbassyNetSocket::local_addr.
+        if let Err(_e) = socket.bind(addr.port()) {
+            // Bind failed. Release the slot so it doesn't leak.


bind() ignores addr.ip() and binds only by port (socket.bind(addr.port())), but later records local with the caller-provided IP. This can misrepresent what address the socket is actually bound to (and can break server code paths that bind to a specific interface IP). If embassy-net cannot bind to a specific IPv4 address, consider rejecting non-Ipv4Addr::UNSPECIFIED binds with TransportError::Unsupported (or otherwise document/enforce the constraint), or use an embassy-net API that accepts an address-aware listen endpoint if available.

Copilot · 2026-04-29T12:54:43Z

+        //    const generics by coercing &'static SocketPool<...>
+        //    to &'static dyn SlotReclaim — the socket only ever
+        //    needs to call `release(slot_index)` on drop.
+        //
+        // SAFETY: see the lifetime-erasure note above.
+        let pool_dyn: &'static dyn SlotReclaim = unsafe {
+            // Lift `self.pool: &SocketPool<...>` from `'pool` to
+            // `'static`. The `impl<...> for EmbassyNetFactory<'static, ...>`
+            // bound above guarantees the factory we're being called
+            // through has a `'static` pool reference, so the lift
+            // is identity.
+            core::mem::transmute::<
+                &SocketPool<POOL, RX_BUF, TX_BUF>,
+                &'static SocketPool<POOL, RX_BUF, TX_BUF>,
+            >(self.pool)
+        };


The transmute used to lift self.pool to 'static is unnecessary in this impl: TransportFactory is implemented for EmbassyNetFactory<'static, ...>, so self.pool is already &'static SocketPool<...>. Removing the unsafe here would simplify the safety story and avoid relying on a brittle lifetime cast.

Suggested change

// const generics by coercing &'static SocketPool<...>

// to &'static dyn SlotReclaim — the socket only ever

// needs to call `release(slot_index)` on drop.

//

// SAFETY: see the lifetime-erasure note above.

let pool_dyn: &'static dyn SlotReclaim = unsafe {

// Lift `self.pool: &SocketPool<...>` from `'pool` to

// `'static`. The `impl<...> for EmbassyNetFactory<'static, ...>`

// bound above guarantees the factory we're being called

// through has a `'static` pool reference, so the lift

// is identity.

core::mem::transmute::<

&SocketPool<POOL, RX_BUF, TX_BUF>,

&'static SocketPool<POOL, RX_BUF, TX_BUF>,

>(self.pool)

};

// const generics by coercing the already-`'static`

// `self.pool` reference to `&'static dyn SlotReclaim` —

// the socket only ever needs to call

// `release(slot_index)` on drop.

let pool_dyn: &'static dyn SlotReclaim = self.pool;

Copilot · 2026-04-29T12:54:43Z

+pub struct EmbassyNetBindFuture {
+    inner: Option<Result<EmbassyNetSocket, TransportError>>,
+}
+
+impl Future for EmbassyNetBindFuture {
+    type Output = Result<EmbassyNetSocket, TransportError>;
+
+    fn poll(
+        mut self: core::pin::Pin<&mut Self>,
+        _cx: &mut core::task::Context<'_>,
+    ) -> core::task::Poll<Self::Output> {
+        match self.inner.take() {
+            Some(result) => core::task::Poll::Ready(result),
+            None => panic!("EmbassyNetBindFuture polled after completion"),
+        }
+    }


EmbassyNetBindFuture::poll panics if polled after completion. While many executors won’t do this in practice, it makes the future less robust and harder to compose. Since bind() is synchronous here, consider using core::future::Ready directly as the associated BindFuture, or make repeated polls return Poll::Ready again instead of panicking.

Copilot · 2026-04-29T12:54:44Z

 impl<const POOL: usize, const RX_BUF: usize, const TX_BUF: usize> SocketPool<POOL, RX_BUF, TX_BUF> {
-    /// Construct an empty socket pool. Const so it can live in a
-    /// `static`.
+    /// Construct an empty socket pool. `const`, so the pool can live
+    /// in a plain `static` declaration in firmware boot code.
    #[must_use]
    pub const fn new() -> Self {
-        Self { _todo: () }
+        // `[const { ... }; N]` lets us const-init both arrays
+        // without spelling out N copies.
+        Self {
+            slots: [const { Slot::new() }; POOL],
+            in_use: [const { AtomicBool::new(false) }; POOL],
+        }
+    }
+
+    /// Try to claim a free slot. Returns the slot index on success.
+    fn claim(&self) -> Option<usize> {
+        for (i, flag) in self.in_use.iter().enumerate() {
+            if flag
+                .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire)
+                .is_ok()
+            {
+                return Some(i);
+            }
+        }
+        None
    }
 }


There are no unit tests covering the new SocketPool claim/release logic and the Drop-based reclamation path. Since this logic is core to preventing buffer-slot leaks/exhaustion, consider adding host-side tests (they can exercise SocketPool::claim/SlotReclaim::release behavior without needing embassy-net I/O) to lock in the concurrency/edge-case invariants.

JustinKovacich requested a review from Copilot April 29, 2026 12:49

Copilot started reviewing on behalf of JustinKovacich April 29, 2026 12:50 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

phase 19b: EmbassyNetFactory + SocketPool storage#99

phase 19b: EmbassyNetFactory + SocketPool storage#99
JustinKovacich wants to merge 1 commit into
feature/phase19a_embassy_net_scaffoldfrom
feature/phase19b_embassy_net_factory

JustinKovacich commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Copilot AI Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

JustinKovacich commented Apr 29, 2026

SocketPool

EmbassyNetFactory

EmbassyNetSocket

TransportSocket impl (stub)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants