[bugreport] app_timer_stop() resets RTC1 counter when APP_TIMER_KEEPS_RTC_ACTIVE is set

[Keton]

My setup is as follows: nRF5 SDK 13.1 with s132 5.1.0 nRF52832_AA Keil 5.25pre

I use RTC1 counter for timestamping along with app_timer timers. My get_timestamp() function retrieves it's time value by calling app_timer_cnt_get() and then uses app_timer_cnt_diff_compute() to check how much time passed since it was called. Result is used to increment monotonous timer tick counter which is then processed to obtain timestamp:

  uint64_t get_timestamp()
  {
    static uint32_t last_ticks=0;
	static uint64_t timestamp_ticks=0;
	uint32_t curr_ticks=app_timer_cnt_get();

	timestamp_ticks+=app_timer_cnt_diff_compute(curr_ticks, last_ticks);
	last_ticks=curr_ticks;

	return timestamp_ticks;
  }

It is working well in few applications but I've noticed that when I use app_timer_stop() timestamp jumps by large value. It turns out that calling this function on last running timer resets RTC1 counter by calling timer_list_remove() which contains following code snippet:

// Timer is the first in the list
    if (p_previous == p_current)
    {
        mp_timer_id_head = mp_timer_id_head->next;

        // No more timers in the list. Reset RTC1 in case Start timer operations are present in the queue.
        if (mp_timer_id_head == NULL)
        {
            NRF_RTC1->TASKS_CLEAR = 1;
            m_ticks_latest        = 0;
            m_rtc1_reset          = true;
            nrf_delay_us(MAX_RTC_TASKS_DELAY);
        }
    }

IMHO this is a bug because setting APP_TIMER_KEEPS_RTC_ACTIVE=1 in sdk_config.h according to infocenter documentation should result in always running RTC1 which can be used 'for timestamping'. Introducing large and unpredictable jumps in counter values contradicts this.

As a workaround I've started always running recurring timer which does nothing. This works but it's not ideal since nRF will wake from sleep to process timeout events for this timer.

In attempt to have more reliable fix I've patched snippet above so it looks like this:

    // Timer is the first in the list
    if (p_previous == p_current)
    {
        mp_timer_id_head = mp_timer_id_head->next;

        // No more timers in the list. Reset RTC1 in case Start timer operations are present in the queue.
        if (mp_timer_id_head == NULL)
        {
//do not reset RTC counter when it's used for timestamp
#if (APP_TIMER_KEEPS_RTC_ACTIVE == 0)
            NRF_RTC1->TASKS_CLEAR = 1;
            m_ticks_latest        = 0;
            m_rtc1_reset          = true;
            nrf_delay_us(MAX_RTC_TASKS_DELAY);
#else
			m_rtc1_reset          = false;
#endif

        }
    }

I'd appreciate If someone from Nordic could take a look and check if this won't break something or propose different solution.

[MartinBL]

Hi,

The SDK team is aware of the vulnerability, but they haven't looked thoroughly into it because they have been working on an entirely new implementation of the library. The new implementation is scheduled to be included in SDK 15.

[Keton]

@hungbui I see that app_timer2 requires nrf_sortlist module which is not included in SDK 13.1. Should I use SDK14.2 one or will you provide SDK15 variant?

EDIT: also nrfx.h file is missing both from 13.1 and 14.2 EDIT2:

since nrfx.h is missing I've had to patch app_timer_rtc.c and enable nrf_drv_clock. Now my drv_rtc_init() and drv_rtc_uninit() look like this:

ret_code_t drv_rtc_init(drv_rtc_config_t const * p_config)
{
    ret_code_t err_code = nrf_drv_clock_init();

    if (err_code != NRF_SUCCESS)
    {
        return err_code;
    }

    nrf_drv_clock_lfclk_request(NULL);
    m_irq_func = p_config->irq_func;
    nrf_rtc_prescaler_set(NRF_RTC1, p_config->prescaler);
    NVIC_SetPriority(RTC1_IRQn, p_config->irq_priority);
    NVIC_EnableIRQ(RTC1_IRQn);
    return NRF_SUCCESS;
}

void drv_rtc_uninit(void)
{
    NVIC_DisableIRQ(RTC1_IRQn);
    drv_rtc_stop();
    nrf_drv_clock_lfclk_release();
}

Could you please check if that's correct approach or maybe I should adapt rtc1_init() / uninit functions?