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.

Sign up for GitHub

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

Jump to bottom

Dynamic GPIO pin with static mode. #446

Closed
agausmann opened this issue Sep 4, 2022 · 2 comments · Fixed by #585
Closed

Dynamic GPIO pin with static mode. #446

agausmann opened this issue Sep 4, 2022 · 2 comments · Fixed by #585

Comments

@agausmann
Copy link
Contributor

Motivation

In AVR HALs, it is possible to downgrade a pin handle so that the pin number is erased from the type, but the input/output typestate is still preserved.

This makes it possible to create an array of output pins like [Pin<Output, Dynamic>; 12], and it is guaranteed that the pins are in output mode, so that the set_low and set_high methods are infallible.

rp2040-hal has the DynPin API but it isn't quite the same. It is dynamic over both the mode and the pin number, and there doesn't seem to be a way to make only one of those parameters dynamic. This comes with a couple disadvantages:

  • The pin mode isn't enforced at compile time. If I wanted to create an array of output pins like the previous example, the type would be [DynPin; 12] and it would not be a compile error to construct it with an expression like [pins.gpio2.into(), pins.gpio5.into(), ...], which is forgetting to set the pin states.

  • The is_low/is_high/set_low/set_high methods on DynPin are fallible and could return Err if the pin is in the wrong mode. This is not a big deal, as the error variants of InputPin and OutputPin APIs would have to be handled/ignored anyway even if they were infallible.
@9names
Copy link
Member

9names commented Sep 4, 2022

We derived our GPIO impl from atsamd, I think we should get those folks involved if we're adding a new type of Pin to the implementation, at the very least to get their opinion on naming and such.
https://github.com/atsamd-rs/atsamd/blob/master/hal/src/gpio/mod.rs

@ithinuel
Copy link
Member

ithinuel commented Sep 4, 2022

From the feedback and questions I've seen here and on our matrix channel, it seems like in addition to the fully typestate'd pin and the completely dyn-typed pins, we'd need:

  • 1 type that erases the identity of the pin while validating its state so that, for examples, arrays of input pins can be built.
  • 1 type that erases only the state of the pin while keeping its identity, so that a module can change its direction at runtime.

@ithinuel ithinuel mentioned this issue Apr 22, 2023
Merged
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules.
f3b3e65 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules.
29327df 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules.
bca414c 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules and related changes.
488dc92 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules and related changes.
f2f379b 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules and related changes.
8586634 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules and related changes.
00b669b 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules and related changes.
d3aaef3 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules and related changes.
db16d16 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
@ithinuel
Overhaul pin modules and related changes.
06db7b7 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 24, 2023
@ithinuel
Overhaul pin modules and related changes.
8422a7c 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556

# Conflicts:
#	rp2040-hal/src/gpio/mod.rs
ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 27, 2023
@ithinuel
Overhaul pin modules and related changes.
bcb8ece 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue May 4, 2023
@ithinuel
Overhaul pin modules and related changes.
2a25aec 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue May 16, 2023
@ithinuel
Overhaul pin modules and related changes.
601d01b 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556

# Conflicts:
#	rp2040-hal/src/spi.rs
ithinuel added a commit to ithinuel/rp-hal that referenced this issue May 16, 2023
@ithinuel
Overhaul pin modules and related changes.
45cf35b 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
@jannic jannic linked a pull request May 20, 2023 that will close this issue
Rework GPIO API and fix a few inconsistencies #585
Merged
ithinuel added a commit to ithinuel/rp-hal that referenced this issue May 20, 2023
@ithinuel
Overhaul pin modules and related changes.
478d12b 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue May 21, 2023
@ithinuel
Overhaul pin modules and related changes.
539ea83 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit to ithinuel/rp-hal that referenced this issue May 21, 2023
@ithinuel
Overhaul pin modules and related changes.
be0a95d 
- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: rp-rs#140, rp-rs#446, rp-rs#481, rp-rs#485, rp-rs#556
ithinuel added a commit that referenced this issue May 29, 2023
@ithinuel @thejpster @jannic
Rework GPIO API and fix a few inconsistencies (#585)
b087c0c 
* Leverage typelevel implementation in i2c, pwm (and a little bit in uart).

This notably makes the documentation more readable.

* Use an enum for core identification

* Overhaul pin modules and related changes.

- Added `AdcPin` wrapper to disable digital function for ADC operations
- Added `Sealed` supertrait to `PIOExt`
- Added pins to `Spi` to fix inconsistencies in gpio bounds in peripheral (i2c, uart, spi)
- Merge DynPin and Pin into Pin. The type class used in Pin now have a runtime variant allowing for
  the creation of uniform array of pins (eg: `[Pin<DynPinId, PinFnSio, PullDown>]`).
- Fix miss defined ValidPinMode bound allowing any Bank0 pin to be Xip and any Qspi pin to be any
  other function (except for clock).
- Use `let _ = ` to ignore result rather than `.ok();` as this gives a false sense the result is
  checked.

Addresses: #140, #446, #481, #485, #556

* Address first review round from @thejpster.

* Add a few inline to enable better optimisation.

* Expand documentation for the `Adc`

Co-authored-by: Jonathan 'theJPster' Pallant <[email protected]>

* Refactor the ADC module and remove redundant ts_en.set_bit()

- The channel types and their traits impl have been moved to the top of the
file and reassembled following the "Type -> impl Trait for Type" pattern.
- The temperature sensor bias was enabled twice, `enable_temp_sensor` and
  in the Adc::read function. The latter has now been removed because reading
  the Temp channel requires to call `enable_temp_sensor` anyway.

* Spacersssss :) and unicity -> uniqueness

* Add sio bypass when pin is in Sio Function

* Rename `DontInvert` to `Normal`

* Prevent the creation of multiple instances of TempSensor

* Add missing updates on on-target-tests

* rename `PullBoth` to the more accurate `PullBusKeep`

* Enable dyn-pin to be used with peripherals

* update deprecation notice.

* Add `try_into_function` for dynpin'ed Pins.

* Fix some doc warnings

* Implement `PinGroup`

* Update type-level documentation.

There was too many changes required to keep it in sync with the current
implementation. However, this is not required to understand the essence
of the type-level techniques. Therefore it is less maintenance costly to
forward the reader to the upstream of the idea where the documentation is
already excellent.

* rename `Pin::into()` to `Pin::into_typestate()` to avoid conflicts with `Into::into`

* Update rp2040-hal/src/gpio/mod.rs

Co-authored-by: Jan Niehusmann <[email protected]>

* Remove unused NoneT and fix AnyKind documentation's link

* Remove remaining occurences of "into_mode" in examples

* Enhance documentation

* doc: fix placeholder names

* Rename `into_typestate` to the more explicit `reconfigure`.

---------

Co-authored-by: Jonathan 'theJPster' Pallant <[email protected]>
Co-authored-by: Jan Niehusmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rework GPIO API and fix a few inconsistencies ithinuel/rp-hal
3 participants
@ithinuel @agausmann @9names