Running timer with timeout >= 10ms makes next timer run trigger immediately

[maximkulkin]

I'm developing an IR remote library and using Frame Rate Control timer (FRC1, implemented by functions from esp/timer.h) to time IR pulses. The problem I have is: when I have a pulse of 10ms or more (so I set timer for >=10ms and thus it uses TIMER_CLKDIV_256) I observe a pulse that follows this long pulse to not appear, meaning the next pulse timer (which supposedly uses TIMER_CLKDIV_16) triggers immediately.

I haven't done explicit tests for that, but the effect is pretty stable: if I use several small timer runs (< 10ms), everything works fine. Given the long pulse timing actually works, I feel like I am (and esp/timer.c) missing some kind of cleanup/reinitialization.

Also, I have checked ESP8266_RTOS_SDK, they always use CLKDIV_16.

[maximkulkin]

Somewhat stripped down code I use for pulse generation

static void hw_timer_init(void (*isr)(void*), void *arg) {
    timer_set_interrupts(FRC1, false);
    timer_set_run(FRC1, false);

    _xt_isr_attach(INUM_TIMER_FRC1, isr, arg);

    timer_set_interrupts(FRC1, true);
}

static inline void hw_timer_pause() {
    timer_set_run(FRC1, false);
}

static inline void hw_timer_arm(uint32_t us) {
    timer_set_timeout(FRC1, us);
    timer_set_run(FRC1, true);
}

static void IRAM timer_handler(void *encoder) {
    hw_timer_pause();

    int16_t pulse = get_next_pulse(encoder);
    if (pulse == 0) {
        // Done with transmission
        clr_carrier();
        return;
    }

    if (pulse > 0) {
        gen_carrier();
    } else {
        clr_carrier();
    }
    hw_timer_arm(abs(pulse));
}

[ourairquality]

Have you included timer_set_reload(FRC1, false), and might that help? Perhaps there is something not understood when changing the clock rates etc.

Could you try a different approach and not parse the timer, rather let it reload to the max and reload the value in the handler, assuming it will not have time to trigger again.

You might also be interested in extras/mactimer/ which might better support timing the pulses including the faster modulation pulses. See extras/tsoftuart for a software uart output example using this.

[maximkulkin]

I do not actually want to take a different approach, this approach seem to be perfectly fine and pretty lightweight. Worst case I can workaround it by splitting long periods into periods shorter than 10k.

I finally assembled a test code which runs a bunch of timers and measures time it took

#include <stdio.h>
#include <esp/timer.h>
#include <esp/uart.h>
#include <espressif/esp_system.h>

uint16_t timers[] = { 9999, 500, 1500, 1000, 100 };
uint16_t measurements[10];

uint8_t count = 0;

static void IRAM print_measurements() {
  printf("Measurements: ");
  for (uint8_t i=1; i<count; i++) {
    printf("%4d ", measurements[i] - measurements[i-1]);
  }
  printf("\n");
}

static void IRAM timer_handler(void *arg) {
  timer_set_run(FRC1, false);

  measurements[count] = sdk_system_get_time();
  if (count > sizeof(timers) / sizeof(timers[0])) {
    print_measurements();
    return;
  }

  timer_set_timeout(FRC1, timers[count++]);
  timer_set_run(FRC1, true);
}

void user_init(void) {
    uart_set_baud(0, 115200);

    printf("Starting test\n");

    timer_set_interrupts(FRC1, false);
    timer_set_run(FRC1, false);

    _xt_isr_attach(INUM_TIMER_FRC1, timer_handler, NULL);
    timer_set_interrupts(FRC1, true);

    count = 0;
    measurements[0] = sdk_system_get_time();
    timer_set_timeout(FRC1, timers[count++]);
    timer_set_run(FRC1, true);
}

Here is output:

Measurements: 10033  507 1507 1007  107

Notice first 9999 in timers array. If you have anything less than 10000, it measures all periods pretty close. But if you change 9999 to 10000, the second period (500) is always ~7 which I believe is what it means for timer to immediately trigger:

Measurements: 10030    8 1508 1007  107

[ourairquality]

Declare count volatile. Try the following for the interrupt function, just leave it running and always reset the timeout. Add some critical sections when modifying the timer or the interrupt state. Seemed to work then. Seems to be some issue starting and stopping the timer, perhaps you could explore that further.

static uint32_t timers[] = { 10000, 10000, 10000, 10000, 9999, 500, 1500, 1000, 100 };
=>
Measurements: 10007 10016 10005 10011 10009  508 1507 1007  106 

static void IRAM timer_handler(void *arg) {
  measurements[count] = sdk_system_get_time();
  if (count > sizeof(timers) / sizeof(timers[0])) {
      timer_set_timeout(FRC1, 10000);
    return;
  }
  timer_set_timeout(FRC1, timers[count++]);
}

The following also seems to work, resetting the timeout before stopping the timer. Perhaps that clears the interrupt and leaves the timer in the expected state???

static void IRAM timer_handler(void *arg) {
    timer_set_timeout(FRC1, 10000);
    timer_set_run(FRC1, false);

  measurements[count] = sdk_system_get_time();
  if (count > sizeof(timers) / sizeof(timers[0])) {
    return;
  }
  timer_set_timeout(FRC1, timers[count++]);

  timer_set_run(FRC1, true);
}

Perhaps you could share if any of these suggestions help and it could be documented.

[maximkulkin]

Ok, I've done some more testing with dumping FRC1 state. Docs say that when RELOAD bit is not set, upon triggering COUNTER should be updated to max value (7fffff) which is not the case if you have 256 divider. Also when you arm next timer, and it triggers, you can see that COUNT value is just slightly less than LOAD value which (taking into account that actual time passed is pretty small compared to what was expected) means that it triggers without even reaching 0. I was able to get it working with either using timer_set_interrupts() instead of timer_set_run() to stop/run timer or setting HOLD bit to prevent timers interrupts and manually clearing bit 0 of STATUS register to enable them again. That being set, the question is: what is the expected effect of timer_set_run() and how it relates/different from timer_set_interrupts(). It seems that just using timer_set_run() alone (which just sets RUN bit) is not enough to actually stop the timer (and all side effects) so maybe it makes sense to merge those two.