Skip to content

Update the Watchdog & Reset_reason host test scripts #8846

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

Merged

Conversation

fkjagodzinski
Copy link
Member

Description

Update the host test scripts used by Watchdog and Reset_reason HAL tests to use forced_reset_timeout config parameter instead of program_cycle_s. This is based on PR #8309, which explains such change refering to https://github.com/ARMmbed/htrun/blob/1c570ec77d8c5673dae55dc790302d86d712c36b/mbed_host_tests/__init__.py#L132-L144

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[x] Test update
[ ] Breaking change

Filip Jagodzinski added 2 commits November 22, 2018 20:43
Move the calls to get_config_item() to the setup() method. The config is
not available in the initializer yet.
Change the config parameter used as a delay before sending the sync
packet after the device reset in watchdog and reset_reason tests. Use
'forced_reset_timeout' instead of 'program_cycle_s'.
Copy link
Contributor

@donatieng donatieng left a comment

Choose a reason for hiding this comment

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

Looks good @fkjagodzinski, thanks!

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 23, 2018

This one needs to land for 8801 to pass ? Or it's just cosmetic update?

@donatieng
Copy link
Contributor

@0xc0170 We should get it in before #8801 unless we should hold on?

@fkjagodzinski
Copy link
Member Author

This one needs to land for 8801 to pass ? Or it's just cosmetic update?

#8801 should be OK without this patch. I'd consider this PR as a refactor to keep the code aligned with other host script changed by #8309.

@donatieng
Copy link
Contributor

@0xc0170 as we're bringing that feature branch to master and CI is heavily loaded; could we merge this one to the feature branch and gate the whole branch on CI (through #8801)?

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 23, 2018

Sounds good to me 👍

@mbed-ci
Copy link

mbed-ci commented Nov 25, 2018

Test run: SUCCESS

Summary: 3 of 3 test jobs passed
Build number : 1
Build artifacts
Build logs

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 26, 2018

As this is ready, I'll merge it as we are waiting for reviews in 8801 (I am reviewing now).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants