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

AdcFifo not configurable with DynPin #741

Closed
jannic opened this issue Dec 22, 2023 · 4 comments · Fixed by #739
Closed

AdcFifo not configurable with DynPin #741

jannic opened this issue Dec 22, 2023 · 4 comments · Fixed by #739

Comments

@jannic
Copy link
Member

jannic commented Dec 22, 2023

To configure the channel used by AdcFifo, one has to call the following function on AdcFifoBuilder:

     pub fn set_channel<PIN: Channel<Adc, ID = u8>>(self, _pin: &mut PIN) -> Self {               

But for DynPin, Channel is defined with ID = (). Therefore, set_channel can't be with a DynPin as its parameter.

@ithinuel
Copy link
Member

DynPin can be "concretised" before being used in the AdcFifoBuilder.

@jannic
Copy link
Member Author

jannic commented Dec 24, 2023

Is it the intended way of using it?
OneShot has a separate implementation for that case, so read can be called directly on a DynPin. IMHO it would be more consistent if all functions were callable on the same set of types (as long as it's meaningful).

@ithinuel
Copy link
Member

As long as the required configuration is applied on the pin before the sampling and restored when the pin's no longer used as adc input, it's fine by me :)

@jannic
Copy link
Member Author

jannic commented Dec 27, 2023

As long as the required configuration is applied on the pin before the sampling and restored when the pin's no longer used as adc input, it's fine by me :)

The additional impl should still require an AdcPin<Pin<DynPinId, F, M>>, just with a where bound AdcPin<Pin<DynPinId, F, M>>: Channel<Adc, ID = ()> or similar. So the proper configuration should already be in place.

@jannic jannic linked a pull request Jan 3, 2024 that will close this issue
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.

2 participants