Giter Site home page Giter Site logo

embedded-nal's Introduction

embedded-nal

An Embedded Network Abstraction Layer

This crate defines a simple set of traits that can be implemented by almost any TCP/IP stack. This might be an on-chip stack like smoltcp, or it could be an off-chip TCP/IP stack on an AT modem.

How-to: add a new trait

This is the suggested approach to adding a new trait to embedded-nal

Research / Discussion

Ideally, before proposing a new trait, or set of traits, you should check for an existing issue suggesting the need for the trait, as well as any related works / use cases / requirements that are useful to consider in the design of the trait.

These issues will be labeled as discussion in the issue tracker.

Implementation / Demonstration

Proposed traits should then be implemented and demonstrated, either by forking embedded-nal or by creating a new crate with the intent of integrating this into embedded-nal once the traits have stabilized. You may find cargo workspaces and patch useful for the forking approach.

Proposing a trait

Once the trait has been demonstrated a PR should be opened to merge the new trait(s) into embedded-nal. This should include a link to the previous discussion issue.

If there is determined to be more than one alternative then there should be further discussion to try to single out the best option. Once there is consensus this will be merged into the embedded-nal repository.

These issues / PRs will be labeled as proposals in the issue tracker.

Minimum Supported Rust Version (MSRV)

This crate is guaranteed to compile on stable Rust 1.77.0 and up. It might compile with older versions but that may change in any new patch release.

License

Licensed under either of

at your option.

Contribution

Unless you explicitly state otherwise, any contribution intentionally submitted for inclusion in the work by you, as defined in the Apache-2.0 license, shall be dual licensed as above, without any additional terms or conditions.

embedded-nal's People

Contributors

chrysn avatar dirbaio avatar dnadlinger avatar eldruin avatar jonahd-g avatar lulf avatar mathiaskoch avatar newam avatar rmja avatar ryan-summers avatar sympatron avatar thejpster avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

embedded-nal's Issues

TCP socket state/error handling insufficiently specified

The correspondence of the TcpClientStack API to TCP socket states is currently not specified clearly enough to be able to write robust TCP client applications against the generic embedded-nal interface. For instance:

  • What does is_connected mean? That connect has been called on the socket? That the socket is ESTABLISHED in TCP terms? That there might still be data to receive()? (where the socket could already be in CLOSE-WAIT, but still with data in the receive buffer)
  • What does connect() returning Ok() instead of WouldBlock mean? is_connected() == true immediately afterwards?
  • What happens if the socket is closed gracefully on the remote side? Does receive() return Some(0) like in the Berkeley sockets API? Or does it return some sort of error? If the latter, how does a client application differentiate between that and unexpected errors?

This is complexity inherent to the problem domain, and needs to be (carefully) exposed in any interface abstracting TCP sockets. Otherwise, it is simply not possible to write robust applications using embedded-nal. For instance, even in the common case where an embedded device just wants to try reconnecting to some server when there are some sort of connection problems, without any care for the exact state of the previous connection, is it okay to try and connect the socket again after is_connected returns false, or is a new socket() necessary?

For an example of a library that has (save for a few typos) done a decent job at this, see https://docs.rs/smoltcp/0.7.0/smoltcp/socket/struct.TcpSocket.html.

need an object that can impl uWrite

I was refactoring some code to use embedded-nal instead of a custom driver and found that embedded-nal does not have any implementation of ufmt::uWrite.

So I created a struct StackAndSocket which combines mutable references to the TcpClientStack and TcpSocket so that it can impl uWrite.

mutantbob@e4a65f4

It is not meant to exist for a long time; you just create it for a quick burst of activity, and then drop it. I use it like this:

        if let Ok(client) = client.as_mut() {
            buffer.flush(&mut ethernet.with_socket(client), spi)
        } else {
            buffer.discard()
        }

I suspect this idea has overlap with #53 ,

Managing failed operations

For specific operations, errors are an expected use case, but specific errors cannot be detected by the user of the NAL trait.

Consider the following case where a user is attempting to connect a TCP socket. Possible errors in the operation are:

  1. Communication failure with an external TCP/IP stack (e.g. NACK over I2C, invalid data over SPI, etc)
  2. TCP received a RST during the sync process
  3. ARP failed to resolve the requested address
  4. TCP failed to receive a SYN-ACK in a specified timeout

In these cases, the first error is likely fatal and a user of a TcpStack would want to propagate this error to the user application. However, the following 3 errors (network-based) are an error that should not be propagated to the user application. Instead, it's likely that the requested device is not present on the network and the user of the TcpStack will want to re-attempt to connect() some time in the future.

I believe the solution here is to provide a non-exhaustive Error enum in the embedded-nal and allow device drivers to report those instead. This allows a user of the TcpStack to use known error types for detecting specific types of errors (e.g. recoverable vs fatal conditions).

TCP: receive_exact?

