[embassy-net]: Mutability mismatch between embassy_net_driver::RxToken::consume() and smoltcp::phy::RxToken::consume()

[AnthonyGrondin]

smoltcp::phy::RxToken::consume() was updated in:

• Change mutability of RxToken's consume argument. smoltcp-rs/smoltcp#924

To change the mutability of the callback. The equivalent in embassy-net-driver hasn't been updated, creating a mismatch between the two traits. If an implementer wants to use the same callback for implementing embassy-net-driver and smoltcp, they cannot because the mutability doesn't match anymore.

[Dirbaio]

If an implementer wants to use the same callback for implementing embassy-net-driver and smoltcp, they cannot because the mutability doesn't match anymore.

can you expand on this? it seems to me you can make the callback require &mut, and for implementing smoltcp's you just coerce &mut to &.

EDIT: okay should've read the PR in the pingback. I've commented there.

[AnthonyGrondin]

Thankfully, there is a trivial solution provided for this issue. I'll explain it here for future reference; Basically, in esp-wifi, we implement the traits for both embassy_net_driver::RxToken::consume() and smoltcp::phy::RxToken::consume(). Since it's the same behaviour for both, it can be simplified with a callback. One can use |t| f(t) to let rust coerce the type for a F: FnOnce(&mut [u8]) -> R callback.

Example:

impl<Dm: WifiDeviceMode> embassy_net_driver::RxToken for WifiRxToken<Dm> {
        fn consume<R, F>(self, f: F) -> R
        where
            F: FnOnce(&mut [u8]) -> R,
        {
            self.consume_token(f)
        }
    }


#[cfg(feature = "smoltcp")]
impl<Dm: Sealed> smoltcp::phy::RxToken for WifiRxToken<Dm> {
    fn consume<R, F>(self, f: F) -> R
    where
        F: FnOnce(&[u8]) -> R,
    {
        self.consume_token(|t| f(t))
    }
}

impl<Dm: Sealed> WifiRxToken<Dm> {
    /// Consumes the RX token and applies the callback function to the received
    /// data buffer.
    pub fn consume_token<R, F>(self, f: F) -> R
    where
        F: FnOnce(&mut [u8]) -> R,
    { 
    // ...
     }
}

That being said, there is still a mismatch between the two APIs and the question is, does the trait provided by embassy_net_driver deserves the same treatment as the one from smoltcp and get its mutability changed?

If so, I'll leave this issue open for addressing it.

[Dirbaio]

the question is, does the trait provided by embassy_net_driver deserves the same treatment as the one from smoltcp and get its mutability changed?

Being consistent would be nice, but it would be a breaking change for embassy-net-driver which is somewhat disruptive, so IMO it isn't worth it.

(embassy-net-driver is a separate crate from embassy-net because it prevents needing updates to all drivers when a new embassy-net release comes out. For example embassy-net 0.4 uses embassy-net-driver 0.2, which esp-wifi implements. Then embassy-net 0.5 still uses embassy-net-driver 0.2, which made it compatible out of the box with esp-wifi, instantly the moment it was released, without requiring any update to esp-wifi, which is quite nice)

[AnthonyGrondin]

The same breaking change argument could've been made for smoltcp (see esp-wifi upgrade), but I understand that we should avoid breaking changes as much as possible. I strongly dislike them myself too.

I'll close this issue as the discussion provided an easy and trivial fix for this specific edge case.

[Dirbaio]

The same breaking change argument could've been made for smoltcp

smoltcp doesn't split the traits into a separate crate, so it'd have been breaking anyway even if the trait didn't change.