Allow reading the input state of pins in any function mode

[ThadHouse]

On the actual chip, all input pins are always wired to the SIO block for inputs, no matter what mode the device is in. This means that no matter the function selected, reading the input state would be valid. It would be very nice if the InputPin trait could be implemented for all function types, as this doesn't break any of rust's safety rules.

I actually ran into a case where I needed to work around this. I was using a PWM Input, and needed to know when the falling edge was triggered. However, for the pin to be readable by PWM, it has to be in FunctionPwm mode, which doesn't allow reading the input. I actually had to use into_unchecked to turn the pin back into an input just so I could read it.

[jannic]

I see one trade-off to consider: While we technically are able to implement InputPin for all modes, it does decrease the amount of help the type system provides. In the current implementation, if you think you configured something as input pin, but in fact you configured the pin differently, you get a compile error. This can be really useful in case you mix up some pin numbers.

If all pins impl InputPin, there are a lot more cases that can no longer be caught by the compiler.

Perhaps we shouldn't just impl InputPin for everything, but instead provide some manual way to enable it? For example some wrapper type could be used. That way, accidental mix-ups of pins will still be caught, while special use-cases are covered.

[nilclass]

Perhaps we shouldn't just impl InputPin for everything, but instead provide some manual way to enable it? For example some wrapper type could be used.

The wrapper could be accessible via an as_input() method on pins, like:

let pin = pin.into_function::<FunctionPwm>();
if pin.as_input().is_low() {
    // ...
}

[ThadHouse]

Perhaps we shouldn't just impl InputPin for everything, but instead provide some manual way to enable it? For example some wrapper type could be used.

The wrapper could be accessible via an as_input() method on pins, like:

let pin = pin.into_function::<FunctionPwm>();
if pin.as_input().is_low() {
    // ...
}

That’d be fine, and makes sense to me. Keeps the type safety, but still allows safely reading the state when necessary.

[jannic]

I just noticed that we already have impl<I, P> embedded_hal::digital::v2::InputPin for Pin<I, FunctionSio<SioOutput>, P>, ie. we implement InputPin for pins configured as output. That weakens my argument regarding type safety. We might as well just implement InputPin for all Pin.

If we still want the as_input variant, e4f1a0b is a possible implementation.

[ithinuel]

For some reasons I cannot point yet, implementing InputPin for any function of a pin (effectively for Pin<_,_,_> seems off.

On the other hand, the datasheet explicitly says:

Software control of GPIO, from the single-cycle IO (SIO) block. The SIO function (F5) must be selected for the processors to drive a GPIO, but the input is always connected, so software can check the state of GPIOs at any time.

So I'm not strongly against it.

Alternatively, one can use Sio::read_bank0 to get a raw version of the pins status.

[jannic]

For some reasons I cannot point yet, implementing InputPin for any function of a pin (effectively for Pin<_,_,_> seems off.

I share that feeling. I think it's because we use the type-system to reach several goals, and some of those have divergent requirements:

• Avoid undefined behavior. (This is not optional, as UB isn't localized. A program with UB is buggy.)
• Steer developers towards working programs. "If it compiles, it works". Try to make buggy programs fail to compile.
• Feed information to the compiler. Eg. if the compiler knows the desired pin function, a call like .reconfigure() can automatically do the right thing, without any parameters specifying what should be configured.
• Of course, it should allow as many valid programs as possible. If it prevents the user from doing something the hardware is capable of, there usually is an option to use unsafe instead, but that should be a last resort, and not a common pattern.

In this case, implementing InputPin for all pin modes is fine with the first and the third goal, but it weakens the second goal. Meanwhile, it helps reaching the fourth goal.

In my opinion, there are two valid approaches regarding InputPin:

• either implement it only for Pin<_, FunctionSio<SioInput>, _> (and not, as it's currently done, also for Pin<_, FunctionSio<SioOutput>, _>
• or implement it for all Pin<_, _, _>

I don't see a good reasoning for the current middle ground.

I prefer the first option, with the addition of something like asInput() as an escape hatch. But removing the impl for SioOutput would be a breaking change.

[ThadHouse]

asInput() does seem like it fits into the Rust type system a bit more. Generally the explicit modes do feel a lot better, and help the writer ensure they're actually doing the correct thing. It just makes sense to have a decently safe escape hatch, because reading an output is perfectly valid behavior, and the current solution of either a raw read or into_unchecked is much worse and much more error prone.