search

rust-embedded / embedded-hal

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

guidance on error handling/propagation of drivers #576

Open rursprung opened 8 months ago

rursprung commented 8 months ago

since virtually all embedded-hal APIs are fallible (returning a Result) the question has arisen how drivers should return these to the caller when they don't deal with them themselves: should they propagate them upwards 1:1 or should they cover them in a generic "something went wrong" error? the former has the benefit that the caller of the driver could potentially try to rectify the situation if that were possible (how often is that the case?) but it has the downside of making the API a lot more verbose (presuming that the driver doesn't just have a single e-h error type to propagate up).

a concrete case where this came up is this PR: https://github.com/rust-embedded-community/tb6612fng-rs/pull/37

i think that there should be a general guidance for drivers on how to handle this (what was the reasoning behind the structuring of the errors in the current way?) which should be documented here, centrally.

Rahix commented 8 months ago

a generic "something went wrong" error?

As you stated, this can make the API less verbose and will probably be good enough for the vast majority of use-cases. However, I see one major downside: It makes debugging a lot harder because you can no longer just "print the error" in your application to introspect what is going wrong under the hood.

ryan-summers commented 7 months ago

Yes, I have used the approach of mapping all errors into a different "something went wrong" error in the past, and it made things a nightmare in my end application to figure out what was actually happening.

I then immediately refactored my approach to wrapping the e-h Error type into my custom error type so I could propogate it outwards, and I would highly recommend that approach. As @Rahix mentioned, debugging is exceptionally difficult otherwise.

BartMassey commented 7 months ago

+1 for wrapping. I wish thiserror had a no_std feature. Looks like there's some substitute crates for this, but it would be nicer to bundle it maybe?