Skip to content
This repository has been archived by the owner on Jan 29, 2023. It is now read-only.

Poor accuracy on timer interrupt frequency or interval. #4

Closed
pixpop opened this issue Oct 12, 2022 · 18 comments
Closed

Poor accuracy on timer interrupt frequency or interval. #4

pixpop opened this issue Oct 12, 2022 · 18 comments
Labels
bug Something isn't working

Comments

@pixpop
Copy link

pixpop commented Oct 12, 2022

Describe the bug

Timer interrupts at a lower frequency than expected.

Arduino IDE 1.8.19
Arduino MBED nano 3.3.0
MBED_RPI_PICO_TimerInterrupt 1.1.2
Hardware: Arduino nano rp2040

Steps to Reproduce

I set a frequency of 20 kHz, and then count 20k interrupts. I check mills() before and after. The delta comes out to 1020 ms where it should be 1000. So it's off by 2%.

Expected behavior

I expected it to take 1000 ms to count 20000 interrupts at 20000 Hz

Actual behavior

It took 1020 ms, so it's off by 2%. I see the same error if I program it by interval or by frequency.

Test code:

#include <MBED_RPi_Pico_TimerInterrupt.h>
#include <MBED_RPi_Pico_TimerInterrupt.hpp>
#include <MBED_RPi_Pico_ISR_Timer.h>
#include <MBED_RPi_Pico_ISR_Timer.hpp>

#define TIMER_FREQ_HZ        20000

MBED_RPI_PICO_Timer ITimer1(1);

int delta = 0;

void TimerHandler(uint alarm_num) // Frequency version
{
  static int zz = 0;
  static int prev_millis = 0;
  int new_millis;

  TIMER_ISR_START(alarm_num);
  
  zz++;
  if(zz >= TIMER_FREQ_HZ) {
    zz = 0;
    new_millis = millis();
    delta = new_millis - prev_millis;
    prev_millis = new_millis;
  }

  TIMER_ISR_END(alarm_num);
}

void setup() {
  Serial.begin(9600);
  delay(2500);

  if (ITimer1.attachInterrupt(TIMER_FREQ_HZ, TimerHandler))
    Serial.println("Starting ITimer OK, millis() = " + String(millis()));
  else
    Serial.println("Can't set ITimer. Select another freq. or timer");
}

void loop() {
  delay(1000);
  Serial.println(delta);
}

Sample printout:

Starting ITimer OK, millis() = 2500
0
3520
1020
1020
1020
1020
1020
1020
1020
1020
1020
1020
1020
@khoih-prog
Copy link
Owner

khoih-prog commented Oct 12, 2022

You can't use inaccurate millis() to measure TimerInterrupt accuracy.

To make some sense, you have to, at least, use accurate enough Oscilloscope

Check ISR_16_Timers_Array_Complex on RaspberryPi Pico and/or use ISR_16_Timers_Array_Complex example for better way to measure the accuracy without Scope

@khoih-prog khoih-prog added the invalid This doesn't seem right label Oct 12, 2022
@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

Well, I have a scope, so I'll check it that way. I haven't found mills() to be inaccurate though.

@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

By the way, the code in the loop doesn't measure the period. That's measured in the interrupt. The loop just prints out a snapshot of the most recent period.

@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

Okay, I just checked it with a scope. It's off by 2%. I have the interrupt at 20 kHz, and I toggle an output pin every time through. One cycle of that output pin takes 102 us. It should be 100 us.

Here's the modified code with the scope trigger.

void TimerHandler(uint alarm_num) // Frequency version
{
  static int zz = 0;
  static int prev_millis = 0;
  int new_millis;

  TIMER_ISR_START(alarm_num);
 
  zz++;
  digitalWrite(18, zz & 1);    // This pin should go high every 100 us, but it takes 102.

  if(zz >= TIMER_FREQ_HZ) {
    zz = 0;
    new_millis = millis();
    delta = new_millis - prev_millis;
    prev_millis = new_millis;
  }

  TIMER_ISR_END(alarm_num);
}

@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

I checked it at some other frequencies. At 50 kHz, I get 42 us instead of 40, so it's off by 2 us.
At 10 kHz I get 202 us instead of 200, so also 2 us. It looks like there's a constant 2 us error, regardless of frequency.

@khoih-prog khoih-prog added bug Something isn't working and removed invalid This doesn't seem right labels Oct 12, 2022
@khoih-prog
Copy link
Owner

Hi @pixpop

OK, the issue is convincing now. I'll have a look soon when having time.

Can you help try using the previous cores, such as v2.7.2, and v3.0.0 to see if the issue caused by the library or the cores.

Thanks,

@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

The bug still shows as closed. Can you reopen it?

@khoih-prog khoih-prog reopened this Oct 12, 2022
@khoih-prog
Copy link
Owner

