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: support for ADC extension API #10527

Closed
wants to merge 11 commits into from

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Nov 30, 2018

Contribution description

This is an implementation of the extension API as proposed in #9582 for ADC extensions. It corresponds to the implementation of the GPIO extension API in #9860 and #9958. The PR contains the following contributions:

  • Low level function prototypes adc_*_ll in drivers/include/periph.h.
  • Changes of adc_* functions to redirect a function call either to the low level functions adc_*_ll or to the adc_ext_* function provided by an ADC extension device driver.
  • New inline function adc_channels which returns the number of channels of a device, in case of MCU channels simply ADC_NUMOF. This new function makes it possible to get the information from a certain device how many channels it has and allows to iterate over the all channels of all devices.
  • Changes of low level function adc_*_ll for all CPUs that support ADC channels.
  • Boards that base on SAM0 (SAMD21, SAML21) MCU the ADC channel configuration adc_channels was renamed to adc_config to solve conflicts with the new function adc_channels.

The ADC extension API does not require feature periph_adc which makes it possible to extend boards that do not provide ADC capabilities by the MCU by external ADC modules.

Testing procedure

A test case is provided that implements a soft-driver for the ADC extension interface. The test case uses this driver to confirm that interception and redirection of the API call are working properly. This has been tested and is working properly on esp8266-esp-12x, esp32-wroom-32 and arduino-mega2560.

To test only the ADC extension API use

make  flash test -C tests/extend_adc BOARD=...

To test the ADC extension API together with internal ADC channels use

USEMODULE=periph_adc make  flash test -C tests/extend_adc BOARD=...

In that case feature periph_adc is required of course.

Issues/PRs references

Implements #9582
Corresponds to #9860 and #9958

The changes in the makefiles/pseudomodules.mk and in drivers/Makefile.include made to get it working are the same as in #9958. These changes might be removed once #9958 is merged.

@miri64 miri64 added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels Nov 30, 2018
@gschorcht gschorcht force-pushed the adc_ext_api branch 2 times, most recently from 9ce8525 to bb27217 Compare December 1, 2018 16:16
@ZetaR60 ZetaR60 self-assigned this Dec 6, 2018
adc_* functions are changed to be inline functions for redirection of calls either to low level or to extenstion API functions dependent on the line identifier. Macro definitions for extension API added. Function to get the number of ADC channels added.
adc_init and adc_sample functions renamed to adc_init_ll and adc_sample_ll
adc_channels renamed to adc_config to avoid conflicts with new adc_channels function
adc_channels renamed to adc_config to avoid conflicts with new adc_channels function
contains the extension redirection and not support function device
@gschorcht
Copy link
Contributor Author

gschorcht commented Dec 16, 2018

@ZetaR60 I have tried the approach you proposed in #9582 (comment) for the ADC extension API in a separate branch. This branch creates the defines ADC_NUMOF_LL, ADC_LINE to and ADC_EXT_LINE in drivers/include/periph/adc.h

#ifndef ADC_NUMOF_LL
#define ADC_NUMOF_LL        (0U)
#endif

#define ADC_NUMOF           (ADC_NUMOF_LL + ADC_EXT_NUMOF)

#define ADC_LINE(x)         (x < ADC_NUMOF_LL \
                             ? ADC_LINE_LL(x) \
                             : ADC_EXT_LINE(x - ADC_NUMOF_LL))

and an example of an ADC extension configuration with 4 ADC extension devices as following:

#define ADC_EXT_DEV0_NUMOF  (4)
#define ADC_EXT_DEV1_NUMOF  (2)
#define ADC_EXT_DEV2_NUMOF  (8)
#define ADC_EXT_DEV3_NUMOF  (0)

#define ADC_EXT_DEV0_OFFSET (0)
#define ADC_EXT_DEV1_OFFSET (ADC_EXT_DEV0_OFFSET + ADC_EXT_DEV0_NUMOF)
#define ADC_EXT_DEV2_OFFSET (ADC_EXT_DEV1_OFFSET + ADC_EXT_DEV1_NUMOF)
#define ADC_EXT_DEV3_OFFSET (ADC_EXT_DEV2_OFFSET + ADC_EXT_DEV2_NUMOF)

#define ADC_EXT_NUMOF       (ADC_EXT_DEV0_NUMOF + \
                             ADC_EXT_DEV1_NUMOF + \
                             ADC_EXT_DEV2_NUMOF + \
                             ADC_EXT_DEV3_NUMOF)

#define ADC_EXT_LINE(x)     (x < ADC_EXT_DEV1_OFFSET \
                             ? ADC_EXT_LINE_REV(0, (x - ADC_EXT_DEV0_OFFSET)) \
                             : (x < ADC_EXT_DEV2_OFFSET \
                                ? ADC_EXT_LINE_REV(1, (x - ADC_EXT_DEV1_OFFSET)) \
                                : (x < ADC_EXT_DEV3_OFFSET \
                                   ? ADC_EXT_LINE_REV(2, (x - ADC_EXT_DEV2_OFFSET)) \
                                   : ADC_EXT_LINE_REV(3, (x - ADC_EXT_DEV3_OFFSET)))))

With this approach, it is indeed possible to iterate from 0 to ADC_NUMOF-1 and to access onboard ADC channels and ADC extension channels with a common ADC_LINE(i) macro in subsequent order. But, I think the price we have to pay by the definition of the complex ADC_EXT_LINE macro is quite high.

If you would like to take a look you will find the code at https://github.com/gschorcht/RIOT-Xtensa-ESP/tree/adc_ext_api_numof_ll_change. If you prefer, I could also provide it as a second PR for discussion?

@ZetaR60
Copy link
Contributor

ZetaR60 commented Dec 16, 2018

@gschorcht I have attempted to address this in #9582

@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@gschorcht gschorcht added the State: waiting for other PR State: The PR requires another PR to be merged first label Aug 10, 2019
@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@stale
Copy link

stale bot commented Feb 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added State: stale State: The issue / PR has no activity for >185 days and removed State: stale State: The issue / PR has no activity for >185 days labels Feb 11, 2020
@gschorcht
Copy link
Contributor Author

This PR is closed in favor of PR #13247 which should allow to integrate external ADC devices.

@gschorcht gschorcht closed this Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers State: waiting for other PR State: The PR requires another PR to be merged first Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants