digital: enforce all traits have the same Error type.

[Dirbaio]

Equivalent of #331 for GPIO traits.

This one is a bit trickier, so it maybe warrants some discussion (this is the reason I've opened everything as separate PR's, sorry for the spam!):

• The bounds on IoPin are becoming very cursed, but I think they're correct... Is there some HAL out there implementing it, so that I can verify this doesn't break it?
• This forces the "input" and "output" errors to be the same. Perhaps we want to only unify Output and Input separately? In practice I don't think this will be an issue, as structs usually impl only Input or Output but not both (except with IoPin maybe?)

[rust-highfive]

r? @therealprof

(rust-highfive has picked a reviewer for you, use r? to override)

[burrbull]

• The bounds on IoPin are becoming very cursed, but I think they're correct... Is there some HAL out there implementing it, so that I can verify this doesn't break it?

https://github.com/stm32-rs/stm32f4xx-hal/blob/dc1552b2fc2e14901203a13ab42e76a5121a08eb/src/gpio.rs#L574

[Dirbaio]

Thanks for the link @burrbull . It seems the bounds are OK, the obvious changes worked with no fuss! Dirbaio/stm32f4xx-hal@091d945

[eldruin]

Seems correct to me, thanks!
Breaking up the unified error type requirement for IoPin in the future would be a non-breaking change, right?
Iff so, this is fine as-is I would say.

[Dirbaio]

Breaking up the unified error type requirement for IoPin in the future would be a non-breaking change, right?

I have no idea to be honest. Removing the ErrorType bounds on TInput/TOutput seems like it could break consumers relying on it, so it's maybe breaking...?

[eldruin]

Breaking up the unified error type requirement for IoPin in the future would be a non-breaking change, right?

I have no idea to be honest. Removing the ErrorType bounds on TInput/TOutput seems like it could break consumers relying on it, so it's maybe breaking...?

Hmm, I see.
There might be an IoPin impl wanting to define a different error type. Something like:

enum IoPinError {
  Input(...),
  Output(...),
  FailedToChangeMode
}

Given this, I would tend not to unify the error type for IoPin.

[Dirbaio]

Changed to leave IoPin alone.

Anyway IMO IoPin requires a redesign.

• It's incredibly unwieldy to impl IoPin trait not practical to implement with multiple input/output types #340
• It's incredibly unwieldy to use: you can't simply own one in a driver struct, because every time you use it it changes type. Tshi forces you to store Options or enums and constantly .take()ing it out and putting it back, which results in panicky code with lots of runtime checks.
• There's still no way to switch between PullUp/PullDown/NoPull, and PushPull/OpenDrain
• There's no way to impl it in HALs without typestates (such as Embassy). It'd be impl<T: Pin> IoPin<Input<T>, Output<T>> for Blah, but there's not enough info in the type signature to know if you want PushPull or OpenDrain when you switch from Input to Output. Embassy won't impl IoPin.

but that's a different story I guess...

[eldruin]

Hmm, I still see IoPin inherit from ErrorType, though.
Could you add that criticism of IoPin to #340?
Have you read through the original discussion at #29?

[Dirbaio]

I screwed up the rebase somehow, it's OK now

[Dirbaio]

Friendly ping @eldruin @therealprof @ryankurte are there any pending concerns on this now that IoPin is left untouched?

[therealprof]

LGTM, thanks.

bors r+

[bors]

Build succeeded:

• ci-linux (1.46.0, x86_64-unknown-linux-gnu)
• ci-linux (stable, thumbv6m-none-eabi)
• ci-linux (stable, thumbv7m-none-eabi)
• ci-linux (stable, x86_64-unknown-linux-gnu)
• ci-linux-test (1.46.0, x86_64-unknown-linux-gnu)
• ci-linux-test (stable)
• fmt