Skip to content

Commit

Permalink
Fix watchdog test warnings, fix broken hal_watchdog_get_reload_value(…
Browse files Browse the repository at this point in the history
…), fix missing frequency field in watchdog features
  • Loading branch information
multiplemonomials committed Jul 10, 2023
1 parent 789bd95 commit 42e0780
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 50 deletions.
33 changes: 17 additions & 16 deletions hal/tests/TESTS/mbed_hal/watchdog/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
#include <stdlib.h>

/* The shortest timeout value, this test suite is able to handle correctly. */
#define WDG_MIN_TIMEOUT_MS 50UL
#define WDG_MIN_TIMEOUT_MS 50ms

// Do not set watchdog timeout shorter than WDG_MIN_TIMEOUT_MS, as it may
// cause the host-test-runner return 'TIMEOUT' instead of 'FAIL' / 'PASS'
// if watchdog performs reset during test suite teardown.
#define WDG_TIMEOUT_MS 100UL
#define WDG_TIMEOUT_MS 100ms

#define MSG_VALUE_DUMMY "0"
#define MSG_VALUE_LEN 24
Expand All @@ -57,7 +57,7 @@
* (1 start_bit + 8 data_bits + 1 stop_bit) * 128 * 1000 / 9600 = 133.3 ms.
* To be on the safe side, set the wait time to 150 ms.
*/
#define SERIAL_FLUSH_TIME_MS 150
#define SERIAL_FLUSH_TIME_MS 150ms

int CASE_INDEX_START;
int CASE_INDEX_CURRENT;
Expand All @@ -67,7 +67,7 @@ using utest::v1::Case;
using utest::v1::Specification;
using utest::v1::Harness;

const watchdog_config_t WDG_CONFIG_DEFAULT = { .timeout_ms = WDG_TIMEOUT_MS };
const watchdog_config_t WDG_CONFIG_DEFAULT = { .timeout_ms = WDG_TIMEOUT_MS.count() };

void test_max_timeout_is_valid()
{
Expand Down Expand Up @@ -115,12 +115,12 @@ void test_update_config()
}

