Overhaul peripheral signal system, first run #2388
Conversation
bugadani
force-pushed
the
gpio
branch
3 times, most recently
from
d32893e to
d650388
Compare
bugadani
force-pushed
the
gpio
branch
2 times, most recently
from
f1f810a to
bf8fb0b
Compare
| impl<P: OutputPin> PeripheralOutput for P {} | ||
|
|
||
| impl PeripheralInput for InputSignal {} | ||
| impl PeripheralInput for OutputSignal {} |
There was a problem hiding this comment.
Why have OutputSignal and OutputConnection implement PeripheralInput?
I noticed in the drivers that used to ask for bidirectional signals (i.e. impl PeripheralInput + PeripheralOutput), they've been changed to now just ask for PeripheralOutput, and this impl here allows that to work, but I don't see/understand the motive for that.
I suppose there's no such thing as an output-only pin, so on the hardware level it doesn't matter.
I still liked being able to see what the driver actually needed though. input, output or both.
There was a problem hiding this comment.
I suppose there's no such thing as an output-only pin, so on the hardware level it doesn't matter.
Exactly this. PeripheralInput + PeripheralOutput is just the same thing written in more words.
There was a problem hiding this comment.
Would a naming change help here? e.g. PeripheralInputOutput?
There was a problem hiding this comment.
I guess, but we also don't call our GPIO traits InputOutputPin
There was a problem hiding this comment.
I think we had naming like that before 🤔 but not anymore 😄
bugadani
force-pushed
the
gpio
branch
4 times, most recently
from
66ef39f to
b753900
Compare
Co-authored-by: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com>
There was a problem hiding this comment.
LGTM. I'm a bit skeptical about the input+output thing but let's see where it goes.
Once #2273 is sorted, my skepticism will be gone.
This PR (also part of #2341) removes logic from the PeripheralInput and PeripheralOutput traits, and updates peripherals to use these traits, acquire the type-erased signals and work with those.
GPIO pins no longer have to implement signal functions directly, which is I think a big plus as it allows removing duplicate code from the gpio module.
As this PR is quite large, I did not complete all allowed cleanup opportunities.