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` (dynamic approach) #629

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 add an impl core::error::Error for dyn Error for every custom error type to provide an automatic coverage. HALs can still add an implementation for their specific error type if they wish to add more details (e.g. implement source). as core::error::Error requires core::fmt::Display to be implemented a simple blanket implementation has been added which prints the Debug output.

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

it seems that this doesn't work as intended, as this doesn't build (tested with i2c::ErrorKind for which an impl Error for ErrorKind exists which in turn has an impl core::error::Error for dyn Error {}):

fn cause_error() -> Result<(), ErrorKind> {
    Err(ErrorKind::Bus)
}

fn foo() -> Result<(), Box<dyn core::error::Error>> {
    cause_error()?;
    Ok(())
}

with #630 (alternative implementation) this works.

i don't know if there's a way to make the approach from this PR here work (in which case HALs wouldn't need to do anything to support core::error::Error except maybe for special cases) or if the other approach has to be used.

rursprung commented 1 month ago

closed in favour of #630