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

Watchdog issue with timeout #11175

Closed
Tharazi97 opened this issue Aug 7, 2019 · 6 comments

Comments

@Tharazi97
Copy link
Contributor

commented Aug 7, 2019

Description

Currently calculated timout is rounded down on STM devices, but in this test:

void test_start_max_timeout()
{
Watchdog &watchdog = Watchdog::get_instance();
uint32_t max_timeout = watchdog.get_max_timeout();
TEST_ASSERT_TRUE(watchdog.start(max_timeout));
// The watchdog should trigger at, or after the timeout value.
TEST_ASSERT(watchdog.get_timeout() >= max_timeout);
}

It is checked if calculated is bigger or equal to the value passed via init function. This test passes for most of the targets, because of their LSI frequency value which after division throgh it gives good integer value so that reload value can be calculated without any rounding. But not in the case of STM32-L0xx boards where the LSI frequency value is set to 37 kHz.

So should we change the test so we accept rounding down, or should we round the values up (if it doesn't exceed the upper limit of timeout)?

@maciejbocianski @mprse @fkjagodzinski @jamesbeyond

Issue request type

[ ] Question
[ ] Enhancement
[x] Bug
@ciarmcom

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

@ciarmcom ciarmcom added the mirrored label Aug 7, 2019

@fkjagodzinski

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

So should we change the test so we accept rounding down, or should we round the values up (if it doesn't exceed the upper limit of timeout)?

The test verifies our timing requirements for watchdog timeout. Has to stay as it is. It is the implementation, that has to be updated, if it doesn't comply with requirements.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

@jeromecoutant

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

Issue confirmed, you can st st device label

@fkjagodzinski

This comment has been minimized.

Copy link
Member

commented Aug 22, 2019

#11178 has been merged. @Tharazi97 could you confirm it fixed this issue?

@Tharazi97

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2019

Tested, it fixes the issue. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.