Can you also try RPI_PICO_TimerInterrupt library, especially have a look at Enable fixed timing between timer calls (vs fixed time btw. end of timer call and next call as implemented) #3 to see if related.

The previous problem caused by weird syntax introduced by RPi-Pico SDK, possibly confusing the Arduino MBED core people ???

I'll fix the severe bug to let users to set fixed timing between timer calls by using the negative delay

4.2.14.4.2. add_repeating_timer_us
static bool add_repeating_timer_us (int64_t delay_us,
repeating_timer_callback_t callback,
void *user_data,
repeating_timer_t *out)
Add a repeating timer that is called repeatedly at the specified interval in microseconds
Generally the callback is called as soon as possible after the time specified from an IRQ handler on the core of the
default alarm pool (generally core 0). If the callback is in the past or happens before the alarm setup could be
completed, then this method will optionally call the callback itself and then return a return code to indicate that the
target time has passed.
Parameters
• delay_us the repeat delay in microseconds; if >0 then this is the delay between one callback ending and the next
starting; if <0 then this is the negative of the time between the starts of the callbacks. The value of 0 is treated as 1
• callback the repeating timer callback function
• user_data user data to pass to store in the repeating_timer structure for use by the callback.
• out the pointer to the user owned structure to store the repeating timer info in. BEWARE this storage location must
outlive the repeating timer, so be careful of using stack space

@khoih-prog
Copy link
Owner

khoih-prog commented Oct 12, 2022

Can you try to modify MBED_RPi_Pico_TimerInterrupt.hpp#L134 from

_timerCount[_timerNo] = (uint64_t) TIM_CLOCK_FREQ / frequency;

to

_timerCount[_timerNo] = (uint64_t) ( ( float) TIM_CLOCK_FREQ / frequency ) - 1;

and see if better

@khoih-prog
Copy link
Owner

khoih-prog commented Oct 12, 2022

The real code of mbed_rp2040 core, relied by this library, is hidden in the variants/RASPBERRY_PI_PICO/libs/libmbed.a precompiled library file, and we have no way to check if it's doing correctly or not.

@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

Re: line 134. Should it be -1 or -2 ?

@khoih-prog
Copy link
Owner

khoih-prog commented Oct 12, 2022

After replacing with (-1, not -2)

_timerCount[_timerNo] = (uint64_t) ( ( float) TIM_CLOCK_FREQ / frequency ) - 1;

The result running your first code is better

Starting ITimer OK, millis() = 1729
0
2729
1000
1000
1000
1000
1000
1000
1000

@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

It's good. It's possibly a little off at 50 kHz. Hard to be sure. Scope says it's 39.8 us instead of 40.

@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

It's hard to measure it accurately enough on my scope at 50 kHz. It might be okay.

@khoih-prog
Copy link
Owner

khoih-prog commented Oct 12, 2022

Try using the RPI_PICO_TimerInterrupt library to see your scope is OK. The timing was checked and OK.

I'll investigate this issue later. If we can't fix it, I have to rewrite the library using SDK directly, not relying on the hidden mbed_rp2040 core functions.

@pixpop
Copy link
Author

pixpop commented Oct 12, 2022

I have a frequency counter that's very accurate. Let me check with that. Of course, I don't know how accurate the Arduino clock is to start with ;-(

khoih-prog added a commit that referenced this issue Oct 12, 2022
### Releases v1.2.0

1. Fix `poor-timer-accuracy` bug. Check [Poor accuracy on timer interrupt frequency or interval. #4](#4)
@khoih-prog
Copy link
Owner

Hi @pixpop

The new MBED_RPI_PICO_TimerInterrupt v1.2.0 has just been published. Your contribution is noted in Contributions and Thanks


Releases v1.2.0

  1. Fix poor-timer-accuracy bug. Check Poor accuracy on timer interrupt frequency or interval. #4

khoih-prog added a commit to khoih-prog/MBED_RP2040_Slow_PWM that referenced this issue Oct 12, 2022
### Releases v1.3.0

1. Fix `poor-timer-accuracy` bug. Check [Poor accuracy on timer interrupt frequency or interval. #4](khoih-prog/MBED_RPI_PICO_TimerInterrupt#4)
khoih-prog added a commit to khoih-prog/MBED_RP2040_Slow_PWM that referenced this issue Oct 12, 2022
### Releases v1.3.0

1. Fix `poor-timer-accuracy` bug. Check [Poor accuracy on timer interrupt frequency or interval. #4](khoih-prog/MBED_RPI_PICO_TimerInterrupt#4)
@khoih-prog
Copy link
Owner

Hi @pixpop

Your contribution has been spread to the newly published MBED_RP2040_Slow_PWM v1.3.0. Your contribution is also noted in Contributions and Thanks


Releases v1.3.0

  1. Fix poor-timer-accuracy bug. Check Poor accuracy on timer interrupt frequency or interval. #4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants