Input/Output pin trait proposal

[rumatoest]

Hello.

This is my vision related to RFC #29

I think such interface is more convenient to use because it implies that there should not be any new structures creation, "mappings" from input ping structure to output, etc. Everything should work in place and it have to be fast.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ryankurte (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[ryankurte]

seems like a reasonable start to me, with a few questions:

• what should happen if you try to set the output on a pin in input mode (or vice-versa)?
• are there likely to be cases where the input and output errors are different?
• since this is basically just a trait over two existing traits, would it be equivalent to introducing a ModeSet trait and using InputPin<...> + OutputPin<...> + ModeSet<...> as bounds? not suggesting this would be better or worse, just an alternative

the first two points could be addressed by providing a wrapper error enum i think.

cc. @therealprof since you've been in the prior discussions.

[therealprof]

Has this ever seen a compiler? Error should be (or rather is) an associated type. Also I'm not sure this is actually implementable in this way, also for the reason @ryankurte mentioned that error handling becomes messy if the pin is in the wrong mode, but especially because on a lot of hardware those InputPin and OutputPin implementations are usually mutual exlusive and thus non-composable.

I still think that the only clean way to implement this would be a TriStratePin trait which allows for Low, High and Input modes (where Input could be either mean Pull-Low, Pull-High or High-Z configuration). Such a trait needs a special implementation usually involving internal state. Also those implementation are going to be not quite as cheap as the other would like them to be because switching a pin from input to output and vice versa safely is going to be a somewhat expensive operation with most MCUs due to requiring a RMW operation on a shared register.

[rumatoest]

• what should happen if you try to set the output on a pin in input mode (or vice-versa)?

It means that pin should be "reconfigured" to work using input (floating) mode. If there are such pins that can work both ways at the same time I presume that in such case pin should "release" previously set high or low mode.

are there likely to be cases where the input and output errors are different?

IMHO allowing to have different errors for input/output (and this also implies to have additional error for InputOutputPin) could make API hard to use.

since this is basically just a trait over two existing traits, would it be equivalent to introducing a ModeSet trait and using trait and using InputPin<...> + OutputPin<...> + ModeSet<...> as bounds? not suggesting this would be better or worse, just an alternative

In this case there have to be another trait that combines all this 3 traits together. I think that if there is such trait as InputOutputPin all HAL implementations should definitely provide it. If it would be only ModeSet<...> trait than HAL implementation many not have such structure that implements all 3 of them InputPin<...> + OutputPin<...> + ModeSet<...>

because on a lot of hardware those InputPin and OutputPin implementations are usually mutual exlusive and thus non-composable.

Also those implementation are going to be not quite as cheap as the other would like them to be because switching a pin from input to output and vice versa safely is going to be a somewhat expensive operation with most MCUs due to requiring a RMW operation on a shared register.

This is mostly HW implementation issues that I think have be fixed anyway. Do not forget about API simplicity for end user. Arduino API can do this, also you can achieve same reslult using C/C++ API. Right now in rust HAL implementation it is extremely hard to do ant this is why I think it has to be fixed.

Traits is a great advantage. It allows to create drivers for HW components without depending on particular architecture. So it is a good idea to cover all possible usage scenarios via traits. Probably there would be performance loss for such implementations, but this price should be paid for generic solution. And this is what HAL is about.

I still think that the only clean way to implement this would be a TriStratePin

Separate trait without including InputPin and OutputPin is also a good idea if it's API can do the same.

[therealprof]

Arduino API can do this, also you can achieve same reslult using C/C++ API.

Yeah but not safely and potentially not on all hardware. In general "What would Arduino do?" is an irrelevant question because the implementations are horrific hacks for most of the part and nothing we strive to replicate.

Probably there would be performance loss for such implementations, but this price should be paid for generic solution.

Performance is not really a concern because the traits cannot provide a blanket implementation for such low-level operation anyway. The concerns are implementability and safety; there's little value in such a trait if it's not implementable on real hardware and the same is true if it's not safe to implement it.

I'll leave a few more comments inline on your proposal about this.

Separate trait without including InputPin and OutputPin is also a good idea if it's API can do the same.

Sure, but with protection against misuse.

[therealprof]

These functions will (in most real world uses) require inline blocking due to the read-modify-write operations involved to change between input an output which is generally frowned upon.

[rumatoest]

Yes, indeed, but there are some platform related options available to make it more safe like using asm instructions to block interrupts or using CAS instructions.
I know that it would not be fastest possible implementation, but it has to be as easy to use as possible. Performance drawbacks can be described in trait documentation. BTW there is an opinion that for such cases performance (1 pin bus throughput) should not be big deal.

[luojia65]

Maybe fn mode_output(Self) -> Result<Self::Output, Self::Error>?

[therealprof]

This cannot work because an object implementing InputPin cannot know that it also is implementing InputOutputPin and needs to behave differently if it is in output mode and vice versa. That's what I mean by "this is not composable".

[rumatoest]

But there can be some kind of structure which does know about current pin mode and can use one or another nested pin structure (via enum for example) to provide required functionality.

[therealprof]

You cannot retroactively impose additional restrictions or interfaces on existing implementations with a new trait.

[rumatoest]

Why not? HAL has not released yet and it has unproven feature. So it is possible to add new unproven trait to new HAL versions.

[therealprof]

Sure, but whatever you put in the traits will not have any influence on the trait impls which are out there.

[rumatoest]

the implementations are horrific hacks for most of the part and nothing we strive to replicate.

But unfortunately it is looks like they have biggest market share among hobbyist at least. And I bet that many developers would compare your HAL API with Arduino platform in some way. No matter how correct safe and HW friendly API would be if it would not provide easy to use interfaces to cover all major cases that Arduino already has that this can be a huge issue for developers willing migrate to rust ecosystem.

there's little value in such a trait if it's not implementable on real hardware and the same is true if it's not safe to implement it.

Does not look like it is not possible. Because in this case interfaces which use single pin input/output communications would never exists.

[therealprof]

But unfortunately it is looks like they have biggest market share among hobbyist at least. And I bet that many developers would compare your HAL API with Arduino platform in some way. No matter how correct safe and HW friendly API would be if it would not provide easy to use interfaces to cover all major cases that Arduino already has that this can be a huge issue for developers willing migrate to rust ecosystem.

We're trying to cater for different use cases than Arduino which is meant for hobbyists/makers.

Does not look like it is not possible. Because in this case interfaces which use single pin input/output communications would never exists.

I have no idea what you're trying to tell us here. Rust is all about safety, performance and reliability; if you throw those attributes over board, everything is possible but then you might as well just stick to Arduino.

[rumatoest]

We're trying to cater for different use cases than Arduino which is meant for hobbyists/makers.

I can bet that almost all coffee vending machines in my city and county as well are based on Arduino. Very nice DIY hobby.

because on a lot of hardware those InputPin and OutputPin implementations are usually mutual exlusive and thus non-composable.

I wonder why nobody meant that trait that I proposed was more about open drain pins than switchable input output? For last 2 weeks I found out that in most cases open drain pin or its software emulation needed to implement custom communication bus.

I still think that this is something that have to be in HAL specification. I will update my pull request soon.

[david-sawatzke]

I wonder why nobody meant that trait that I proposed was more about open drain pins than switchable input output?

Implementing InputPin & OutputPin on an OpenDrain pin should be sufficient to encapsulate the functionality, right? There's no need for a switch as proposed in your pr. There are already some implementations doing this, see https://github.com/stm32-rs/stm32f0xx-hal/blob/c944a4e88230972df1f5f1dd6d8eeb62cdf6f68e/src/gpio.rs#L108.

[rumatoest]

Implementing InputPin & OutputPin on an OpenDrain pin should be sufficient

Yes, but this is optional feature. Developers are free to not implement both traits for pin. I suggest to have separate trait that includes both Input+Output and describes it behavior. This should push developers to implement such behavior on software level even if there is no HW support for it.

For example I was able to implement open drain like behavior for Rasberry Pi pins via rppal library which does have only separated input / output behavior. Looks like Arduino drivers do the same most of the time

There's no need for a switch as proposed in your pr.

I will update it soon.

[therealprof]

Yes, but this is optional feature. Developers are free to not implement both traits for pin.

If you invent a new trait then developers are similarly free not to implement it so I don't quite see your point here.

This should push developers to implement such behavior on software level

Nothing prevents you from wrapping a separate InputPin and OutputPin implementations in a a new implementation which implicitly switches the pin from input to output and vice versa and implements both InputPin and OutputPin.

even if there is no HW support for it.

That's nonsense. If the HW doesn't support what you want to do, you can't implement it.

[rumatoest]

That's nonsense. If the HW doesn't support what you want to do, you can't implement it.

AFAIK RPi does not have OpenDrainOutput HW mode for pins which means that there is no direct HW support for such behaviour but it is possible to emulate it by simply switching between input and output modes.

[therealprof]

AFAIK RPi does not have OpenDrainOutput HW mode for pins which means that there is no direct HW support for such behaviour but it is possible to emulate it by simply switching between input and output modes.

That is not relevant. If you want to switch between input/output or tri-state pins, that also works in push-pull configuration if your connected hardware plays ball (i.e. doesn't drive against the MCU).

You cannot emulate electrical properties in software, if you need OD and the hardware doesn't support it there's nothing software will fix.

[eldruin]

Closing since #29 was closed in #269. Thanks everybody.