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

Provide a way to construct a DynPin (for C interop) #481

Closed
9names opened this issue Oct 23, 2022 · 2 comments · Fixed by #585
Closed

Provide a way to construct a DynPin (for C interop) #481

9names opened this issue Oct 23, 2022 · 2 comments · Fixed by #585

Comments

@9names
Copy link
Member

9names commented Oct 23, 2022

cr1901 asked on #rust_embedded if there was a way to construct a pin for use with C code.
As in: existing C project. new code written in Rust, assuming it's called through an extern "C" fn

We're almost able to construct a DynPin from scratch now, but the DynRegisters struct is private.

Here's an example of the almost working syntax we currently have:

use embedded_hal::digital::v2::ToggleableOutputPin;
use hal::gpio::*;
use hal::gpio::DynGroup::Bank0;
use hal::gpio::DynOutput::PushPull;

extern "C" fn do_work(pin_id: u8) {
    let pin = DynPinId {
        group: Bank0,
        num: pin_id,
    };
    let mode = DynPinMode::Output(PushPull);
    // This doesn't work because DynPin::new() is private
    let mut dynpin1 = unsafe { DynPin::new(pin, mode) };
    // Panic if the pin wasn't in the right mode
    dynpin1.toggle().unwrap();
}

If we were to provide such a facility (and I think we should) how should we enable this?
Should we add an (unsafe) function that allows you to conjure a DynPin based solely on pin number and mode, along with documentation on how it will explode if you get these wrong?
Would appreciate any thoughts on whether we should do this, and how.

See the below draft PR for a simple impl

@cr1901
Copy link

cr1901 commented Oct 23, 2022

For more context: the idea is that the C code provides a header that defines the pin layout. I will parse this on the Rust side to create structs for appropriately initializing the used hardware on the Rust side (and provide a context pointer back to the C side after init is done).

Since there's no real way to enforce this on the C side, it's up to the C side to not call hw init functions that Rust is using.

@jannic
Copy link
Member

jannic commented Oct 23, 2022

Without commenting on the implementation details, as I didn't have time to read the code yet: l think this is a valid use case and should be supported somehow. If it's only possible with some (unsafe) function which crates the DynPin out of thin air, so be it.

Are there alternatives? What about some container storing the real DynPins on the rust side, and handling the pin id as some kind of handle? That way C code could refer to the pin by its id, but could still only access pins which were previously registered in that structure. But that's probably just overcomplicating the issue.

ithinuel added a commit to ithinuel/rp-hal that referenced this issue Apr 22, 2023
- 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
- 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
- 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
- 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
- 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
- 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
- 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
- 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
- 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
- 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
- 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
- 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
- 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
- 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
- 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
ithinuel added a commit to ithinuel/rp-hal that referenced this issue May 20, 2023
- 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
- 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
- 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
* 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 <github@thejpster.org.uk>

* 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 <jan@gondor.com>

* 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 <github@thejpster.org.uk>
Co-authored-by: Jan Niehusmann <jan@gondor.com>
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

Successfully merging a pull request may close this issue.

3 participants