embedded-hal
embedded-hal copied to clipboard
Suggestion: Avoid `ErrorType`
I agree that the ErrorType trait is somewhat useful, motivation here, but I think that it may actually in most cases be detrimental to the API?
In each of the cases where it is currently used, I think there is a better pattern to use instead, that will be simpler for users to understand, easier for implementers to use, and more flexible and correct.
Let's consider the modules one at a time:
-
embedded_hal::digital:ErrorTypeshould be renamedPinsince that is what it logically represents (it is a supertrait ofInputPinandOutputPin). -
embedded_hal::i2c:ErrorTypeshould be removed, and the associatedErrortype should be moved toI2c.The downside is that now the error type is no longer shared between the blocking and the
asyncversion, though I believe this tradeoff is worth it, since it is very rare that you'd be working with both traits at the same time. -
embedded_hal::pwm:ErrorTypeshould be removed, and the associatedErrorshould be moved toSetDutyCycle(unless you have other plans for extensions to this module, then it may make sense to rename the trait toPwmor similar instead?) -
embedded_hal::spi: Unsure about this, pending discussion in https://github.com/rust-embedded/embedded-hal/issues/525. -
embedded-io: More unsure here, but I suspect theErrorTypedesign is also the wrong design here: an implementation ofReadshould be allowed to have a different error type fromWrite, even though it may be a bit more hazzle to work with types that areRead + Write(users would have to write<T: Read<Error = E> + Write<Error = E>, E>if they wanted to force the same error type).