search

rust-embedded-community / embedded-nal

An Embedded Network Abstraction Layer
Apache License 2.0
177 stars 25 forks source link

Managing failed operations #18

Open ryan-summers opened 4 years ago

ryan-summers commented 4 years ago

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).

ryan-summers commented 4 years ago

Right now, the current solution to this would be to just return a TcpSocket as a result of connect() that is not in the connected state (e.g. calling is_connected() would return false). This seems counter-intuitive at best.

ryan-summers commented 4 years ago

Worth noting is the conversation happening in https://github.com/rust-embedded/embedded-hal/issues/229, as we will likely want to follow a very similar pattern here.

Sympatron commented 3 years ago

In general I like this idea, because generic drivers currently don't know anything about what went wrong, other than that it's an Err.

I would like a trait bound on Error more than an enum. There you could define a method like is_internal_error() (I know I'm bad with naming things) for your first case, without locking implementers into a specific type they cannot extend.