It might be helpful to have a receive_exact method, similar to stdio's read_exact. That method would block if some (but not enough) bytes are available to be read.

Why

Such a method would make it easy to implement TCP based message exchange protocols that have some kind of initial length description. Per connection, the application would only need to store the previously read header (indicating the length, and possibly the type), and could then wait for the rest to arrive. Per-connection state would be size of a Stack::TcpSocket plus a few bytes -- rather than having to keep a maximally sized buffer around per connection.

Why not

Providing receive_exact pushes the onus of buffering that data to the network stack. Most can probably[1] provide a function like that easily, but what is the consequence of programs actually exercising it?

If 4 connections are open, each receive simultaneously a short TCP segment that indicates that 1KiB of data is coming up, then a stack with 2KiB network buffer is in no position to serve any of them, and all starve.

Then again, such locking situations can probably also come up without this already, eg. if an application sees traffic on one stack, decides to plunge in there, only polls that socket until the message is there, but that never arrives because the other peers were a tad faster and filled up the network stack's buffers that now don't get drained b/c the application is set on processing this connection first.

[1]: I don't know sufficiently many well enough to really say.

Alternatives

A function to get the number of pending bytes, or a pair to peek at data but only acknowledge it separately, would have the same positive and negative effects. On some stacks (eg. on std), only implementing receive_exact (but not peeking or count-peeking) would be implementable easily.

This doesn't really change when async (#47) is added: Sure in async one can just (stack or slab) allocate a receive buffer and read out piecemeal, but depending on how the corresponding tasks are allocated, this brings us either back into having a full receive buffer for every connection of "Why" (if there's a task per connection and all tasks are fully allocated), or into the commit-and-starve situation of "Why not" (because there's a large task running and the others can't be started until that one is done).

Steps forward

I'd hope for some discussion here informed by different implementers' experiences before the technical points (bump the trait? Make it runtime-fallible with a default impl that says it doesn't work? Add TcpClientStackWithReadExact?) need to be addressed.

[Question] Closure-based API for zero-copy

The current read function of of the traits takes in a buffer, requiring an additional buffer to be allocated

Would it make sense to add something along the lines of

fn read_with<F>(&self, socket: &mut Self::TcpSocket, f: F) -> nb::Result<usize, Self::Error>
where
    F: FnOnce(&[u8]) -> usize,

Making it easier to use with socket buffers of the kind used in smoltcp, without the additional clone? https://github.com/smoltcp-rs/smoltcp/blob/bdfa44270e9c59b3095b555cdf14601f7dc27794/src/socket/tcp.rs#L752

SocketAddr associated types

Implementing embeded-hal(-async), it seems I'm always converting SocketAddr back and forth with the platform's own socket addr type.

Would it make sense to add an associated SocketAddr type (probably constrained to Into<SocketAddr> + TryFrom<SocketAddr, Error=Unavailable> + PartialEq<Self::SocketAddr> + Hash)? In all places where we previously pass SocketAddr, we'd now pass Self::SocketAddr. Wherever users store addresses, they'd use their stack's SocketAddr type, and only convert through a SocketAddr when entering or leaving the stack (or even not at all if we require Display and FromStr).

Some reasons why a stack would not like working with SocketAddr all the time:

  • It is built on something where Rust's SocketAddr is just not the native format (eg. any non-Rust network stacks), so it needs to copy fields around all the time. (This is the case with RIOT OS)
  • It is an IPv6 only stack that doesn't constantly want to deal with erring out on IPv4 addresses.
  • It is an IPv4/6 dual stack that internally represents IPv4 addresses as IPv4-mapped addresses.
  • It has only a single network interface and thus doesn't keep a zone identifier around. (In RIOT, there are some optimizations for what is called "highlander mode" when there can be only one).

This is mainly a concern for users of unconnected UDP sockets, where recvfrom-style use is predominant, and addresses are handled in every single packet. For TCP or connected UDP sockets, it's probably a secondary concern.

New release of `embedded-nal`

It seems that there is small discrepancy between the embedded-nal & embedded-nal-async crates which causes some issues for me. embedded-nal current release has dependency on no-std-net 0.5 but -async has on 0.6.

Even though no-std-net hasn't had any release in a while and is not actively maintained, there is a small difference and with the new support for ip_in_core it seems fitting to make a new release of embedded-nal.

`Dns` traits force caching in-flight DNS requests

Currently, because the Dns resolver functions return an nb::Result, this means that some higher-level has to actually cache the the hostname being looked up during repeated calls to resolve the name. However, this isn't really a great API because it enforces caching and RAM usage if the hostnames are long. Thus, if an implementation wants to support N simultanenous queries of 255 bytes, it must cache at least 255 * N bytes locally.

Ideally, we would refactor the functions to be get_host_by_name() -> Handle, where Handle is some unique ID that can be queried against in the future (and can ideally be really lightweight, like a u32).

So the API may be:

trait DnsLookup {
    type Handle;
    fn start_query(&mut self, hostname: &str, typ: QueryType) -> Result<Handle, Self::Error>;
    fn check_query(&mut self, handle: &Handle) -> Result<IpAddr, Self::Error>;
    fn cancel_query(&mut self, handle: Handle);
}

Also, it seems like get_host_by_ip() is a bit of a different DNS use-case and isn't super well-suited for embedded (I just mark it as unimplemented!() in my case because smoltcp doesn't support it). Perhaps it would be best to move this to a separate trait.

