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

Tracking issue for ADC trait #377

Open
eldruin opened this issue Apr 13, 2022 · 6 comments
Open

Tracking issue for ADC trait #377

eldruin opened this issue Apr 13, 2022 · 6 comments

Comments

@eldruin
Copy link
Member

eldruin commented Apr 13, 2022

The ADC traits (adc::Channel and adc::OneShot) available on the 0.2.x versions have been removed ahead of the 1.0.0 release. See: #376
This issue servers as a tracking issue until we add them back/reject them.

The reasons were:

  • The traits are quite complicated to use in generic code.
  • It seems users would be better served by inherent methods.
  • Also, these traits are nb-only.

ADC traits as of the 0.2.7 release:

Relevant issues/PRs:

Please feel free to participate, highlight your current use cases, problems and provide solutions.

@ryankurte
Copy link
Contributor

ryankurte commented Apr 28, 2022

the updated spi traits have me wondering if a similar approach would suit for ADCs... something like a shared AdcDevice that is split into separate channels implementing AdcChannel which drivers then take in the same manner as digital pins or any other injected type.

ADC peripherals would seem to have very similar constraints as sharing as a bus; one ADC peripheral with multiple channels that may be used in different places, and a need to be able to take ownership of the peripheral for more complex behaviors like interleaving or DMA.

(i think this is also relevant to our previous timer / delay traits, though these do not always need to be exclusive you also often need to be able to duplicate delays or split timers into multiple drivers)

@Dirbaio
Copy link
Member

Dirbaio commented Apr 28, 2022

Maybe having a trait for an "ADC device" is not really needed? We could just have this

trait AdcChannel
    fn read(&mut self) -> Result<u32, Error>;
}

If the hardware has a single multi-channel ADC it's up to the implementors to do the "sharing" for it. Via RefCell, Mutex, or whatever. Or it could be even a noop for MCU peripheral ADCs: hit the registers directly from read() and ensure the channels are !Send so that the user can't hit the ADC concurrently from multiple threads (irq priority levels).

need to be able to take ownership of the peripheral for more complex behaviors like interleaving or DMA.

I don't think making traits for these is feasible, they're incredibly hardware dependent...

@ryankurte
Copy link
Contributor

Maybe having a trait for an "ADC device" is not really needed? We could just have this

what i'm mostly wondering is whether this is a consistent problem with spi / i2c / adc / etc. that we can extract in a consistent way, but yeah seems to me like AdcChannel could be a good start.

I don't think making traits for these is feasible, they're incredibly hardware dependent...

agreed, but the in the spi model you can still take exclusive ownership of the bus and do whatever is needed via the closure based approach.

@eldruin
Copy link
Member Author

eldruin commented May 11, 2022

A complication in the case of ADCs is that some channel combinations can be configured to work in comparison mode, where the measurements taken correspond to the difference between two (otherwise-independent) channels.
See the ads1x1x driver for an example.

@crjeder
Copy link

crjeder commented Jul 14, 2022

trait AdcChannel
    fn read(&mut self) -> Result<u32, Error>;
}

I'd strongly vote for a read() interface for ADC as this would abstract away how the ADC is connected. Could be a GPIO or via SPI.

@Ben-PH
Copy link

Ben-PH commented May 28, 2024

I would like to propose making the result type generic. Perhaps the vast majority of the time it's a u32, but making it such that it can be any unsigned would make this trait more universally applicable.

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

No branches or pull requests

5 participants