port: Add methods to temporarily switch a pin to a different mode #205

Rahix · 2021-07-28T06:46:52Z

This allows temporarily making an output pin an input and vice versa:

let mut pin = pins.d0.into_output();

pin.with_pin_as_floating_input(|p| {
    // p is input here
    p.is_high();
});

Similarly:

pin.with_pin_as_pull_up_input()
pin.with_pin_as_output()
pin.with_pin_as_output_high()

TODO

Deduplicate implementation.
Check that all invariants are upheld and pin-state is always returned to what it was before.

Investigate whether we want the pin to be in the same state, or just the same mode - an alternative API could be one where a correct pin must be returned from the closure again:

let pin = pins.d0.into_output();
assert!(pin.is_set_low())
pin.with_temporary(|p| {
  let p = p.into_floating_input();
  // ...
  p.into_output_high()
})
assert!(pin.is_set_high())

Rahix · 2022-01-28T23:05:59Z

FWIW, other HALs have come up with a very similar API. For example: https://docs.rs/stm32f4xx-hal/0.11.1/stm32f4xx_hal/gpio/struct.Pin.html#method.with_floating_input

mutantbob · 2022-05-10T20:04:40Z

I wonder how this would behave if the pin were currently being used as the CS for an SPI transaction.

Rahix · 2022-05-10T20:10:36Z

Well, the CS pin must be managed manually anyway so you can do weird stuff in any case... I don't think this changeset here would make it worse than it already is, or am I missing something?

haennes · 2022-07-20T14:45:43Z

Hello,
I really need this for my project.
Is there any way I can help?
Should I fork it and then open a new PR?

Rahix · 2022-07-20T17:01:13Z

@haennes, giving me some feedback and testing this PR here is all I need. I just held off merging it because I wanted to hear from users first. Maybe you can comment on some of the open questions from the OP? What would be best for your usecase?

haennes · 2022-07-23T16:32:59Z

It works on an Arduino Nano (from Elegoo) (Atmega m328p).
I will test it on an Arduino Uno R3 (from Elegoo) (Atmega 328p) too.
Although the 2nd implementations seems better at a first glance, the way it currently is is quite perfect, as it removes a lot of boilerplate code in the users project.

Rahix · 2022-07-26T11:32:41Z

avr-hal-generic/src/port.rs

+        let pin: Self = unsafe { core::ptr::read(self) };
+        let mut pin = pin.into_pull_up_input();
+        let res = f(&mut pin);
+        *self = if state {
+            pin.into_output_high()
+        } else {
+            pin.into_output()
+        };


This implementation is still quite ugly and not 100% sound either. I quite like the approach in stm32f4xx-hal: https://docs.rs/stm32f4xx-hal/0.11.1/src/stm32f4xx_hal/gpio/convert.rs.html#476-490

This is at least more sound than my hack. So I'll try to reimplement that pattern here.

Source: https://github.com/stm32-rs/stm32h7xx-hal/blob/21d9c06a57169e92cf5745e582dca171c73294c9/src/gpio/convert.rs

It looks like this will unfortunately require a larger change to make it work. But I think following that route is still a good idea and will improve the remainder of the port module as well. Will just need a bit more time...

…ferent mode

hiszd · 2023-05-18T13:00:23Z

Am I correct in saying that implementing the IoPin trait from embedded-hal would take care of this as well?
https://docs.rs/embedded-hal/latest/embedded_hal/digital/v2/trait.IoPin.html

Rahix · 2023-06-01T00:40:04Z

@hiszd, not really. IoPin moves the pin to change its state more permanently while the methods in this PR only change the pin state for the duration of a closure.

RenjiSann · 2023-07-14T23:08:34Z

Hi,

I just finished implementing a driver for the DHT11 sensor on an Elegoo Arduino Uno R3.
I had to use a pull up pin with temporary output mode to send the data request signal, and I can attest that it worked without any problem.

This might need a little refactoring before it is anywhere close to presentable, but it could eventually be integrated into the examples someday.

jaqx0r · 2024-09-23T21:52:54Z

With regards to the final bullet in the first comment, I think the user should not be responsible for returning a valid pin from the closure. We should assume the with* function takes care of maintaining state and correctness. I think that also makes it easier to return a pin's value from the closure in the case the pin was made input.