Evaluate effectiveness of DNS traits

It is currently my understanding that DNS can be implemented on top of generic sockets. As such, it should be possible to write a generic no_std DNS client crate that relies on the embedded-nal. This would mean that the dns traits in this crate could likely be removed, since they no longer need to be generic.

I'm curious to hear other opinions here - I have not yet written a DNS client, but likely will be doing so in the near future (and would publish it as said no_std DNS resolver)

[Question] No way to set UDP port?

I see that SocketAddr supports an IP address and a remote port number. However, I see no way to specify a local port number. How would a program specify the port to listen on/send from without that ability?

How to implement RAII / Drop for Sockets?

I have been working on a modem driver for a Quectel BG77 which is a modem that is controlled via AT commands over a UART. The network stack is implemented on the modem and the modem supports multiple sockets.

To me, it seems obvious to keep track of acquired sockets in a list and recycling/returning those as soon as TcpClientStack::close is called. Unfortunately, it seems to be impossible to guarantee that the close-method gets called at all. It is easy to imagine cases which will cause socket exhaustion in the long run:

let mut socket = tcp_stack.socket()?;
function_call_that_can_fail()?; // e.g. socket.send(...), function returns here, socket is leaked
tcp_stack.close(socket);

Normally, this would be tackled with RAII, i.e. implementing the Drop trait for the socket. But as far as I see, this is impossible (at least with the current trait definitions) because we need the &mut TcpStack to close the socket.

In the beginning, I thought that this could be solved with interior mutability. But to implement that, the socket type needs to contain a &RefCell<TcpClientStack> (which still is a reference to the TcpClientStack). First, I could not get this to compile at all. The closest I could get was type TcpSocket<'a> = Bg77TcpSocket<'a, ...>; which fails with "generic associated types are unstable". Secondly, regarding the lifetimes, the socket would borrow the tcp stack in TcpClientStack::socket(&mut self). So the tcp stack would be borrowed until the socket is destroyed which limits us to a single usable socket.

Personally, I would really like to implement this feature because not doing so brings us back to C-style manual resource management with all its downsides (even worse, because the ?-operator is harder to spot than a return).

The solution will most probably be based on interior mutability because the (possibly multiple) sockets require (possibly mutable) access to the underlying hardware. I have seen discussions here that want to save the driver developers from re-inventing this interior mutability wheel over and over again, so it would be nice to have a solution which is generic over different driver implementations I guess. So what are your thoughts and ideas on this?

Notes for implementer not visible on docs.rs

The "notes for implementers" in the async version's udp.rs

//! Traits for using UDP on embedded devices
//!
//! ## Notes for implementers
//!
//! * At several places, the APIs expect to provide a local address. Backends that can not obtain
//! it, such as some AT-command based stacks, <!-- should question whether they may really call
//! themselves UDP and --> may pretend to have performed some form of network address
//! translation, and present invalid addresses as the local address.
//!
//! * Implementing [`UdpStack::UniquelyBound`] and [`UdpStack::MultiplyBound`] unconnected sockets
//! separately allows discarding the local addresses in the bound case. With LTO enabled, all the
//! overhead compared with a third trait variant between [ConnectedUdp] and [UnconnectedUdp] (in
//! which the local address is static but the remote address is flexible) should optimized out.
//! Implementing `UniquelyBound` and `MultiplyBound` with the same type is expected to be a
//! common choice.

do not appear on docs.rs as from what I see.

If you let me know where it is supposed to appear, I could try fixing it and make a PR. Supposedly it should appear at the top of each trait's page, i.e., be moved below this line:

/// address at connect time.

[Suggestion] Ownership-based API for zero-copy

Considering expressing RIOT's network API (in particular GNRC; with their sockets it'd be different) using embedded-nal, their API could profit (read: avoid copies) from an ownership based model.

This suggestion is similar to #12, but while there things are about stream-based sockets, this is for datagram sockets. Like there, this might need an additional trait to implement (which could also be implemented by an alloc- or fixed-buffer backed wrapper for cases where a consumer needs it).

Rough API sketch:

trait OwnedReadable: UdpStack {
    // Anything more scatter-gather-y would make it easier to implement on
    // ring-buffer backed stacks, but harder to consume.
    type UdpMessage: AsRef<[u8]>;
    fn read_owned(&self, socket: &mut Self::UdpSocket) -> Result<Self::UdpMessage, nb::Error<Self::Error>>;
}

