rust-embedded / embedded-hal

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

Why do all `Error` traits need to implement `Debug`? #551

Closed Sympatron closed 9 months ago

Sympatron commented 9 months ago

The current design makes it needlessly complicated to write wrappers using embedded-hal traits if the underlying HAL driver implementation error types don't derive Debug for some reason IMO. You either have to create a newtype wrapping the driver error or file a PR in the HAL crate. I don't think this is a huge issue, but still wondered why this was done, because I don't see any benefits. If the HAL driver error derives Debug, it can be used by the end user anyway even if embedded-hal does not require it. And if it doesn't, the traits can still be implemented and this will only be a problem if someone tries to actually invoke Debug::fmt on the error.

Dirbaio commented 9 months ago

If the HAL driver error derives Debug, it can be used by the end user anyway even if embedded-hal does not require it.

This is only true in code that is using concrete HAL types. Requiring Debug allows relying on that on generic (HAL-independent) drivers. For example it allows using .unwrap() which is quite handy.

Implementing Debug on errors is a good practice anyway, HALs should do it.

Sympatron commented 9 months ago

So every driver should just expect every HAL to implement Debug for all error types? Otherwise the drivers won't work with these HALs at all.

What is the advantage of knowing Error will implement Debug as opposed to requiring it with a trait bound when you need it in the HAL-independent driver?

I stumbled upon this while writing a driver and realizing that I can only make this work if I require Debug for the HAL error types. This felt quite repetitive since my driver did not make use of Debug.

Dirbaio commented 9 months ago

I don't understand why implementing Debug is a problem for you. Are you trying to implement the traits for a HAL that is not "yours"? like, wrapping it with a newtype?

The intended usage is HALs themselves implement the traits, then drivers just use the traits. When a HAL author adds the trait implementations, they'll run into "does not implement Debug" errors and add the required Debug derives to fix them.

What is the advantage of knowing Error will implement Debug as opposed to requiring it with a trait bound when you need it in the HAL-independent driver?

If it's optional, an end user will get stuck if they try to use a driver that requires Debug with a HAL that doesn't implement Debug. Forcing HALs to add Debug ensures max compatibility, all drivers will work with all HALs.

jessebraham commented 9 months ago

This is explained in the Types eagerly implement common traits (C-COMMON-TRAITS) section of the official Rust API Guidelines, as well.