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

periph/adc: add adc_continuous_sample_multi() #20619

Closed
wants to merge 5 commits into from

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Apr 24, 2024

Contribution description

When collecting multiple samples, re-configuring the ADC lines is still too slow to keep up with the ADC sampling frequency.

Instead, add a new API function that allows to supply a buffer in which the samples from a single ADC line will be stored. This would be used with DMA in the future too.

Here setting the ADC frequency statically at compile time (and for all ADC lines) is no longer sufficient, so I extended adc_continuous_begin() with a frequency parameter.
This was added in #20077, not sure if anyone is using it yet - I could add a compat API wrapper instead.

In our application, we want to sample two ADC lines for Maximum Power Point Tracking of a solar cell. Here we set a voltage level to which the solar cell will be discharged, depending on the solar intensity the time until the solar cell recharged will vary. If we discharge it too deep, there is a lot of dead time before until the next discharge period starts.
If we discharge it too lightly, we leave power on the table.

So the idea is to measure both solar current and solar voltage to integrate the power level at different discharge voltage points to find the optimum.

This is still not possible with just sampling a single ADC line to a buffer, but SAM D5x/E5x has two ADC instances, so if the lines are on two instance we can sample them at the same time.

The hardware provides a host/client mode where ADC1 can be controlled in lockstep with ADC0 so both collect a sample at the same time.

I added an API for that too.

Testing procedure

No integration in the test app yet, but I can produce some nice graphs of the sample data with Gnuplot:

too much discharge

5800 dat

max power point

5c00 dat

Issues/PRs references

@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: cpu Area: CPU/MCU ports labels Apr 24, 2024
@benpicco benpicco requested a review from maribu April 24, 2024 17:28
@maribu
Copy link
Member

maribu commented Apr 24, 2024

Maybe the API could be a bit more generic. This API is tailored quite specifically towards the "two ADCs" use case.

If a single ADC instead could be set up to do round robin sampling for a number of channels with DMA, this would still be a useful feature to expose. And this likely would also work with 3, 4, ... n channels in round robin fashion.

Maybe something like:

/**
 * @brief Sample multiple ADC lines in parallel / round robin
 * @param lines ADC lines to sample
 * @param lines_numof Number of ADC lines to sample
 * @param buf Memory buffer to write to
 * @param buf_size Length of @p buf in bytes
 *
 * @retval 0 Success
 * @retval -ENOTSUP Configuration not supported by the driver
 * @retval -EINVAL Invalid configuration (e.g. alignment of buffer, size of buffer)
 */
int adc_continuous_sample_multi(const adc_t *lines, size_t lines_numof, uint16_t *buf, size_t buf_size);

/**
 * @brief Extract a sample from the buffer
 * @pre The buffer was written to using @ref adc_continuous_sample_multi
 * @param lines ADC lines to sample
 * @param lines_numof Number of ADC lines to sample
 * @param buf Memory buffer to extract the same from
 * @param line_idx Index of the ADC line in @p lines to extract the same of
 * @param sample_idx Index of the sample to extract
 * @return The sample number @p sample_idx of adc line `lines[line_idx]`
 */
uint16_t adc_sample_access(const adc_t *lines, size_t lines_numof, const uint16_t *buf, size_t line_idx, size_t sample_idx);

The idea is that for round-robin sampling the DMA may want to write them also in round-robin fashion. But if two (or more) ADCs are read in parallel, the single buffer could be split into two (or more) buffers that can be independently be filled. The adc_sample_access() function could be used to abstract away the details in which order the samples are in the buffer.

@benpicco
Copy link
Contributor Author

I'm starting to wonder if it even makes sense to couple this with adc_continuous (which is a but of a hack anyway), might as well just call it adc_sample_multi().

What's the motivation for the adc_sample_access() function? I would expect it's always possible to use different buffers for different ADC lines, can't think of a reason why they would be e.g. interleaved.

There is the question if it makes sense to only use a byte per sample for ≤ 8 bit samples though.

@maribu
Copy link
Member

maribu commented Apr 25, 2024

I think it is quite possible to have it interleaved. E.g. the DMA may not know anything about the ADC config, but just gets fed the ADC samples without context knowledge and places them in the order it got them into RAM.

The ADC on the other hand may be configured to do round robin sampling.

I can check some datasheets to confirm this does indeed exists in the real world and not only in my head ;)

@MrKevinWeiss MrKevinWeiss added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Apr 25, 2024
@github-actions github-actions bot removed the Area: tests Area: tests and testing framework label Apr 25, 2024
@benpicco
Copy link
Contributor Author

I think it's better to contain this into one function, since the PR description now no longer fits, I'd rather create a new one: #20622

@benpicco benpicco closed this Apr 25, 2024
@maribu
Copy link
Member

maribu commented Apr 25, 2024

Interesting. It seems that on Kinetis (and almost certainly also on RP2040) you can use the DMA to fetch one (or more samples) from the DMA with the first ADC channel selected. That DMA channel triggers another DMA channel to write to the ADC registers, switching the ADC channel, which triggers the first DMA channel to fetch the ADC samples.

So it seems to indeed be possible to end up with interleaved samples with real hardware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Platform: ARM Platform: This PR/issue effects ARM-based platforms Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants