Skip to content
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.

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

Add a delay after rtc_set_datetime #2156

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 6 additions & 0 deletions src/rp2_common/pico_aon_timer/aon_timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ static aon_timer_alarm_handler_t aon_timer_alarm_handler;
#if HAS_RP2040_RTC
#include "hardware/rtc.h"
#include "pico/util/datetime.h"
#include "pico/time.h"
#include "hardware/clocks.h"

#elif HAS_POWMAN_TIMER
#include "hardware/powman.h"
Expand Down Expand Up @@ -56,6 +58,10 @@ bool aon_timer_set_time_calendar(const struct tm *tm) {
datetime_t dt;
tm_to_datetime(tm, &dt);
rtc_set_datetime(&dt);

// Writing to the RTC will take 2 clk_rtc clock periods to arrive
uint rtc_freq = clock_get_hz(clk_rtc);
busy_wait_us((1000000 / rtc_freq) * 2);
return true;
#elif HAS_POWMAN_TIMER
struct timespec ts;
Expand Down
2 changes: 1 addition & 1 deletion test/pico_time_test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ if (NOT PICO_TIME_NO_ALARM_SUPPORT)
target_compile_definitions(pico_time_test PRIVATE
PICO_TIME_DEFAULT_ALARM_POOL_MAX_TIMERS=250
)
target_link_libraries(pico_time_test PRIVATE pico_test)
target_link_libraries(pico_time_test PRIVATE pico_test pico_aon_timer)
pico_add_extra_outputs(pico_time_test)
endif()
50 changes: 42 additions & 8 deletions test/pico_time_test/pico_time_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "hardware/clocks.h"
#include "pico/stdlib.h"
#include "pico/test.h"
#include "pico/aon_timer.h"
// Include sys/types.h before inttypes.h to work around issue with
// certain versions of GCC and newlib which causes omission of PRIi64
#include <sys/types.h>
Expand Down Expand Up @@ -72,10 +73,11 @@ static bool repeating_timer_callback(struct repeating_timer *t) {
#define RESOLUTION_ALLOWANCE PICO_HARDWARE_TIMER_RESOLUTION_US
#endif

int issue_195_test(void);
int issue_1812_test(void);
int issue_1953_test(void);
int issue_2118_test(void);
static int issue_195_test(void);
static int issue_1812_test(void);
static int issue_1953_test(void);
static int issue_2118_test(void);
static int issue_2148_test(void);

int main() {
setup_default_uart();
Expand Down Expand Up @@ -250,6 +252,8 @@ int main() {

issue_2118_test();

issue_2148_test();

PICOTEST_END_TEST();
}

Expand All @@ -260,7 +264,7 @@ int64_t issue_195_callback(alarm_id_t id, void *user_data) {
return -ISSUE_195_TIMER_DELAY;
}

int issue_195_test(void) {
static int issue_195_test(void) {
PICOTEST_START_SECTION("Issue #195 race condition - without fix may hang on gcc 10.2.1 release builds");
absolute_time_t t1 = get_absolute_time();
int id = add_alarm_in_us(ISSUE_195_TIMER_DELAY, issue_195_callback, NULL, true);
Expand All @@ -279,7 +283,7 @@ int issue_195_test(void) {
}

// Setting an alarm should not swallow a sev
int issue_1812_test(void) {
static int issue_1812_test(void) {
PICOTEST_START_SECTION("Issue #1812 defect - Setting an alarm should not ignore a sev");

__sev(); // Make sure the call below does not ignore this
Expand All @@ -303,7 +307,7 @@ static void alarm_pool_stuck_issue_1953(uint alarm) {
hard_assert(false);
}

int issue_1953_test(void) {
static int issue_1953_test(void) {
PICOTEST_START_SECTION("Issue #1953 defect - Alarm can be set in the past");
int alarm = hardware_alarm_claim_unused(true);
hardware_alarm_set_callback(alarm, alarm_pool_stuck_issue_1953);
Expand Down Expand Up @@ -336,7 +340,7 @@ static bool timer_callback_issue_2118(repeating_timer_t *rt) {
return true;
}

int issue_2118_test(void) {
static int issue_2118_test(void) {
PICOTEST_START_SECTION("Issue #2118 defect - failure to set an alarm");

// this problem only happens when running the clock fast as it requires the time between
Expand Down Expand Up @@ -364,3 +368,33 @@ int issue_2118_test(void) {
PICOTEST_END_SECTION();
return 0;
}

static int issue_2148_test(void) {
#if HAS_RP2040_RTC
PICOTEST_START_SECTION("Issue #2148 defect - get time after rtc start");
struct tm tm = { 0 };
struct tm tm_check = { 0 };

tm.tm_sec = 55;
Copy link
Contributor

Choose a reason for hiding this comment

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

I know that this test will probably take much less than 5 seconds to run, but given that the checking-code below is checking for tm_sec rollover but isn't checking for tm_min rollover, perhaps it makes sense to set the tm_sec to something like 15 rather than 55 ? 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about 42?

tm.tm_min = 36;
tm.tm_hour = 20;
tm.tm_mday = 21;
tm.tm_mon = 10;
tm.tm_year = 124;
tm.tm_wday = 4;
tm.tm_yday = 325;
tm.tm_isdst = 0;
hard_assert(aon_timer_start_calendar(&tm));
hard_assert(aon_timer_get_time_calendar(&tm_check));

PICOTEST_CHECK(tm.tm_sec == tm_check.tm_sec || tm.tm_sec == tm_check.tm_sec - 1, "failed to get seconds");
PICOTEST_CHECK(tm.tm_min == tm_check.tm_min, "failed to get minutes");
PICOTEST_CHECK(tm.tm_hour == tm_check.tm_hour, "failed to get hour");
PICOTEST_CHECK(tm.tm_mday == tm_check.tm_mday, "failed to get day");
PICOTEST_CHECK(tm.tm_mon == tm_check.tm_mon, "failed to get month");
PICOTEST_CHECK(tm.tm_year == tm_check.tm_year, "failed to get year");

aon_timer_stop();
PICOTEST_END_SECTION();
#endif
}
Loading