Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for input/output pin traits #397

Open
eldruin opened this issue Aug 9, 2022 · 3 comments
Open

Tracking issue for input/output pin traits #397

eldruin opened this issue Aug 9, 2022 · 3 comments

Comments

@eldruin
Copy link
Member

eldruin commented Aug 9, 2022

The input/output pin trait available on the 0.2.x versions have been removed ahead of the 1.0.0 release. See: #357
This issue servers as a tracking issue until we add them back.
An interface should be worked out which allows changing the pin mode while being useful and usable by both generic drivers as well as MCU/OS HAL implementations.

IOPin trait as of the 0.2.7 release:

Relevant issues/PRs:

Please feel free to participate, highlight your current use cases, problems and provide solutions.

@Finomnis
Copy link

Finomnis commented Sep 20, 2022

I already mentioned this in the prior IoPin discussion: It's unclear to me what IoPin is meant to do.

There is two things that could change between an input and an output config:

  • the read functionality could be enabled/disabled (possible on nrf chips)
  • the pin could switch between being driven/floating

Should switching an IoPin to "read" disable its output? Or should it just enable input capability?

Further, should pin configs that are already InputPin + OutputPin now implement IoPin<Self, Self>? What would that even mean? For example, let's take the pretty common OpenDrain pin config. It is usually already InputPin + OutputPin.

  • Should it implement IoPin<Self, Self>?
  • If it is in its driving state, should changing it to "read" switch its output to the floating state?
  • Should the read buffer (exists for example on nrf chips) be disabled when switching to "write"?
  • Should it be possible to do .try_into_input_pin(), followed by .set_high()? (As it would be with all IoPin<Self, Self>)

Another question is: Who decides which pin configuration combinations should form an IoPin? Due to having n^2 possibilities, annotating all of the possible combinations would get out of hand very quickly.

@ryankurte
Copy link
Contributor

it's certainly tricky to express all this huh! i wonder if the type-based swap read and write types is a little artificial.

could we stack traits into something like IoPin: InputPin + OutputPin + PushPull + OpenDrain + OpenSource + TriState?
no change for folks using InputPin or OutputPin, writing to OutputPin while in TriState would set pulls, and to switch from Output to Input you would need to go via TriState (then reconfigure pulls if required).

alternately we could add a DriveMode trait to enable the swapping, generic over markers for common modes like PushPull, OpenDrain, OpenSource, TriState, which for drivers would look something like: IoPin: InputPin + OutputPin + DriveMode<PushPull> + DriveMode<TriState> (and if we wanted, auto-impl the traits above for convenence).

@peckpeck
Copy link

First I'd like to point out that there are 2 questions here :

  • Do we want/need a trait for dynamic type where the mode is not known at compile time (ie a pin with a set_mode method) ?
  • How to properly define a Input/Output trait which handles the mode at compile time ?

Let's consider only the second question and answer the first one once we get there.

I know that all of you know this but et me state the actors here, and their needs :

  • The trait implementer, who has a pin that can often fake many modes Input,Output,Alt,Floating,Opendrain ...
    they don't want to implement all In x Out IoPin types
    they don't want to implement a big IoPin with all sub types inside it
    they want to keep the promise that only one mode exist for a given pin at any time
  • The trait consumer, typically a device diver writer who need a common ground to use on any system
    here the common ground is having only one Input and one Output mode for given IoPin
    they also want to store this IoPin permanently
  • The trait user, who assembles a device and a chip
    they want to create a specific iopin with one input and one output mode and give it to the consumer

Translated into code

  • User need means we want a trait IoPin<In,Out>
  • Common modes means a where In: InputPin, Out: OutputPin
  • Not having to implement all cases means we must split the trait trait IoPin<In,Out>: IntoInput<In> + IntoOutput<Out>
  • Keeping the single mode promise imply implementing ̀ fn into_input(self) -> In` with moving semantics
  • But being able to store it permanently means we prefer using a fn as_input(&mut self) -> &mut In instead

Implementing as_input is not easy, so we have is an impedance mismatch between what consumer need and what implementer can easily implement.

I think this impedance mismatch should be resolved by the hal, it should provide a trait for the consumer,
another trait for the implementer and glue then together with some code.

Here is my attempt : 25d4f39

There is a conversion trait that everyone can implement (and same for output) and could be useful not only for IoPin

trait TryIntoInputPin<I:InputPin> {
   //...
   fn try_into_input_pin(self) -> Result<I, Self::Error>;
}

There is a, iopin trait that is nice to use

trait IoPin<I:InputPin,O:OutputPin> {
  // ...
  fn as_input_pin(&mut self) -> Result<&I, Self::Error>;
  fn as_output_pin(&mut self, state: PinState) -> Result<&mut O, Self::Error>;
}

And there is an implementation of an IoPin called GenericIoPin if the spare implementer from writing the logic.
The drawback of this implementation is that it is not possible to have an infallible version nor a zero sized version. But you can still write your own.

I don't know if having infallible conversion would allow an infallible GenericIoPin since there is is still an Option::take() and I'm not sure if rust can guarantee that we will always insert back something before returning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants