Add error traits for communication interfaces

[eldruin]

There has been a lengthy discussion about this in #229.
This implements proposal 3 by @TeXitoi which was the latest for I2C, SPI and Serial interfaces.
See here for the analysis
I included a minimal number of variants for each protocol, mostly borrowed from embedded-error.

This raises the MSRV to Rust 1.40.0 as well in order to mark the ErrorKinds as #[non_exhaustive] so that adding a new variant is not a breaking change.
I am also fine with not marking the ErrorKinds as #[non_exhaustive].

Closes #229

[rust-highfive]

r? @therealprof

(rust-highfive has picked a reviewer for you, use r? to override)

[therealprof]

@eldruin Is this in line with the plans of the Rust error handling project group? Haven't had time to check, just making sure that we're not working contrary to what's coming up in upstream Rust...

[eldruin]

Good point.
This proposal is simpler than that and only includes the already standard Error::kind() mechanism, without any kind of chaining, which is what the error WG described here.
If we were to offer a mechanism somewhat similar to Error::source, I think we would need to make the errors contain a generic parameter for the source error, which we have already discussed.
Nevertheless, with this proposal users and HALs can continue to create an error chaining mechanism however they like.

The only thing this proposal does is requiring that whatever error type the HAL implementation returns, a mapping to a set of common error variants exists, upon which generic drivers can act.

[ryankurte]

this seems to me like a good synthesis of the work in the related issue! as with @therealprof i wonder how this aligns with the error-handling WG and existing std:error::Error / failure::Fail infrastructure.

it doesn't seem like it'd be useful to enforce those bounds through our API, but, it might be helpful to add (feature-gated) implementations for either std:error::Error or failure::Fail to our ErrorKinds for if/when they're used directly?

[eldruin]

Hmm, ErrorKinds would not be part of the error chain themselves as I do not understand them as a std::error::Error::source().
Rather, the original HAL-defined error types are the sources and those would form the error chain.
What we would add here is requiring that those errors could be translated into a predefined subset: ErrorKinds, similar to std::io::Error.

If those HAL-defined error types implement std::error::Error, they get failure::Error for free IIUC.

I have implemented core::fmt::Display for the ErrorKinds, though. Which I guess could be useful.

[yodaldevoid]

Hmm, ErrorKinds would not be part of the error chain themselves as I do not understand them as a std::error::Error::source().
Rather, the original HAL-defined error types are the sources and those would form the error chain.
What we would add here is requiring that those errors could be translated into a predefined subset: ErrorKinds, similar to std::io::Error.

I believe there was talk about allowing ErrorKind to directly be as the HAL error type where it fits for a specific HAL. It's fine to explicitly disallow or discourage this use, but if we would still like to support this then I would ask that std::error::Error be conditionally implemented for ErrorKind.

[eldruin]

HALs can indeed use ErrorKind as their error type if that is enough for them. I have implemented the embedded-hal::<protocol>::Error traits for those in this PR.

A different topic would be implementing std::error::Error for ErrorKind as well. That would be an independent addition but only really useful for linux-embedded-hal or so, which I highly doubt would adopt ErrorKind as its protocol error type.
The main point against it being that std::error::Error requires memory allocation so it is uninteresting for no_std HALs.
Anyway if there would really be interest, we could add this in a future release via an opt-in feature, but I would not hold the merge of this since that would be a hypothetical non-breaking addition.

[yodaldevoid]

This PR is just waiting for a final review and a quick update to update the change-log, correct?

[dbrgn]

Thanks for the proposal, I like it. It's a lightweight approach that provides flexibility for HALs to define their custom error types, while at the same time offering a possibility to map custom errors to a standardized set of error types.

[Dirbaio]

+1 to having official "error kinds" that generic drivers can react to. This is sorely needed!

Maybe this PR should enforce that the Error associated types impl the Error trait? The original proposal does so.

[eldruin]

@Dirbaio @yodaldevoid I missed re-adding the Error trait requirements on the associated types today by mistake as I re-did this PR completely on top of the current master branch. Sorry for the confusion.

[Dirbaio]

I've commented some bikeshedding on the error enums :)

[therealprof]

Still not too much of a fan of the Other kind and would have preferred an ImplError instead as I've done in embedded-error. Anyway, this PR is fine with me.

[yodaldevoid]

Is this still awaiting further reviews?

[therealprof]

LGTM

[eldruin]

🎉
bors r=therealprof

[bors]

Build succeeded:

• ci-linux (1.40.0, x86_64-unknown-linux-gnu)
• ci-linux (stable, thumbv6m-none-eabi)
• ci-linux (stable, thumbv7m-none-eabi)
• ci-linux (stable, x86_64-unknown-linux-gnu)
• ci-linux-test (1.40.0, x86_64-unknown-linux-gnu)
• ci-linux-test (stable)
• fmt

[dbrgn]

🎉 thank you @eldruin!