Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared NAL #43

Closed
jonahbron opened this issue Jan 18, 2021 · 5 comments · Fixed by #46
Closed

Shared NAL #43

jonahbron opened this issue Jan 18, 2021 · 5 comments · Fixed by #46

Comments

@jonahbron
Copy link

Once #42 is merged, the NAL traits in this crate will no-longer be sharable on their own. It is necessary to create a new layer that can handle the sharing internally.

This can be split into two different types of sharing: single-thread (where RefCell can be used), and multi-threaded. Both can draw inspiration from Rahix/shared-bus.

I've drafted a simple RefCell-based wrapper here, inspired by the shared-bus Simple Manager. It's suitable for sharing access to the network stack in a single-threaded context. However @ryan-summers you mentioned that you think there may be issues with this approach? Would you mind clarifying?

More work will be necessary to create a sharing layer that's either Sync or Send.

use core::cell::RefCell;
use embedded_nal::{nb, SocketAddr, TcpClientStack, TcpFullStack, UdpClientStack, UdpFullStack};

pub struct SharedNalManager<T> {
    driver: RefCell<T>,
}

impl<T> SharedNalManager<T> {
    pub fn new(driver: T) -> Self {
        SharedNalManager {
            driver: RefCell::new(driver),
        }
    }

    pub fn acquire(&self) -> SharedNal<T> {
        SharedNal {
            driver: &self.driver,
        }
    }
}

pub struct SharedNal<'a, T> {
    driver: &'a RefCell<T>,
}

macro_rules! forward {
    ($func:ident($($v:ident: $IT:ty),*) -> $T:ty) => {
        fn $func(&mut self, $($v: $IT),*) -> $T {
            self.driver.borrow_mut().$func($($v),*)
        }
    }
}

impl<'a, T: UdpClientStack> UdpClientStack for SharedNal<'a, T> {
    type Error = T::Error;
    type UdpSocket = T::UdpSocket;

    forward! {socket() -> Result<Self::UdpSocket, Self::Error>}
    forward! {connect(socket: &mut Self::UdpSocket, address: SocketAddr) -> Result<(), Self::Error>}
    forward! {send(socket: &mut Self::UdpSocket, data: &[u8]) -> Result<(), nb::Error<<T as embedded_nal::UdpClientStack>::Error>>}
    forward! {receive(socket: &mut Self::UdpSocket, data: &mut [u8]) -> Result<(usize, SocketAddr), nb::Error<<T as UdpClientStack>::Error>>}
    forward! {close(socket: Self::UdpSocket) -> Result<(), Self::Error>}
}

impl<'a, T: UdpFullStack> UdpFullStack for SharedNal<'a, T> {
    forward! {bind(socket: &mut Self::UdpSocket, local_port: u16) -> Result<(), Self::Error>}
    forward! {send_to(socket: &mut Self::UdpSocket, remote: SocketAddr, buffer: &[u8]) -> Result<(), nb::Error<<T as UdpClientStack>::Error>>}
}

impl<'a, T: TcpClientStack> TcpClientStack for SharedNal<'a, T> {
    type TcpSocket = T::TcpSocket;
    type Error = T::Error;

    forward! {socket() -> Result<Self::TcpSocket, Self::Error>}
    forward! {connect(socket: &mut Self::TcpSocket, address: SocketAddr) -> Result<(), nb::Error<<T as TcpClientStack>::Error>>}
    forward! {send(socket: &mut Self::TcpSocket, data: &[u8]) -> Result<usize, nb::Error<<T as embedded_nal::TcpClientStack>::Error>>}
    forward! {receive(socket: &mut Self::TcpSocket, data: &mut [u8]) -> Result<usize, nb::Error<<T as TcpClientStack>::Error>>}
    forward! {is_connected(socket: &Self::TcpSocket) -> Result<bool, Self::Error>}
    forward! {close(socket: Self::TcpSocket) -> Result<(), Self::Error>}
}

impl<'a, T: TcpFullStack> TcpFullStack for SharedNal<'a, T> {
    forward! {bind(socket: &mut Self::TcpSocket, port: u16) -> Result<(), <T as TcpClientStack>::Error>}
    forward! {listen(socket: &mut Self::TcpSocket) -> Result<(), <T as TcpClientStack>::Error>}
    forward! {accept(socket: &mut Self::TcpSocket) -> Result<(<T as TcpClientStack>::TcpSocket, SocketAddr), nb::Error<<T as TcpClientStack>::Error>>}
}
@ryan-summers
Copy link
Member

My comments were only related to sharing the stack in multiple contexts (e.g. interrupt priorities), however that would also necessitate the usage of a mutex similar to shared-bus.

For a single-threaded/single-context, this approach should be fine.

@jonahbron
Copy link
Author

How do folks feel about this wrapper existing in this crate vs another one? It can even be in this same repo but be published as a separate crate. A common pattern I've picked up on is separating trait crates from implementation crates, and this is an implementation. But it's so closely related, I wouldn't strongly object to it being published in parallel.

@ryan-summers
Copy link
Member

I'm personally okay with having it implemented in this crate, which will give it some level of "officialness" as well. That's one issue I've always had with shared-bus - it seems like it really should be part of embedded-hal IMO.

@MathiasKoch
Copy link
Collaborator

I share @ryan-summers point of view on this one. I feel like the goal is to put usefull blanket implementations like this into focus, rather than hiding them away for people to find themselves.

@eldruin
Copy link
Member

eldruin commented Jan 20, 2021

I agree with @ryan-summers and @MathiasKoch. I think that would be the easiest for now. It would always be possible to spin it off in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants