Incorrect use of app_timer_create in nrf_serial.c when used with FreeRTOS

[Austin]

The file components/libraries/serial/nrf_serial.c in nRF5 SDK is used for UART serial tx/rx and has support for timeouts.  There appears to be a bug with use of timeouts.

When the nrf_serial_write() function is called with a non-zero timeout, app_timer_create() is called for each write.  While this is not an issue for a non-FreeRTOS system (since it's possible to call app_timer_create multiple times for the one timer), this results in a failure when compiled with FreeRTOS.  app_timer_create should only be called once during initialisation of the nrf_serial module, not repeatedly for every write.

The attached patch makes the following modifications:

• Create UART timer once only when nrf_serial is initialised.  All calls to nrf_serial_write() will reuse the time created during initialisation.
• Use correct #defines when choosing whether to use easyDMA.  #define values modified to match the #defines used by the nrfxxx_peripherals.h headers.

These bugs exist in nRF5 SDK 15.0.0.  The UART timer bug also exists in at least nRF5 SDK 14.2.0 and possibly in earlier releases.

nrf_serial.c.patch

[Susheel Nuguru]

Hi Austin,

Good point, but all the libraries that are needs an rtos port are postfixed with the RTOS name. For example app_timer_freertos.c or app_timer_rtx.c.

In other words, nrf_serial was designed to be standalone. It just happened to be that only one restriction came forward with nrf_serial conflicting freertos with the use of app_timer_create function, but with other libraries there could be others.

Also the patch you attached is not correct as it shows already existing code in SDK15

[Austin]

Hi Aryan,

Thank you for your response.  It's my understanding that the app_timer interface should be usable by other modules such as nrf_serial, irrespective of whether the module is running on FreeRTOS or using the SDK scheduler.  The app_timer APIs appear to have been carefully designed to be equal across different RTOS/schedulers.

Irrespective, calling app_timer_create() for each UART write seems like a bug whether running on FreeRTOS or using the SDK scheduler.  It just so happens that when running the SDK scheduler, the app_timer.c implementation allows for app_timer_create to be called multiple times on the one timer; this doesn't seem like the intent of the timer interface.

I haven't tested my patch using app_timer.c, but I believe this patch should work for all implementations of app_timer_xxx.c.

This patch was generated against SDK15.

[Einar Thorsrud]

Hi,

Thank you for providing this patch. I will forward it to R&D so that they can consider it for a future SDK release.