ADC stream API #90285
ADC stream API #90285
Conversation
github-actions
bot
added
area: ADC
github-actions
bot
requested review from
57300,
anangl,
ceolin,
dcpleung,
galak,
jeremybettis,
MaureenHelm,
microbuilder,
nashif,
nordicjm,
ozersa,
tejlmand,
ttmut and
yasinustunerg
|
The following west manifest projects have changed revision in this Pull Request:
⛔ DNM label due to: 1 project with PR revision Note: This message is automatically posted and updated by the Manifest GitHub Action. |
github-actions
bot
added
manifest
manifest-hal_adi
DNM (manifest)
vladislav-pejic
force-pushed
the
adc_stream_api
branch
from
1a80178 to
47e59e2
Compare
MaureenHelm
requested review from
teburd,
ubieda and
yasinustunerg
and removed request for
yasinustunerg and
dcpleung
vladislav-pejic
requested review from
ttmut,
ubieda,
MaureenHelm,
yperess and
teburd
vladislav-pejic
force-pushed
the
adc_stream_api
branch
from
dfcf1e1 to
b3b4f64
Compare
MaureenHelm
added
the
Architecture Review
|
Architecture WG meeting:
|
| #include <zephyr/dsp/print_format.h> | ||
|
|
||
| /* ADC node from the devicetree. */ | ||
| #define ADC_NODE DT_ALIAS(adc0) |
There was a problem hiding this comment.
Please namespace stuff in the sample, prefixing something ADC_ or DT_ will quickly clash with something in adc.h or devicetree.h, DT_SPEC_AND_COMMA even sounds like an "official" DT macro :) namespacing/prefixing with SAMPLE_ is quite effective, so SAMPLE_ADC_NODE for example
| #define ADC_TRIGGERS \ | ||
| {ADC_TRIG_FIFO_FULL, ADC_STREAM_DATA_INCLUDE}, \ | ||
| {ADC_TRIG_FIFO_WATERMARK, ADC_STREAM_DATA_INCLUDE} |
There was a problem hiding this comment.
A comment here describing what each element is and means would be helpful
|
|
||
| ADC_DT_STREAM_IODEV(iodev, ADC_NODE, adc_channels, ADC_TRIGGERS); | ||
|
|
||
| RTIO_DEFINE_WITH_MEMPOOL(adc_ctx, 20, 20, 20, 512, sizeof(void *)); |
There was a problem hiding this comment.
A comment as to how the mempool is used could be added here, along with the rational behind the hardcoded sizes and use of void *
| adc_stream(local_iodev, &adc_ctx, NULL, &handles); | ||
|
|
||
| while (1) { | ||
| cqe = rtio_cqe_consume_block(&adc_ctx); |
There was a problem hiding this comment.
a quick description of what an CQE is, where its being created from, and what it contains of data would be helpful
| return rc; | ||
| } | ||
|
|
||
| const struct device *adc = dev; |
There was a problem hiding this comment.
this seems redundant, there is only one ADC in this sample right? just name the argument of the print_adc_stream adc in the first place :)
| read_cfg->sequence = &sequence; | ||
|
|
||
| init_adc(); | ||
| (void)adc_sequence_init_dt(&adc_channels[0], &sequence); |
There was a problem hiding this comment.
why ignore the return? a user would be expected to check it normally right?
|
|
||
| ret = print_adc_stream(adc_channels[0].dev, &iodev); | ||
|
|
||
| return ret; |
There was a problem hiding this comment.
main must return 0
| init_adc(); | ||
| (void)adc_sequence_init_dt(&adc_channels[0], &sequence); | ||
|
|
||
| ret = print_adc_stream(adc_channels[0].dev, &iodev); |
There was a problem hiding this comment.
unless you print something based in the ret value, it can be removed as it is unused :)
vladislav-pejic
force-pushed
the
adc_stream_api
branch
from
b3b4f64 to
02c8b3b
Compare
vladislav-pejic
force-pushed
the
adc_stream_api
branch
from
02c8b3b to
b211511
Compare
Introduce a streaming APIs for ADC devices. Two new APIs are added to the adc_driver_api: submit and get_decoder. Added decoder following APIs: get_frame_count, get_size_info, decode, has_trigger. Supported triggers are: - ADC_TRIG_DATA_READY - ADC_TRIG_FIFO_WATERMARK - ADC_TRIG_FIFO_FULL Supported operations to be done on trigger: - include - whatever data is associated with the trigger - nop - do nothing with data associated with the trigger - drop - clear data associated with the trigger Some changes to the linker scripts were needed to add decoder APIs. Signed-off-by: Vladislav Pejic <vladislav.pejic@orioninc.com>
Fix for compatible property. Addition of hardware timer to be used with APARD32690 board. Signed-off-by: Vladislav Pejic <vladislav.pejic@orioninc.com>
Added support for RTIO stream.
Also added sampling_period property to the DTS. This is for setting the
sampling period when streaming is used.
Hardware or kernel timer can be
used for this.
To use hardware timer you need to add it to the DTS. Example:
{
chosen {
zephyr,adc-clock = &counter0;
};
};
Signed-off-by: Vladislav Pejic <vladislav.pejic@orioninc.com>
Updated MAX32 driver with RTIO stream functionality. Signed-off-by: Dimitrije Lilic <dimitrije.lilic@orioninc.com>
Fix for gpio-map-mask and gpio-map-pass-thru to enable use of MAX32_GPIO_VSEL_VDDIOH and other defines with arduino_header connector. Signed-off-by: Vladislav Pejic <vladislav.pejic@orioninc.com>
Add sample application to demonstrate new ADC stream APIs. Two tests are also added: - ad4052-stream - max32-stream Signed-off-by: Vladislav Pejic <vladislav.pejic@orioninc.com>
vladislav-pejic
force-pushed
the
adc_stream_api
branch
from
b211511 to
e4b654d
Compare
Thank you for the feedback. I added comments and corrected everything that you have suggested. Can you check if the changes are ok? |
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
Addition of streaming APIs for ADC devices that enables them to stream data to the application in more efficient way then it is currently possible.
Our goal was to provide a way to continuously stream data from ADC. We used similar approach as sensor stream API implementation.
On the user side adc_steam and adc_get_decoder APIs are added while on the driver side adc_driver_api is extended with submit and get_decoder.
Following decoder APIs are also added: get_frame_count, get_size_info, decode and has_trigger. Result of decode API should be in V in q31_t format.
Supported triggers:
There are 3 supported operations on data associated with trigger:
In this PR there are two driver implementations that are demonstrating use of streaming APIs with RTIO.
Signed-off-by: Vladislav Pejic vladislav.pejic@orioninc.com