Skip to content

Bugfix: micros() returns smaller number when timer wraps #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 8, 2023
Merged

Bugfix: micros() returns smaller number when timer wraps #52

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
733020d
See https://github.com/arduino/ArduinoCore-renesas/issues/49.
Kees-van-der-Oord Jul 16, 2023
c653486
set correct priority for timer
Kees-van-der-Oord Jul 16, 2023
7bef4fc
added const qualifiers, gave register access macro function format, b…
Kees-van-der-Oord Jul 24, 2023
8dcbd08
corrected typo in comments
Kees-van-der-Oord Jul 24, 2023
46d5403
added support for the Portenta C33
Kees-van-der-Oord Aug 5, 2023
8ad4dd3
read counter direct from AGT0 count register: micros() is now 2x faster
Kees-van-der-Oord Aug 6, 2023
b6a73f8
moved all const defintions inline
Kees-van-der-Oord Aug 7, 2023
c47fb07
Suffix constant "clock_freq" with "clock_freq_Hz" to make clear the u…
aentinger Aug 8, 2023
70ef6c7
Split function call across multiple lines and document parameters for…
aentinger Aug 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ cores/arduino/mydebug.cpp
libraries/Storage/.development
cores/arduino/mydebug.cpp.donotuse
.DS_Store
.DS_Store?
.DS_Store?
/.vs
/.gitignore
41 changes: 27 additions & 14 deletions cores/arduino/time.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
#include "IRQManager.h"
#include "FspTimer.h"

// this file implements the following public funcions: delay, delayMicroseconds, yield, millis, micros
// The millis and micros implementation uses timer AGT0 (24 HMz, 16-bits, count-down mode, 1 ms period)

volatile unsigned long agt_time_ms = 0;
uint32_t _freq_hz = 0;

__attribute__((weak)) void delay(uint32_t ms) {
R_BSP_SoftwareDelay(ms, BSP_DELAY_UNITS_MILLISECONDS);
Expand All @@ -17,20 +19,25 @@ __attribute__((weak)) void yield() {
}

static FspTimer main_timer;
// specifying these details as constants makes micros() faster !
#define _timer_type AGT_TIMER
#define _timer_index 0
#define _timer_get_underflow_bit() R_AGT0->AGTCR_b.TUNDF
#define _timer_clock_divider TIMER_SOURCE_DIV_8 // dividers 1, 2 and 8 work because _timer_period is < 16-bit. the divider 4 seems not supported: acts as 1
#define _timer_clock_freq 24000000UL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not work on all FSP supported boards (like Portenta C33). It's very important to fix this before merging since otherwise the number of ifdefs in the core would soon become unbearable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the defines have to go @Kees-van-der-Oord , as already mentioned here.

I don't see how there can be any runtime speed-up by having them in the code, as its simple string search and replace done by the preprocessor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the comment here. I will try to find the most efficient implementation that also works for the Portenta C33.

#define _timer_counts_per_us (_timer_clock_freq / ((1 << _timer_clock_divider) * 1000000UL))
#define _timer_period (_timer_counts_per_us * 1000UL)
#define TIMER_PRIORITY 8

static uint32_t _top_counter;

static void timer_micros_callback(timer_callback_args_t __attribute((unused)) *p_args) {
agt_time_ms += 1; //1ms
static void timer_micros_callback(timer_callback_args_t __attribute((unused))* p_args) {
agt_time_ms += 1;
}

void startAgt() {
main_timer.begin(TIMER_MODE_PERIODIC, AGT_TIMER, 0, 2000.0f, 0.5f, timer_micros_callback);
IRQManager::getInstance().addPeripheral(IRQ_AGT,(void*)main_timer.get_cfg());
main_timer.begin(TIMER_MODE_PERIODIC, _timer_type, _timer_index, _timer_period, 1, _timer_clock_divider, timer_micros_callback);;
main_timer.setup_overflow_irq(TIMER_PRIORITY);
main_timer.open();
_top_counter = main_timer.get_counter();
main_timer.start();
_freq_hz = main_timer.get_freq_hz();
main_timer.start(); // bug in R4 1.0.2: calling start() is not necessary: open() starts the counter already !?
}

unsigned long millis()
Expand All @@ -43,10 +50,16 @@ unsigned long millis()
}

unsigned long micros() {

// Convert time to us
// Return time in us
NVIC_DisableIRQ(main_timer.get_cfg()->cycle_end_irq);
uint32_t time_us = ((main_timer.get_period_raw() - main_timer.get_counter()) * 1000 / main_timer.get_period_raw()) + (agt_time_ms * 1000);
uint32_t ms = agt_time_ms;
uint32_t const down_counts = main_timer.get_counter();
if (_timer_get_underflow_bit() && (down_counts > (_timer_period / 2)))
{
// the counter wrapped around just before it was read
++ms;
}
NVIC_EnableIRQ(main_timer.get_cfg()->cycle_end_irq);
return time_us;
uint32_t const up_counts = (_timer_period - 1) - down_counts;
return (ms * 1000) + (up_counts / _timer_counts_per_us);
}