Skip to content

Low power timer needs to be reset when setting time #7849

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 2 commits into from
Nov 8, 2018
Merged

Low power timer needs to be reset when setting time #7849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d82990c
Low power timer needs to be reset when setting time.
Aug 21, 2018
0de5ab2
Add a protected HAL set_time() test
Sep 11, 2018
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
28 changes: 28 additions & 0 deletions TESTS/mbed_hal/rtc_time/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,41 @@ void test_local_time_invalid_param()
TEST_ASSERT_EQUAL(false, _rtc_localtime(1, NULL, RTC_4_YEAR_LEAP_YEAR_SUPPORT));
}

/* Test set_time() function called a few seconds apart.
*
* Given is set_time() function.
* When set_time() is used to set the system time two times.
* Then if the value returned from time() is always correct return true, otherwise return false.
*/
#define NEW_TIME 15
void test_set_time_twice()
{
time_t current_time;

/* Set the time to NEW_TIME and check it */
set_time(NEW_TIME);
current_time = time(NULL);
TEST_ASSERT_EQUAL (true, (current_time == NEW_TIME));

/* Wait 2 seconds */
wait_ms(2000);

/* set the time to NEW_TIME again and check it */
set_time(NEW_TIME);
current_time = time(NULL);
TEST_ASSERT_EQUAL (true, (current_time == NEW_TIME));
}

Case cases[] = {
Case("test is leap year - RTC leap years full support", test_is_leap_year<RTC_FULL_LEAP_YEAR_SUPPORT>),
Case("test is leap year - RTC leap years partial support", test_is_leap_year<RTC_4_YEAR_LEAP_YEAR_SUPPORT>),
Case("test make time boundary values - RTC leap years full support", test_mk_time_boundary<RTC_FULL_LEAP_YEAR_SUPPORT>),
Case("test make time boundary values - RTC leap years partial support", test_mk_time_boundary<RTC_4_YEAR_LEAP_YEAR_SUPPORT>),
Case("test make time - invalid param", test_mk_time_invalid_param),
Case("test local time - invalid param", test_local_time_invalid_param),
#if DEVICE_RTC || DEVICE_LPTICKER
Case("test set_time twice", test_set_time_twice),
#endif
};

utest::v1::status_t greentea_test_setup(const size_t number_of_cases)
Expand Down
1 change: 1 addition & 0 deletions platform/mbed_rtc_time.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ static time_t _rtc_lpticker_read(void)

static void _rtc_lpticker_write(time_t t)
{
_rtc_lp_timer->reset();
Copy link
Contributor

Choose a reason for hiding this comment

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

If the RTC is also being used for other parts of the system, such as the low power timer, this may interfere with it. One possible less intrusive alternative is to only adjust _rtc_lp_base by taking into account the current time. Something like this:

_rtc_lp_base = t - _rtc_lp_timer->read();

Copy link
Contributor

Choose a reason for hiding this comment

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

The current version is fine, I think. It's only resetting the internal LowPowerTimer there.

The prefixing of everything in the file with _rtc is confusing.

I'd also specifically suggest not having _rtc_lp_base be a uint64_t though. It's a time_t. Probably wasting a word of RAM and similar amount of ROM there, at least while time_t is 32-bit.

Been looking into C++11 chrono recently, so getting a bit picky about correctly-typed time. Having it be a time_t makes clear that it is the actual time the clock was last adjusted, which is potentially useful information, and I could imagine an API call to read that. Russ's proposed adjustment would lose that information.

_rtc_lp_base = t;
}

Expand Down