search

rust-embedded / embedded-hal

A Hardware Abstraction Layer (HAL) for embedded systems
Apache License 2.0
2.01k stars 202 forks source link

implement `core::error::Error` #630

Closed rursprung closed 1 month ago

rursprung commented 2 months ago

this trait has been stabilised in Rust 1.81.0.

the existing custom Error types cannot be removed / replaced as that'd be a breaking change. for the same reason it's not possible for them to require core::error::Error being implemented.

however, we can already implement the new trait for all cases where the custom trait has been implemented so far.

existing std feature-gated implementations of std::error::Error have also been moved to core::error::Error and the feature gate removed.

this raises the MSRV to 1.81.0 for most crates, but based on the MSRV policy this should not be an issue.

rursprung commented 2 months ago

see #629 for an alternative approach which however doesn't seem to work (but would be much more concise if it did!)

jordens commented 2 months ago

Looks good. Is there an example on how that would be used in practice (in a hal impl and then in e.g. a device driver)?

I would now expect device drivers to impl core::error::Error for their errors (using e.g. thiserror) as well with fn source(&self) wanting to call the hal impl's Some(&hal_impl_error.kind() as &dyn Error). But that ErrorKind result needs to be kept alive making it a bit inconvenient. I guess they'll need to eagerly call and store the ErrorKind on their creation. Playground.

The full solution appears to be ErrorType::Error: Error + core::error::Error. Demanding that from HALs is breaking but it's not difficult to satisfy. Without that requirement this PR is hard to make use of.

MathiasKoch commented 2 months ago

How does the error trait fit with defmt? Is it possible to use it properly while still leveraging the greatness of defmt?

diondokter commented 1 month ago

How does the error trait fit with defmt? Is it possible to use it properly while still leveraging the greatness of defmt?

This doesn't take away anything. It only adds the trait. You don't need to use it, so there's nothing getting in the way of defmt

MathiasKoch commented 1 month ago

Yeah, I get that. The question was more on whether there is a way to utilize the error trait in a defmt application, or if that would always end up being all device side formatting?

Dirbaio commented 1 month ago

2 weeks have passed and no one from the HAL team has raised concerns, let's merge.

I propose merging but waiting a bit more for release.