watchdog_config_t config = WDG_CONFIG_DEFAULT;
uint32_t timeouts[] = {
features.max_timeout / 4,
features.max_timeout / 8,
features.max_timeout / 16
std::chrono::milliseconds timeouts[] = {
std::chrono::milliseconds(features.max_timeout / 4),
std::chrono::milliseconds(features.max_timeout / 8),
std::chrono::milliseconds(features.max_timeout / 16)
};
int num_timeouts = sizeof timeouts / sizeof timeouts[0];
size_t num_timeouts = sizeof timeouts / sizeof timeouts[0];

for (size_t i = 0; i < num_timeouts; i++) {
if (timeouts[i] < WDG_MIN_TIMEOUT_MS) {
Expand All @@ -129,9 +129,10 @@ void test_update_config()
return;
}

config.timeout_ms = timeouts[i];
config.timeout_ms = timeouts[i].count();
TEST_ASSERT_EQUAL(WATCHDOG_STATUS_OK, hal_watchdog_init(&config));
uint32_t reload_value = hal_watchdog_get_reload_value();

auto reload_value = std::chrono::milliseconds(hal_watchdog_get_reload_value());
// The watchdog should trigger at, or after the timeout value.
TEST_ASSERT(reload_value >= timeouts[i]);
// The watchdog should trigger before twice the timeout value.
Expand All @@ -155,7 +156,7 @@ utest::v1::status_t case_teardown_sync_on_reset(const Case *const source, const
// Start kicking the watchdog during teardown.
hal_watchdog_kick();
Ticker wdg_kicking_ticker;
wdg_kicking_ticker.attach_us(mbed::callback(hal_watchdog_kick), 20000);
wdg_kicking_ticker.attach(mbed::callback(hal_watchdog_kick), 20ms);
utest::v1::status_t status = utest::v1::greentea_case_teardown_handler(source, passed, failed, failure);
if (failed) {
/* Return immediately and skip the device reset, if the test case failed.
Expand Down Expand Up @@ -193,7 +194,7 @@ utest::v1::status_t case_teardown_wdg_stop_or_reset(const Case *const source, co
template<uint32_t timeout_ms>
void test_init()
{
if (timeout_ms < WDG_MIN_TIMEOUT_MS) {
if (std::chrono::milliseconds(timeout_ms) < WDG_MIN_TIMEOUT_MS) {
CASE_IGNORED = true;
TEST_IGNORE_MESSAGE("Requested timeout value is too short -- ignoring test case.");
return;
Expand All @@ -216,7 +217,7 @@ void test_init_max_timeout()
TEST_ASSERT(hal_watchdog_get_reload_value() >= features.max_timeout);
}

int testsuite_setup_sync_on_reset(const size_t number_of_cases)
utest::v1::status_t testsuite_setup_sync_on_reset(const size_t number_of_cases)
{
GREENTEA_SETUP(45, "sync_on_reset");
utest::v1::status_t status = utest::v1::greentea_test_setup_handler(number_of_cases);
Expand All @@ -243,7 +244,7 @@ int testsuite_setup_sync_on_reset(const size_t number_of_cases)
}

utest_printf("Starting with test case index %i of all %i defined test cases.\n", CASE_INDEX_START, number_of_cases);
return CASE_INDEX_START;
return static_cast<utest::v1::status_t>(CASE_INDEX_START);
}

Case cases[] = {
Expand All @@ -262,7 +263,7 @@ Case cases[] = {
test_init_max_timeout, (utest::v1::case_teardown_handler_t) case_teardown_sync_on_reset),
};

Specification specification((utest::v1::test_setup_handler_t) testsuite_setup_sync_on_reset, cases);
Specification specification(testsuite_setup_sync_on_reset, cases);

int main()
{
Expand Down
52 changes: 26 additions & 26 deletions hal/tests/TESTS/mbed_hal/watchdog_reset/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
#include "watchdog_reset_tests.h"
#include "mbed.h"

#define TIMEOUT_MS 100UL
#include <cinttypes>

#define TIMEOUT_MS 100ms

/* This value is used to calculate the time to kick the watchdog.
* Given the watchdog timeout is set to TIMEOUT_MS, the kick will be performed
Expand All @@ -40,7 +42,7 @@
* and as short as 66 ms.
* The value of 35 ms is used to cover the worst case scenario (66 ms).
*/
#define KICK_ADVANCE_MS 35UL
#define KICK_ADVANCE_MS 35ms

#define MSG_VALUE_DUMMY "0"
#define CASE_DATA_INVALID 0xffffffffUL
Expand All @@ -66,11 +68,8 @@
* (1 start_bit + 8 data_bits + 1 stop_bit) * 128 * 1000 / 9600 = 133.3 ms.
* To be on the safe side, set the wait time to 150 ms.
*/
#define SERIAL_FLUSH_TIME_MS 150
#define SERIAL_FLUSH_TIME_MS 150ms

#define TIMEOUT_US (1000 * (TIMEOUT_MS))
#define KICK_ADVANCE_US (1000 * (KICK_ADVANCE_MS))
#define SERIAL_FLUSH_TIME_US (1000 * (SERIAL_FLUSH_TIME_MS))

using utest::v1::Case;
using utest::v1::Specification;
Expand All @@ -86,10 +85,10 @@ testcase_data current_case;

Ticker wdg_kicking_ticker;

bool send_reset_notification(testcase_data *tcdata, uint32_t delay_ms)
bool send_reset_notification(testcase_data *tcdata, std::chrono::milliseconds delay_ms)
{
char msg_value[12];
int str_len = snprintf(msg_value, sizeof msg_value, "%02x,%08lx", tcdata->start_index + tcdata->index, delay_ms);
int str_len = snprintf(msg_value, sizeof msg_value, "%02x,%08" PRIx64, tcdata->start_index + tcdata->index, delay_ms.count());
if (str_len < 0) {
utest_printf("Failed to compose a value string to be sent to host.");
return false;
Expand All @@ -110,15 +109,15 @@ void test_simple_reset()

// Phase 1. -- run the test code.
// Init the watchdog and wait for a device reset.
watchdog_config_t config = { TIMEOUT_MS };
watchdog_config_t config = { TIMEOUT_MS.count() };
if (send_reset_notification(&current_case, 2 * TIMEOUT_MS + SERIAL_FLUSH_TIME_MS) == false) {
TEST_ASSERT_MESSAGE(0, "Dev-host communication error.");
return;
}
wait_us(SERIAL_FLUSH_TIME_US); // Wait for the serial buffers to flush.
wait_us(std::chrono::duration_cast<std::chrono::microseconds>(SERIAL_FLUSH_TIME_MS).count()); // Wait for the serial buffers to flush.
TEST_ASSERT_EQUAL(WATCHDOG_STATUS_OK, hal_watchdog_init(&config));
// Watchdog should fire before twice the timeout value.
wait_us(2 * TIMEOUT_US); // Device reset expected.
wait_us(2 * std::chrono::duration_cast<std::chrono::microseconds>(TIMEOUT_MS).count()); // Device reset expected.

// Watchdog reset should have occurred during a wait above.

Expand Down Expand Up @@ -215,28 +214,29 @@ void test_restart_reset()
}

// Phase 1. -- run the test code.
watchdog_config_t config = { TIMEOUT_MS };
watchdog_config_t config = { TIMEOUT_MS.count() };
TEST_ASSERT_EQUAL(WATCHDOG_STATUS_OK, hal_watchdog_init(&config));
wait_us(TIMEOUT_US / 2);
wait_us(std::chrono::duration_cast<std::chrono::microseconds>(TIMEOUT_MS / 2).count());
TEST_ASSERT_EQUAL(WATCHDOG_STATUS_OK, hal_watchdog_stop());
// Check that stopping the Watchdog prevents a device reset.
// The watchdog should trigger at, or after the timeout value.
// The watchdog should trigger before twice the timeout value.
wait_us(TIMEOUT_US / 2 + TIMEOUT_US);
wait_us(std::chrono::duration_cast<std::chrono::microseconds>(TIMEOUT_MS + TIMEOUT_MS / 2).count());

if (send_reset_notification(&current_case, 2 * TIMEOUT_MS + SERIAL_FLUSH_TIME_MS) == false) {

if (!send_reset_notification(&current_case, 2 * TIMEOUT_MS + SERIAL_FLUSH_TIME_MS)) {
TEST_ASSERT_MESSAGE(0, "Dev-host communication error.");
return;
}
wait_us(SERIAL_FLUSH_TIME_US); // Wait for the serial buffers to flush.
wait_us(std::chrono::duration_cast<std::chrono::microseconds>(SERIAL_FLUSH_TIME_MS).count()); // Wait for the serial buffers to flush.
TEST_ASSERT_EQUAL(WATCHDOG_STATUS_OK, hal_watchdog_init(&config));
// Watchdog should fire before twice the timeout value.
wait_us(2 * TIMEOUT_US); // Device reset expected.
wait_us(2 * std::chrono::duration_cast<std::chrono::microseconds>(TIMEOUT_MS).count()); // Device reset expected.

// Watchdog reset should have occurred during a wait above.

hal_watchdog_kick();
wdg_kicking_ticker.attach_us(mbed::callback(hal_watchdog_kick), 20000); // For testsuite failure handling.
wdg_kicking_ticker.attach(mbed::callback(hal_watchdog_kick), 20ms); // For testsuite failure handling.
TEST_ASSERT_MESSAGE(0, "Watchdog did not reset the device as expected.");
}

Expand All @@ -250,21 +250,21 @@ void test_kick_reset()
}

// Phase 1. -- run the test code.
watchdog_config_t config = { TIMEOUT_MS };
watchdog_config_t config = { TIMEOUT_MS.count() };
TEST_ASSERT_EQUAL(WATCHDOG_STATUS_OK, hal_watchdog_init(&config));
for (int i = 3; i; i--) {
// The reset is prevented as long as the watchdog is kicked
// anytime before the timeout.
wait_us(TIMEOUT_US - KICK_ADVANCE_US);
wait_us(2 * std::chrono::duration_cast<std::chrono::microseconds>(TIMEOUT_MS - KICK_ADVANCE_MS).count());
hal_watchdog_kick();
}
if (send_reset_notification(&current_case, 2 * TIMEOUT_MS + SERIAL_FLUSH_TIME_MS) == false) {
if (!send_reset_notification(&current_case, 2 * TIMEOUT_MS + SERIAL_FLUSH_TIME_MS)) {
TEST_ASSERT_MESSAGE(0, "Dev-host communication error.");
return;
}
wait_us(SERIAL_FLUSH_TIME_US); // Wait for the serial buffers to flush.
wait_us(std::chrono::duration_cast<std::chrono::microseconds>(SERIAL_FLUSH_TIME_MS).count()); // Wait for the serial buffers to flush.
// Watchdog should fire before twice the timeout value.
wait_us(2 * TIMEOUT_US); // Device reset expected.
wait_us(2 * std::chrono::duration_cast<std::chrono::microseconds>(TIMEOUT_MS).count()); // Device reset expected.

// Watchdog reset should have occurred during a wait above.

Expand All @@ -279,7 +279,7 @@ utest::v1::status_t case_setup(const Case *const source, const size_t index_of_c
return utest::v1::greentea_case_setup_handler(source, index_of_case);
}

int testsuite_setup(const size_t number_of_cases)
utest::v1::status_t testsuite_setup(const size_t number_of_cases)
{
GREENTEA_SETUP(90, "watchdog_reset");
utest::v1::status_t status = utest::v1::greentea_test_setup_handler(number_of_cases);
Expand All @@ -306,7 +306,7 @@ int testsuite_setup(const size_t number_of_cases)

utest_printf("This test suite is composed of %i test cases. Starting at index %i.\n", number_of_cases,
current_case.start_index);
return current_case.start_index;
return static_cast<utest::v1::status_t>(current_case.start_index);
}

Case cases[] = {
Expand All @@ -321,7 +321,7 @@ Case cases[] = {
Case("Kicking the Watchdog prevents reset", case_setup, test_kick_reset),
};

Specification specification((utest::v1::test_setup_handler_t) testsuite_setup, cases);
Specification specification(testsuite_setup, cases);

int main()
{
Expand Down
2 changes: 1 addition & 1 deletion hal/tests/TESTS/mbed_hal/watchdog_timing/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ void test_timeout_lower_limit()
wait_us(sleep_time_ms * 1000);
hal_watchdog_kick();

if (send_reset_notification(&current_case, 2 * TIMEOUT_LOWER_LIMIT_MS) == false) {
if (!send_reset_notification(&current_case, 2 * TIMEOUT_LOWER_LIMIT_MS)) {
TEST_ASSERT_MESSAGE(0, "Dev-host communication error.");
return;
}
Expand Down
14 changes: 7 additions & 7 deletions targets/TARGET_RASPBERRYPI/TARGET_RP2040/watchdog_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ watchdog_status_t hal_watchdog_init(const watchdog_config_t *config)
{
watchdogConfig = *config;
// The pico watchdogs accept a maximum value of 0x7fffff
if ( config->timeout_ms < 0x1 && config->timeout_ms > 0x7FFFFF ) {
if ( config->timeout_ms < 0x1 || config->timeout_ms > 0x7FFFFF ) {
return WATCHDOG_STATUS_INVALID_ARGUMENT;
}

Expand All @@ -32,11 +32,7 @@ watchdog_status_t hal_watchdog_stop(void)

uint32_t hal_watchdog_get_reload_value(void)
{
uint32_t load_value = watchdogConfig.timeout_ms * 1000 * 2;
if (load_value > 0xffffffu) {
load_value = 0xffffffu;
}
return load_value;
return watchdogConfig.timeout_ms;
}

watchdog_features_t hal_watchdog_get_platform_features(void)
Expand All @@ -46,8 +42,12 @@ watchdog_features_t hal_watchdog_get_platform_features(void)
features.max_timeout = 0x7FFFFF;
features.update_config = true;
features.disable_watchdog = true;
return features;

// SDK configures the watchdog underlying counter to run at 1MHz
features.clock_typical_frequency = 1000000;
features.clock_max_frequency = 1000000;

return features;
}

#endif // DEVICE_WATCHDOG

0 comments on commit 42e0780

Please sign in to comment.