That would work well for the GNRC backend, which AFAICT guarantees contiguous allocation of inbound data. It'd also work well for the very small single-MTU stacks (which off my head I don't remember precisely; I think contiki worked like that). For backends like lwIP that don't guarantee contiguous allocation (because they put data across smaller slabs), it might be better to require something roughly like IntoIterator<Item=&[u8]>, which would then be expressed in its own trait anyway.

On the write side, I don't have that clear a suggestion, but I suppose combined usage would be a bit like

let received = block!(stack.receive(sock))?;
let extracted = parse(received);
drop(received);
let mut out = stack.write_owned_begin(sock, extracted.estimate_response_size())?; // might fail OOM
extracted.populate(&mut out); // might trim the response in the process
stack.write_owned(out);

Some applications might not be able to drop the received before starting the response; that's fully OK but uses more stack resources, so it'd be best practice to drop it ASAP to ensure that that works even on stacks that can have at most a single UDP message in flight.

[Question] Clarity on what Mode affects

I'd like some additional clarity on what Mode does, and how it affects the behavior of both read and write. In a Write operation, there are 2 possible, and several branching ways of handling it.

  1. Write, but device is busy and write operation is not possible
    a. return WouldBlock
    b. block (w/ optional timeout) until device is no-longer busy and write is possible
  2. Write can be done and executes
    a. execute command and return early, assuming write will be successful
    b. block (w/ optional timeout) until write is complete and successful

For Read, there are more scenarios

  1. Read, but no packets have been received
    a. return WouldBlock
    b. block (w/ optional timeout) until a packet arrives
  2. Read and a packet is available
    a. read the packet into the buffer

Which of these decisions are fixed in the spec, and which are controlled by the Mode enum? Note that there is no meaningful concept of "returning early" for the read operation.

Dealing with stacks that need pinning

Do we have a story for how to deal with stacks that need their sockets pinned?

For example, RIOT's sock_udp_t needs to stay in a place in memory from the time it is connected or bound -- aka be pinned.

