periph/adc: Extended documentation

[maribu]

Contribution description

Extended/changed API documentation:

• Add preconditions to adc_sample()
• Point out that adc_res_t should only provide actually supported resolutions
• Point out that adc_sample() should use an assert() to verify the line parameter
• Changed error handling of adc_sample() on incorrect resolutions: This now should lead to an assert() blowing up rather returning -1

Testing procedure

• Check the output of the CI that the documentation is still correctly generated
• Check that the added/changed documentation is reasonable, easy to understand, and grammatically and orthographically correct

Issues/PRs references

None

[maribu]

Marking this as an "API change" because of stating that an assert() rather than returning -1 might be a bit of an overstatement. (Especially as callers still should check for negative values as error codes.) But better being to strict on this.

[maribu]

Also, because of "API change", 2 ACKs should be needed.

[maribu]

@benpicco recently opened a PR to add an ADC implementation. Maybe he could be the second reviewer?

[benpicco]

This is a good clarification of the ADC API.
It's good we mandate compile-time checks where possible.

[benpicco]

Since the resolutions are defined in an enum, we can't do #ifdef ADC_RES_16BIT, so how can we determine which resolutions are supported?

Either we have to change this to a #define or we need to come up with something else.

[gschorcht]

Either we have to change this to a #define or we need to come up with something else.

Might pseudomodules for each resolution or/and FEATURES_PROVIDED += adc_res_12 help?

[maribu]

After thinking a bit about it, I now think this is actually a bad idea. Sooner or later we will want to add support for external ADCs, or MCU with multiple heterogeneous ADCs.

[maribu]

But on the other hand the hole API cannot be extended in any reasonable way to support heterogeneous ADCs. E.g. with having different values for the same item in adc_res_t depending on MCU makes this utterly complex.

Maybe it is time to propose a different ADC API. This API makes it not only unnecessary hard to support multiple ADCs, but also to expose features like selecting the reference voltage, performing multiple conversions into a buffer via DMA, keeping the ADC online for fast conversions, etc.

[gschorcht]

Maybe it is time to propose a different ADC API. This API makes it not only unnecessary hard to support multiple ADCs.

In PR #10504 I started a try to change the ADC API where an ADC line would then be defined as a channel of an ADC device. Each device might then have different capabilities and configurations.

My plans were to resume my work in PR #10527 for an ADC extension API once the GPIO extension API in PR #12877 is on a good way. But at the moment, I'm not working on it.

If you start to think about a new ADC API, please have the use of ADC extenders in mind.

[gschorcht]

• Point out that adc_res_t should only provide actually supported resolutions

The test application uses a hardcadod resolution of ADC_RES_10BIT. Do you know whether all platforms support at least this resolution?

[gschorcht]

I guess that a number of platform implementations have to be changed to be consistent with the documentation. From what I know, ESPs have to be changed both.

[gschorcht]

@maribu Do you want to change the documentation and especially the handling of the supported resolutions after you have opened PR #13247? If so, we would have to adapt all ADC implementations.

issues with the API became apparent

[maribu]

Actually not. I personally consider the API broken beyond repair; that's why I opened the PR proposing an alternative...