This post is older than 2 years and might not be relevant anymore
More Info: Consider searching for newer posts

v12.2.0 SDK ADC bug: Size should be uint16_t and not uint8_t

Setting up an example of the problem

As I promised in this thread, I have been testing out the ADC on the nRF51822 and trying to see how quickly I can sample per minute vs how many interrupts I trigger per minute. This is because the nRF51288 is the chipset that I have available and it does not have the SAADC features that the nRF52 series has.

I have submitted the code to BitBucket so that I can keep track of my changes and I have created this branch to expose the problem: robertmassaioli/ble_app_uart_adc_scan_mode:exposing-v12.2.0-sdk-problem

There are many cosmetic changes in that branch but the TL;DR is this:

  • No matter how fast or slow the ADC is set up to run there should be one BLE UART message sent per second and a printf should happen to the log at the same time.
  • I have set the branch up to have an ADC buffer with 300 slots. Every sample run we are sampling three enabled ADC channels. This means that in 100 sample runs we should hit the DONE ADC event.
  • A sample run is triggered once every 500us (not ms but us); I sped it up from the base example. This means that the DONE event should be hit once every 50ms. Which means that there should be 20 DONE events every second.

These details will be important in understanding the problem.

The problem

When I compile this code and put it on the default 12.2.0 SDK this is what I see in the logs:

UART Start - with ADC !
  adc event counter: 20 (44) [0]
  adc event counter: 40 (44) [0]
  adc event counter: 60 (44) [0]
  adc event counter: 80 (44) [0]
  adc event counter: 100 (44) [0]
  adc event counter: 120 (44) [0]
  adc event counter: 140 (44) [0]
  adc event counter: 160 (44) [0]
  adc event counter: 180 (44) [0]
  adc event counter: 200 (44) [0]
  adc event counter: 220 (44) [0]
  adc event counter: 240 (44) [0]
  adc event counter: 260 (44) [0]
  adc event counter: 280 (44) [0]
  adc event counter: 300 (44) [0]

Just so that you understand, this is the printf line that prints these logs:

printf("  adc event counter: %lu (%u) [%d]\r\n", 
    adc_event_counter, 
    p_event->data.done.size, 
    p_event->data.done.size > 255u
);

As you can see:

  • The first number represents how many DONE events have been triggered since the last device reset.
  • The second number details how many samples are in the done buffer.
  • The third number is a boolean that represents whether or not the number of samples is larger than what a uint8_t can represent.

Thus, looking at logs from the code sample it looks like our expected 300 samples are being truncated down to 44 samples in a manner that looks like uint8_t integer overflow because:

300 - 256 = 44

This is the problem. I'm only getting back 44 samples instead of 300.

Should you be able to get back that many samples?

According to the documentation, I think that I should be able to get that many samples back. For example, if we look at the definition of nrf_drv_adc_buffer_convert we see:

ret_code_t nrf_drv_adc_buffer_convert(nrf_adc_value_t * buffer, uint16_t size)

As you can see, the size argument is a uint16_t. This is also true for the size field of the nrf_drv_adc_done_evt_t struct.

In short, it looks like the exposed SDK API's expect the size field to be a uint16_t but that, internally, something is converting this to a uint8_t.

The solution

As a result, I quickly jumped into the SDK and had a look at the components/drivers_nrf/adc/nrf_drv_adc.c file. In this file there was a very suspicious looking struct:

typedef struct
{
    nrf_drv_adc_event_handler_t event_handler;
    nrf_drv_adc_channel_t     * p_head;
    nrf_drv_adc_channel_t     * p_current_conv;
    nrf_adc_value_t           * p_buffer;
    uint8_t                     size;
    uint8_t                     idx;
    nrf_drv_state_t             state;
} adc_cb_t;

Looking at the size and idx field types, and how there were used in the code, these looked like an obvious cause of this problem. When I simply just changed this struct to look like this instead:

typedef struct
{
    nrf_drv_adc_event_handler_t event_handler;
    nrf_drv_adc_channel_t     * p_head;
    nrf_drv_adc_channel_t     * p_current_conv;
    nrf_adc_value_t           * p_buffer;
    uint16_t                     size;
    uint16_t                     idx;
    nrf_drv_state_t             state;
} adc_cb_t;

And recompiled / reflashed the code for my nRF51822 then these are the logs that I recieved:

UART Start - with ADC !
  adc event counter: 20 (300) [1]
  adc event counter: 40 (300) [1]
  adc event counter: 60 (300) [1]
  adc event counter: 80 (300) [1]
  adc event counter: 100 (300) [1]
  adc event counter: 120 (300) [1]
  adc event counter: 140 (300) [1]
  adc event counter: 160 (300) [1]
  adc event counter: 180 (300) [1]
  adc event counter: 200 (300) [1]
  adc event counter: 220 (300) [1]
  adc event counter: 240 (300) [1]
  adc event counter: 260 (300) [1]
  adc event counter: 280 (300) [1]
  adc event counter: 300 (300) [1]

Perfect! Exactly what I expected. I would like to submit this as a bug report to the SDK. If this is not the right place to do so then please let me know where the right place to raise SDK bugs is. Cheers.

Note: This does not seem to affect v13 of the SDK because it does not seem to have an ADC module; only an SAADC module. And the SAADC module does not "obviously" seem to display the problem. But a Nordic employee would probably be wise to check the SAADC module in v12 and v13 too. Cheers.

Related