Is there a difference between changing mode with into_output_high() and calling set_high() on a low output? I think there isn't so a user can change state if they need after calling this closure with no return control.

The original code sets pins to floating inputs in the `all_off` function, which is not what this code was doing until now. This change converts the activation function to move the LED pins to hi/lo output when activating a specific LED, then moving them back to the pin array when done, as deactivated. This isn't exactly nice, and we eagerly await Rahix/avr-hal#205 to land so this code doesn't use `mode::Dynamic` overhead, or the unsafe pointer access.

Rahix · 2024-09-25T01:04:17Z

With regards to the final bullet in the first comment, I think the user should not be responsible for returning a valid pin from the closure. We should assume the with* function takes care of maintaining state and correctness. I think that also makes it easier to return a pin's value from the closure in the case the pin was made input.

If we switch to the design from stm32-hal I linked above, this can be done. The pin is only passed to the closure by (mutable) reference so there is no way the closure can move the pin to the outside world and break invariants.

I agree that a user-controller closure return type makes for a much nicer API.

Is there a difference between changing mode with into_output_high() and calling set_high() on a low output? I think there isn't so a user can change state if they need after calling this closure with no return control.

Yes, there is a difference. Calling into_output() and then set_high() leads to a brief glitch on the pin where it's state is LOW for a short time. In some applications, this can trip up external components. into_output_high() switches from high-impedance input mode to HIGH output without any glitches.

Rahix mentioned this pull request Aug 2, 2021

Tracking issue for the architectural refactor #130

Open

42 tasks

Rahix added hal-api API design for the different components of avr-hal hal-generic Related to MCU generic parts of avr-hal labels Aug 2, 2021

Rahix mentioned this pull request May 12, 2022

Pin configuration change in loop #269

Closed

Rahix force-pushed the dynamic-mode-switch branch from c1225f2 to 4952650 Compare July 4, 2022 19:45

Rahix commented Jul 26, 2022

View reviewed changes

[WIP] generic: port: Add methods to temporarily switch a pin to a dif…

7a79cf4

…ferent mode

Rahix force-pushed the dynamic-mode-switch branch from 4952650 to 7a79cf4 Compare October 8, 2022 22:22

Rahix mentioned this pull request Sep 23, 2024

Making a tri-state pin (switching between input and output), perhaps like embedded_hal::digital::IoPin #583

Open

Rahix linked an issue Sep 24, 2024 that may be closed by this pull request

Making a tri-state pin (switching between input and output), perhaps like embedded_hal::digital::IoPin #583

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

port: Add methods to temporarily switch a pin to a different mode #205

port: Add methods to temporarily switch a pin to a different mode #205

Rahix commented Jul 28, 2021 •

edited

Loading

Rahix commented Jan 28, 2022

mutantbob commented May 10, 2022

Rahix commented May 10, 2022

haennes commented Jul 20, 2022 •

edited

Loading

Rahix commented Jul 20, 2022

haennes commented Jul 23, 2022

Rahix Jul 26, 2022

Rahix Oct 8, 2022 •

edited

Loading

hiszd commented May 18, 2023

Rahix commented Jun 1, 2023

RenjiSann commented Jul 14, 2023 •

edited

Loading

jaqx0r commented Sep 23, 2024

Rahix commented Sep 25, 2024

port: Add methods to temporarily switch a pin to a different mode #205

Are you sure you want to change the base?

port: Add methods to temporarily switch a pin to a different mode #205

Conversation

Rahix commented Jul 28, 2021 • edited Loading

TODO

Rahix commented Jan 28, 2022

mutantbob commented May 10, 2022

Rahix commented May 10, 2022

haennes commented Jul 20, 2022 • edited Loading

Rahix commented Jul 20, 2022

haennes commented Jul 23, 2022

Rahix Jul 26, 2022

Choose a reason for hiding this comment

Rahix Oct 8, 2022 • edited Loading

Choose a reason for hiding this comment

hiszd commented May 18, 2023

Rahix commented Jun 1, 2023

RenjiSann commented Jul 14, 2023 • edited Loading

jaqx0r commented Sep 23, 2024

Rahix commented Sep 25, 2024

Rahix commented Jul 28, 2021 •

edited

Loading

haennes commented Jul 20, 2022 •

edited

Loading

Rahix Oct 8, 2022 •

edited

Loading

RenjiSann commented Jul 14, 2023 •

edited

Loading