The options I see are:

  1. Making all functions that now take &mut socket take a Pin<&mut socket> -- a breaking API change that'd require all general applications to pin their sockets (general applications that can't would require ``ST: UdpStack, ST::Socket: Unpin`).
  2. Using workarounds like currently in riot-wrappers where Stack upon creation has some pinned memory inside which the immovable sockets are created, and where the exposed sockets are just pointers to these.

Is 1. an option on the long run?

TLS options

Would it make sense to add a third trait to allow "upgrading" a TcpSocket to a TlsSocket somehow? Not sure on the format, but i guess it should add a way of setting certificates etc.

Proposal: embedded-nal-async

I'd like to propose following a similar path to embedded-hal-async and add an embedded-nal-async crate within this repo. I have been maintaining some async traits for TCP1 and DNS2 that I feel would be a good fit for this crate (assuming some adjustments to fit the existing embedded-nal conventions). I'm available to take on that effort.

Footnotes

  1. https://github.com/drogue-iot/drogue-device/blob/main/device/src/traits/tcp.rs โ†ฉ

  2. https://github.com/drogue-iot/drogue-device/blob/main/device/src/traits/dns.rs โ†ฉ

Wrong TcpStack.open definition, or comment?

Open a new TCP socket to the given address and port
I think either this comment, or the definition of TcpStack.open is wrong?

Should open actually contain address and port? Otherwise I am not quite sure what open should do?

UdpClient/UdpServer may be confusing to users

Looking at the whole UdpClient/UdpServer thing from #21 I think the idea behind the trait it kind of changed.

Before it was called UdpStack and represented the whole network interface that creates sockets on demand for new connections, but now the same suddenly a UdpServer that creates either UdpSockets in server or client mode depending on the function you call. I think it is kind of confusing that your network interface has to implement UdpServer now.
I have no definitive answer how to make the API clearer, but maybe the socket types should be different in client and server mode instead of the stack. The stack traits could be called UdpStack/UdpServerStack or something like UdpSimpleStack/UdpAdvancedStack or UdpStackCore/... Maybe somebody has a good idea for names.

The point I am trying to make is, that it may be confusing to mix semantics of network interfaces and their traits and actual sockets. Similar things apply to TCP (#30) of course.

bind-multiple is hard to imlement on smoltcp

I've split this out of #103 as reported by @ivmarkov to keep threads disentangled:

Supporting the bind_multiple contract on top of smoltcp seems currently impossible, or in the best case would require terrible hacks, as smoltcp does seem to assign a source (local) address very late - only when dispatching the UDP packet. The UDP packets in the ring buffer queue however do not have any notion of "source address" before that.

So it seems sending with an arbitrary local address would require us to a) wait for the UDP socket ring buffer to become empty (so that we are sure all other pending packets are sent with the "previous" local addr) b) unbind the UDP socket c) rebind the socket with the local address of the packet we want to send (NOTE: this way however we would be missing packets that we want to receive, as the receive_from part will now work with the local address that we want to use for sending the packet d) unbind the socket once the packet was sent e) rebind the socket again with the local address as specified in bind_multiple

What's also weird is that there is quite a bit of asymmetry wrt UDP sockets between embedded-nal and embedded-nal-async. embedded-nal is pretty simplistic, while embedded-nal-async seems to enforce an API which is too close to the BSD sockets one, and is not necessarily a good fit for smoltcp. (Not that implementing bind_multiple is particularly easy with STD either.)

I'm unsure how to proceed, honestly.

  • One option would be to just not support bind_multiple on top of smoltcp by having the impl always return an error.

  • Another would be to remove it and/or make it optional in embedded-nal-async.

In any case, not having an impl of the UDP socket traits for smoltcp is an odd situation to be in, given that smoltcp is the one stack used in embedded...

@chrysn @lulf @Dirbaio @MabezDev Sorry for pinging. Happy to listen to your feedback / comments / guidance.

Did I miss the elephant in the room?

Confusing TCP connection states

The current life of a TCP socket looks superfluously complex: It can be created ("open", although it's not really clear what is opened there), but only when it is connected it becomes useful. It can be closed again, but it is not clear from the documentation whether anything can be done with a closed socket other than being dropped. (Can a closed socket be connected again? From the types it could, but that wouldn't work on POSIX sockets AFAIK).

Wouldn't it be more straightforward to have a single awaitable (OK, nb::Result) client connecting function?

When TCP server functionality is added, that'd probably be centered around different types anyway (possibly a TcpServerSocket that has an accept method).

get_host_by_name: Clarify behavior w/rt AI_V4MAPPED and AI_ADDRCONFIG

There are some features in name resolution that are quite useful on large systems. I'll refer to them by their POSIX names for lack of better terminology:

  • AI_V4MAPPED: When using an IPv6 stack that can do IPv4 but hides that from the application (using the dual stack features of IPv6), a stack could return Ok(IpAddr::V6("::ffff:10.0.0.1")) to a request.
  • AI_ADDRCONFIG: When a stack knows that it has no addresses of a given family, it can suppress sending requests for the corresponding kind of DNS records. Thus, unusable results are filtered out in some situations.
  • AI_ADDRCONFIG-done-right1: Even when some addresses of a family are available and the host needs to send A and AAAA requests, it may turn out that it has no route to the addresses indicated. With AI_ADDRCONFIG-done-right, the lookup would filter those out (because the stack is in a way better position to do that than the recipient, who would effectively do trial-and-error, espeically with our APIs).

To me, from the docs, it is unclear from the documentation whether or not these are supposed to be done by get_host_by_name / whether support is optional or mandatory / how optional support is signaled.

My suggestions are those:

  • AI_V4MAPPED is behavior a stack may have. It is recommended that stacks be consistent, i.e. that they only work with ::ffff:v4 addresses, but that's hard to enforce.
  • AI_ADDRCONFIG and -done-right should IMO be recommended: the stack has the information, the user doesn't need it, and if a user does need to know the answers to DNS requests without the need to interact with them, chances are they're building some DNS program, and want to access a bunch of other details that this API doesn't expose either.
    If a stack can not determine whether or not they're routable at all, it may violate that recommendation, but then users of that stack need to be aware that once such software is deployed on v4-only clients that talk to v4+v6 servers, they'll be on their own handling resulting errors. (They can recover by using something like #58, or by retrying and hoping that the one address they get is selected randomly).
    Making it mandatory may be the better choice even.

Is that a line you could get behind, and could implement on your implementations of embedded-nal? If so, I can build all of this into a PR on the documentation.

Footnotes

  1. I'm aware that (modulo bugs in implementations), ADDRCONFIG is doing what it is supposed to do, but right now I don't have a better name for what I think it should do. โ†ฉ

UDP: send with source address

If a UDP server is bound to more than a single address (typically because it's bound to the unspecified address) and the system can have more than a single address at an interface (not uncommon at IPv6), then an unconnected UDP server's send function needs a function that allows setting the source address (basically to the .1 of the result of a receive call). Otherwise, the OS will fill in that information -- and if the peer is picky about addresses (as CoAP is), it may guess wrong. (For example, Linux will pick the most recent address).

Adding support for this in a fully reliable way would probably mean adding a trait with an ugly name (UdpServerMultiaddress::send_to_from?). A more compatible, softer but prone-to-silent-misbehavior way forward would be to introduce UdpSever::send_to_from with a default implementation that disregards the from address (which is an OK behavior in the may cases when there is only one legal value anyway). Ideally, that default implementation would go away with the next incompatible version bump. (We can't have Rust show deprecation warnings when relying on a default implementation, can we?)

I'm happy to do a PR for this (just can't do it right now), but it would be good to have some initial feedback first.

Shared NAL

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>>}
}

#use of connect() for tftp protocol

Hello!
I'm trying to write a tftp-client on the basis of embedded-nal UdpClientStack
According to tftp protocol, the client (randomly) chooses its own port, e.g. client:8080. Then the client sends a request to the server on port server:69. The server (randomly) chooses a port as well, e.g. server:8081, and sends an answer to the client on port client:8080. The whole transfer (read or write) will now use the two ports client:8080 and server:8081.
The problem is that according to embedded-nal (and its implementation std-embedded-nal) fn connect() has not the same functions as in std::UdpSocket. It not just connect the socket with remote address (as in std::UdpSocket), but creates a socket by binding with unspecified port and then connect with remote address. So, each time when we use embedded-nal::connect, we create a new socket which is connected with specified remote address. It seems impossible to connect the existing socket with another remote address according to embedded-nal... or i don't understand something :(((

Maybe for tftp-client it is necessary to use create socket without connection with the help of UdpFullStack, then to bind it with specified local port and send messages with the help of send_to()? Or maybe you may advice me a way to change the remote address of the socket according to UdpClientStack?
Thank you for answers

Support for asynchronous TCP connections

The current TcpStack trait doesn't facilitate well for a connect() operation that cannot be completed in a blocking manner. For example, a socket may have called connect(), but is_connected() may return false because the TCP handshake has not completed.

As such, it's required that an application repeatedly call connect() on a socket whenever is_connected() returns false to ensure that an active connection is being made. A single call to connect() may fail due to TCP state mismatch and other related problems within the network stack (see smoltcp-rs/smoltcp#367).

If the network stack isn't implemented to support repeatedly calling connect() on a socket that may already be connecting (but not yet connected), this can have unexpected consequences.

Is it possible for us to extend the TcpStack trait to either:

  1. include an is_connecting() API
  2. Modify the is_connected() API to return an enum indicating the state of the TCP connection (e.g. connecting, disconnected, or connected)
  3. Have connect() return a non-exhaustive error enumeration that allows us to specify if a socket is already in the connecting process

I'm curious to hear some other thoughts on this as well. I'm looking to use these traits for an embedded MQTT client library that's targeting multiple network stacks (e.g. an external "hardware" network stack on a W5500 and a smoltcp network stack for a different device).

I've tried to describe the problem I'm having in an issue for minimq, which relies on a TCP network stack, so there's further context attached there.

Restore feature parity in async TCP

With the embedded-nal-async 0.2.0 release and #70, the -async version of the traits lost feature parity with the sync/nb versions; in particular, missing points are

  • binding to ports, and
  • closing a connection.

For closing a connection, it may make sense to replace the current AT type Connection<'m>: embedded_io::asynch::Read<Error = Self::Error> + embedded_io::asynch::Write<Error = Self::Error> [...]; with a dedicated TcpConnection trait, which depends on Read and Write, but in addition requires implementation of a close method.

TCP: Typestate client and server sockets

For TCP, there are two kinds of sockets, those bound (on which accept makes sense), and those connected or accepted (on which read/write makes sense). These are the same type in POSIX systems, but completely different eg. in RIOT.

This is similar to #33 in methods, but different in goals (#33 is about when what can be called on the same sockets; feel free to close as duplicate if you disagree).

I suggest that the Socket associated type of TcpClientStack be split into a ConnectionSocket and a ListeningSocket, which can still be the same types on POSIX-style systems but distinct on others. (If #33 is followed, a ConnectionSocket would then be further split into its unconnected and its connected version, and the listening into its unbound, bound and listening versions).

Alternatively, sockets on such platforms need to be union-typed (enum Socket { Unspecified, ConnectionSocket(sock_t), ListeningSocket(sock_listener_t) }), and always check on runtime whether the right kind is used, which feels not very idiomatic.

Prepare 0.3.0 release

We need to prepare the repository for a 0.3.0 release and then publish it to crates.io so that we can get access to the new internal mutability.

Revisit on mutable self references

I don't understand the reason for immutable self references that clearly do mutate the state of self or require exclusive access to the underlying implementation or bus. I think this design decision only causes hacks to circumvent it, unnecessary overhead in the driver implementation (which is the opposite of 'zero cost'/'you only pay for what you need') and does not reflect the strategy of the rust ecosystem, such as embedded-hal, std-Read, std-Write, tokio-AsyncRead or tokio-AsyncWrite.

I don't follow the argument of @thejpster here as it seems more of an special case scenario and could be solved otherwise by implementing Clone on what seems to be an FD or raw-pointer(?) internally.

Feel free to convince me otherwise.

We stumbled over this in a discussion here where @jonahd-g (thanky you!) proposes an implementation of embedded-nal for the w5500 driver.

Add support for TCP server APIs

Currently, there is no means to expose a TCP server via the embedded-nal. For example, there is no means to listen() or bind() to a specific port or to accept() connections from clients.

Server operations are added for UDP in #21, but that PR does not address the TCP stack.

One approach would be to refactor TCP similar to UDP:

pub trait TcpClient {
    fn send();
    fn receive();
    fn connect();
    fn close();
}

pub trait TcpServer: TcpClient {
    fn listen();
    fn bind();
    fn accept();
}

This approach would work, but the downside is that the TCP server would be forced to implement the connect() function even when it may never be used by the server.

An alternative approach would be:

pub trait TcpCore {
    fn send();
    fn receive();
    fn close();
}

pub trait TcpClient: TcpCore {
    fn connect();
}

pub trait TcpServer: TcpCore {
    fn listen();
    fn bind();
    fn accept();
}

The downside of approach number 1 is that users may have to implement connect() for a TCP server when it is never used. The downside for approach number 2 is that users will have to implement 2 traits for a client or a server.

I'm curious to hear @jonahd-g and @MathiasKoch 's opinions on this one

Support trait for DNS

Would it make sense to add a support trait for DNS lookups?

Somthing along the lines of

pub trait Dns {
    /// The type returned when we have an error
    type Error: core::fmt::Debug;

    fn resolve(&self, hostname: &str) -> Result<IpAddr, Self::Error>;
}

AVR Applications using the embedded-nal crate fail to compile

If you create an AVR project using cargo generate --git https://github.com/Rahix/avr-hal-template.git and add a dependency on embedded-nal (v 0.6.0) compilation triggers an error

error: cannot find macro `llvm_asm` in this scope
   --> /home/thoth/.cargo/registry/src/github.com-1ecc6299db9ec823/critical-section-0.2.7/src/lib.rs:127:13
    |
127 |             llvm_asm!(
    |             ^^^^^^^^

This is because of a dependency chain
heapless ^0.7 -> atomic-polyfill 0.1.8 -> critical-section 0.2.7

When heapless resolves rust-embedded/heapless#312 it should be possible for embedded-nal to upgrade its dependency and work on modern Rust AVR.

Async/Await

Perhaps worth designing those APIs to be async/await friendly from the beginning? Soon everything in the Rust world will be asynchronous and embedded won't be the exception.

Consider migrating away from nb

Given embedded-nal's similarities to embedded-hal, it's probably worth following the migration from nb to the now available native Rust async features.

Issue at embedded-hal is rust-embedded/embedded-hal#172.

There's probably no immediate action needed in terms of code change in embedded-nal, but participating in the discussion there might help in getting a consistent and usable outcome across the ecosystem.

Add an example with TcpStream

I've been trying to implement the trait for a library I am working on, as a side project.
I'm struggling to make it work with std::net::TcpStream.

My main issue resides in the Error type within TcpClient (I am using the soon-to-be merged branch from Ryan.
The compiler wants to have a embedded_nal::nb::Error but this enum needs a type.
if I try to add a type, the compiler consider it to be the returned type and doesn't agree:

// with UnixError being a enum deriving Debug.
type Error = embedded_nal::nb::Error<UnixError>;
error[E0053]: method `connect` has an incompatible type for trait
  --> src/tests.rs:43:2
   |
43 |     fn connect(&self, socket: &mut Self::TcpSocket, remote: SocketAddr) -> Result<(), Self::Error> {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `embedded_nal::nb::Error`, found enum `tests::UnixError`
   |
   = note: expected fn pointer `fn(&tests::UnixTcpStack, &mut core::option::Option<tests::std::net::TcpStream>, embedded_nal::SocketAddr) -> core::result::Result<_, embedded_nal::nb::Error<embedded_nal::nb::Error<tests::UnixError>>>`
              found fn pointer `fn(&tests::UnixTcpStack, &mut core::option::Option<tests::std::net::TcpStream>, embedded_nal::SocketAddr) -> core::result::Result<_, embedded_nal::nb::Error<tests::UnixError>>`

I tried different things with no luck and I am not an experienced Rust developer...

Thanks in advance,
Cyril

Guidance on TcpConnect and self-referential structs

Hi!

I am using the async TcpConnect. I have a struct that owns both the connector, and at most one socket. This looks a little like so:

struct MyClient<T: TcpConnect> {
    connector: T,
    connection: Option<T::Connection<'?>>,
}

impl<T: TcpConnect> MyClient<T> {
    fn connect(&mut self) {
        self.connection = self.connector.connect(remote); //  lifetime that matches the function
    }
}

The issue arises with lifetimes. T::Connection takes on a lifetime that must be less than that of the connector meaning (I think) that as far as the borrow checker is concerned, this is a self-referential struct.

If that is not the case, then I wonder what the lifetime parameter of T::Connection should be. If I give it one (say a) then the reference to self in connect must also receive the annotation which screws up borrowing in any code that tries to use it.

Is there a way to store T::Connection in this manner?

If the conceptual above is not clear enough, I can put an example together.

For context, I am porting some code that used TcpClientStack to have an async interface. If this is not supported (ie I am not just being dense) then I will just refactor to clean things up; this seemed like the path of least resistance from TcpClientStack::TcpSocket

Re-exports of publicly used crates

The state of crates used publicly is currently inconsistent:

  • nb (which is used in many error types) is pub used.
  • heapless (which is part of the return type of gethostbyaddr) is not.

This means that as an implementer of embedded-nal I need to depend on heapless and hope for the versions to match. The straightforward way to mitigate would be pub use of heapless.

e-nal-async: Missing lifetimes in `UdpStack`

Connected, UniquelyBound and MultiplyBound associated types are currently not lifetimed.

I need them to be, so that I can keep a non-mutable reference to the "Udp Stack" instance in their impls.
While I can Arc the "Udp Stack" and then push the arc into the UDP socket impl, this just doesn't feel right.

Perhaps it is no coincidence that embassy-net's UDP socket stack is not implementing the e-nal-async traits? I don't think it would be possible anyway, without the above lifetimes.

Background: I stumbled on this by accident, while trying to implement a toy websockets gateway that tries to implement embedded-nal-async traits by multiplexing all traffic (UDP and TCP) over a single websocket to an actual STD based embedded-nal-async impl.

Anyway, I'll also work on a PR that implements a lifetimed version of the UDP associated types on top of embassy-net.

Reintroduce UDP traits for async

The UDP traits were removed from the embedded-nal-async were scrapped in #70; so far, only the client-side TCP connections were re-introduced in a socket oriented fashion.

Please update this issue when starting work on UDP sockets; I have some ideas for doing that but no time yet -- creating this issue to ensure we don't all start off simultaneously.

[edit: Writing up traits now, will send a pinging update once there is a branch]

Bind to IP

Currently the docs for bind state:

Create a new TCP/UDP socket and bind it to the specified local port.

While trying to implement UdpServer / UdpFullStack for std-embedded-nal I noticed that usually these sockets are also bound to specific IPs / interfaces:

pub fn bind<A: ToSocketAddrs>(addr: A) -> Result<UdpSocket>

How is this supposed to work with the embedded_nal traits?

[Proposal] Socket-directed API

I was discussing the design of the NAL with @jordens the other day and an interesting point was brought up.

Why not change the API of the embedded-nal to more closely follow python/std::net?

Sample proposal:

pub trait UdpSocket {
    type Error;

    fn bind(&mut self, port: u16) -> Result<(), Error>;
    fn send(&mut self, data: &[u8]) -> Result<usize, Error>;
    fn receive(&mut self, data: &mut [u8]) -> Result<usize, Error>;
    fn close(self);
}

pub trait TcpSocket {
    type Error;

    fn connect(&mut self, remote: SocketAddr) -> Result<(), Error>;
    fn send(&mut self, data: &[u8]) -> Result<usize, Error>;
    fn receive(&mut self, data: &mut [u8]) -> Result<usize, Error>;
    fn close(self);
}

pub trait NetworkStack {
    type UdpSocket: UdpSocket;
    type TcpSocket: TcpSocket;
   
    fn open_udp(&mut self) -> Result<Self::UdpSocket, Error>;
    fn open_tcp(&mut self) -> Result<Self::TcpSocket, Error>;
}

This allows stack implementations to freely hand out sockets without having to worry about coherency between multiple threads when the network stack assigns a unique buffer to each socket. In other cases, the network stack can implement some means of synchronizing access (e.g. for off-chip network stacks controlled via serial communications).

This makes it much easier to use multiple sockets in different threads/contexts with a single network stack.

What are some of your thoughts on this @jonahd-g / @MathiasKoch ?

Use existing crates

This looks great for getting us started!

Maybe we should consider using the existing https://github.com/dunmatt/no-std-net for IPAddress and SocketAddress, as that is essentially the std::net without the std part.

Perhaps it would even make sense eventually to merge the two..

Socket as reference when closing

Hello,
I was wondering if there wasn't an error when closing the socket

Shouldn't the signature be:

	fn close(&self, socket: &Self::TcpSocket) -> Result<(), Self::Error>;

Instead of:

	fn close(&self, socket: Self::TcpSocket) -> Result<(), Self::Error>;

So that it can take the reference of the socket.

Best,
Cyril

Publish crate to crates.io

Currently, I know both I and @MathiasKoch are looking to publish crates that utilize the nal to crates.io, but that's currently not possible because the embedded-nal has not yet been published.

Is it possible for us to generate an initial release for this crate, and/or are there things we should consider before publishing?

self mutable reference

I am a bit unsure on why this shouldn't have a mutable reference to self?

The way i see it, you would usually implement this on some sort of network client, owning access to a peripheral (unix fd, SPI, I2C, Serial etc.), but most (all?) of these require a mutable reference to be able to send data. This could be fixed by wrapping the peripheral in a RefCell, but even then there is a good chance that the client object would hold stuff like internal state etc that needs to be mutated, meaning that in my particular case, i would end up with a client object of 5 fields, all wrapped in RefCell.

Is this intended behavior, or am i implementing something wrong in general?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.