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

API ergonomics for ADC free running mode when not using FIFO #731

Closed
jlpettersson opened this issue Dec 19, 2023 · 2 comments · Fixed by #739
Closed

API ergonomics for ADC free running mode when not using FIFO #731

jlpettersson opened this issue Dec 19, 2023 · 2 comments · Fixed by #739

Comments

@jlpettersson
Copy link
Contributor

It is great that we now have ADC free running mode! I was using it in a small project, but I find that the API ergonomics could be improved.

When I want to use ADC free running mode without FIFO currently, I had to configure it and use it as follows (from the example in hal source code):

let mut adc_fifo: AdcFifo = adc.build_fifo().set_channel(&mut adc_pin).start();

loop {
    do_something_timing_critical();

    // read the most recent value:
    if adc_fifo.read_single() > THRESHOLD {
       led.set_high().unwrap();
    } else {
       led.set_low().unwrap();
    }
}

For me, it is a bit confusing to use so much "fifo"-named types and methods when I only want to use free running mode without FIFO.

How about if we could configure ADC free running mode separate from FIFO configuration? e.g. something like below:

let mut adc_fr: AdcFreeRunning = adc.configure_free_running().set_channel(&mut adc_pin).start();

loop {
    do_something_timing_critical();

    // read the most recent value:
    if adc_fr.read_most_recent() > THRESHOLD {
       led.set_high().unwrap();
    } else {
       led.set_low().unwrap();
    }
}

The benefit here is that it is a bit more explicit of what I wanted, ADC free running mode but no FIFO involved.

It would also be good if I could "add" a FIFO to that free running mode, in some way.

@jannic
Copy link
Member

jannic commented Dec 19, 2023

What about just renaming things?
Looking at this comment, the function might as well be named free_running():

    /// Start configuring free-running mode, and set up the FIFO
   [...]
    pub fn build_fifo(&mut self) -> AdcFifoBuilder<'_, u16> {

As it might be useful to use free-running mode without the FIFO, but not the other way around, that may actually be the better choice.

@jlpettersson
Copy link
Contributor Author

jlpettersson commented Dec 20, 2023

What about just renaming things?

yes, I think that could be a good alternative.

As it might be useful to use free-running mode without the FIFO, but not the other way around, that may actually be the better choice.

Yes, that is my use case. Although the FIFO gets activated anyway, but that is a small detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants