FPGA CI shield: Add a watchdog timing test #11433

fkjagodzinski · 2019-09-06T13:18:35Z

Description

Replace the tests-mbed_hal-watchdog_timing with ~~tests-mbed_hal_fpga_ci_test_shield-watchdog_timing~~ tests-mbed_timing_fpga_ci_test_shield-watchdog.

The timeout measurement from the original test is based on heartbeat messages sent to a host PC via the greentea serial. This method is fairly inaccurate because the last timestamp received by the host will always be before the expected reset time. Thanks to IOMetrics feature of our new FPGA CI test shield, the watchdog timeout measurement boils down to counting the falling edges of the heartbeat signal. The pin used to output this signal is added to a list of restricted pins. This guarantees that no other signals (i.e. FPGA control) will interfere.

Pull request type

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

Reviewers

@mprse, @maciejbocianski, @jamesbeyond

Release Notes

ciarmcom · 2019-09-06T15:00:42Z

@fkjagodzinski, thank you for your changes.
@maciejbocianski @mprse @jamesbeyond @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

0xc0170 · 2019-09-17T09:29:39Z

@mprse, @maciejbocianski, @jamesbeyond

Please review

fkjagodzinski · 2019-09-24T13:49:05Z

@mprse, @maciejbocianski, @jamesbeyond

Please review

Ping

adbridge · 2019-09-27T15:10:47Z

@mprse, @maciejbocianski, @jamesbeyond we are still awaiting review on this , could you please take a look asap?

mprse

The test is okay, but I think we should decide:

where to put the timing tests,
do we want to remove old tests (which use host pc to count elapsed time)

The same problem I have here: PR #11147.

Currently, all FPGA tests are in the following location:
TESTS\mbed_hal_fpga_ci_test_shield

I'm not sure if this is a good idea to put alternative timing tests in the same location together with peripheral fpga tests. My proposition is to create an additional folder for the timing tests. Something like:
TESTS\mbed_timing_fpga_ci_test_shield.

Alternatively, we may also break the current convention and add FPGA tests close too non-FPGA versions:
TESTS\mbedmicro-rtos-mbed\basic
TESTS\mbedmicro-rtos-mbed\basic_fpga.

I think we should not remove the original tests since they might be valuable to users who do not have FPGA shields. On CI we may run only one test version depending if FPGA is supported for the board.

Use the FPGA IOMetrics feature to verify the watchdog timeout accuracy.

fkjagodzinski · 2019-10-14T14:34:12Z

Ok, I rebased this patch to current master (079564b) and removed the last commit that deleted the original, non-FPGA test.

Regarding the test location -- I do not have a strong opinion about this as long all FPGA tests have the string fpga in their names (which makes them easier to filter). @jamesbeyond, what do you think -- should we have a separate dir for all timing tests as @mprse proposed?

jamesbeyond · 2019-10-14T14:41:39Z

I think put FPGA timing related test into one folder would ideal.
TESTS\mbed_timing_fpga_ci_test_shield
easy to manage the test and, clear enough to identify these test will require an FPGA shield.

Create a directory for all timing tests that make use of the FPGA shield, regardless of the component they test. Move the FPGA-Watchdog-timing test.

fkjagodzinski · 2019-10-14T15:12:44Z

@jamesbeyond, @mprse, I moved the test to a new directory. The full test suite name is now tests-mbed_timing_fpga_ci_test_shield-watchdog.

0xc0170 · 2019-10-23T13:00:27Z

Please review

Lets run one CI meanwhile

mbed-ci · 2019-10-23T15:32:31Z

Test run: FAILED

Summary: 1 of 5 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2019-10-24T09:34:10Z

Watchdog test failed, please review

fkjagodzinski · 2019-10-30T15:39:34Z

Turns out the failures are pretty straight forward to explain this time. :)

Because this new test suite was moved to a new dir (as requested above), it has to be filtered out in the CI until we provide the FPGA shields.

From the logs I can tell we have a filter with tests-mbed_hal_fpga* pattern. We have to add tests-mbed_timing_fpga* as well.

0xc0170 · 2019-11-26T12:09:52Z

@fkjagodzinski Please update when you can

yarbcy · 2019-12-13T15:28:48Z

@0xc0170 Good to have this test. Do you plan to merge it?

yarbcy · 2019-12-13T15:29:27Z

@fkjagodzinski

fkjagodzinski · 2019-12-13T15:31:45Z

Yes @yarbcy this requires a minor update. Will take care of this shortly.

fkjagodzinski · 2019-12-13T16:10:19Z

https://github.com/ARMmbed/mbed-os-ci/pull/1084 should fix the greentea failures on the CI.

adbridge · 2019-12-24T11:57:37Z

ARMmbed/mbed-os-ci#1084 should fix the greentea failures on the CI.

The aforementioned CI update is still pending so we can't take this yet.

0xc0170 · 2020-01-03T13:22:17Z

ARMmbed/mbed-os-ci#1084 should fix the greentea failures on the CI.

Integrated, will restart CI

mbed-ci · 2020-01-03T15:02:08Z

Test run: SUCCESS

Summary: 5 of 5 test jobs passed
Build number : 2
Build artifacts

0xc0170 · 2020-01-03T15:03:40Z

@mprse, @maciejbocianski, @jamesbeyond

Please review

maciejbocianski · 2020-01-08T14:50:01Z

Test LGTM

Regarding FPGA test placement I think that creating another folder for timing FPGA tests mbed_timing_fpga_ci_test_shield-watchdog is pointless. What are pros for doing this?

For me ideal would be to have fpga and non-fpga tests in the same place. In the near future FPGA shield become inherent part of test infrastructure and having the same functionality tests gathered in one place will make life easier.

\TESTS\mbed_hal\watchdog
\TESTS\mbed_hal\watchdog_fpga
\TESTS\mbed_hal\watchdog_timing
\TESTS\mbed_hal\watchdog_timing_fpga

What do you think @jamesbeyond @fkjagodzinski @mprse

fkjagodzinski · 2020-01-09T10:07:29Z

As I said earlier, I don't have a strong opinion about the dir structure. I like your proposal @maciejbocianski. 👍 Isn't this a topic for a new PR though? This would require to move all the tests that make use of the FPGA shield.

@jamesbeyond, @mprse shall I keep the current timing-specific dir, or revert to the original location?

0xc0170 · 2020-01-09T12:35:25Z

I believe restructure would be a new PR.

@jamesbeyond, @mprse shall I keep the current timing-specific dir, or revert to the original location?

As soon as this is decided, this PR is ready for integration, isn't it?

fkjagodzinski · 2020-01-09T12:54:09Z

As soon as this is decided, this PR is ready for integration, isn't it?

Yep, correct.

jamesbeyond · 2020-01-10T09:57:39Z

As I said earlier, I don't have a strong opinion about the dir structure. I like your proposal @maciejbocianski. 👍 Isn't this a topic for a new PR though? This would require to move all the tests that make use of the FPGA shield.

@jamesbeyond, @mprse shall I keep the current timing-specific dir, or revert to the original location?

I think we can keep the test as it is in the TESTS\mbed_timing_fpga_ci_test_shield for now, as you already to PR to the CI merged,
Regarding what @maciejbocianski proposed, I don't have a very strong opinion on this as well. Anyway, we are going to have a folder restructure task soon. we can leave the discussion for that task.

mbed-ci · 2020-01-10T16:46:38Z

Test run: SUCCESS

Summary: 5 of 5 test jobs passed
Build number : 3
Build artifacts

ciarmcom requested review from a team, jamesbeyond, maciejbocianski and mprse September 6, 2019 15:00

ciarmcom added the needs: review label Sep 6, 2019

0xc0170 approved these changes Sep 17, 2019

View reviewed changes

mprse suggested changes Oct 7, 2019

View reviewed changes

adbridge added needs: work and removed needs: review labels Oct 7, 2019

Test: FPGA shield: Add a Watchdog-timing test

7f50168

Use the FPGA IOMetrics feature to verify the watchdog timeout accuracy.

fkjagodzinski force-pushed the fpga-watchdog_timing branch from b63189a to 7f50168 Compare October 14, 2019 14:24

Test: FPGA shield: Add directory for timing tests

763d7e8

Create a directory for all timing tests that make use of the FPGA shield, regardless of the component they test. Move the FPGA-Watchdog-timing test.

0xc0170 added needs: review and removed needs: work labels Oct 18, 2019

0xc0170 requested review from 0xc0170 and mprse October 18, 2019 12:49

0xc0170 approved these changes Oct 21, 2019

View reviewed changes

0xc0170 removed the needs: review label Oct 24, 2019

0xc0170 added the needs: work label Oct 24, 2019

0xc0170 added needs: CI and removed needs: work labels Jan 3, 2020

0xc0170 added needs: review and removed needs: CI labels Jan 3, 2020

0xc0170 added needs: CI and removed needs: review labels Jan 10, 2020

maciejbocianski approved these changes Jan 12, 2020

View reviewed changes

0xc0170 added ready for merge release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 and removed needs: CI ready for merge labels Jan 13, 2020

0xc0170 merged commit 4922d10 into ARMmbed:master Jan 15, 2020

fkjagodzinski deleted the fpga-watchdog_timing branch January 15, 2020 12:46

FPGA CI shield: Add a watchdog timing test #11433

FPGA CI shield: Add a watchdog timing test #11433

Uh oh!

Conversation

fkjagodzinski commented Sep 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Sep 6, 2019

Uh oh!

0xc0170 commented Sep 17, 2019

Uh oh!

fkjagodzinski commented Sep 24, 2019

Uh oh!

adbridge commented Sep 27, 2019

Uh oh!

mprse left a comment

Choose a reason for hiding this comment

Uh oh!

fkjagodzinski commented Oct 14, 2019

Uh oh!

jamesbeyond commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fkjagodzinski commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Oct 23, 2019

Uh oh!

mbed-ci commented Oct 23, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Oct 24, 2019

Uh oh!

fkjagodzinski commented Oct 30, 2019

Uh oh!

0xc0170 commented Nov 26, 2019

Uh oh!

yarbcy commented Dec 13, 2019

Uh oh!

yarbcy commented Dec 13, 2019

Uh oh!

fkjagodzinski commented Dec 13, 2019

Uh oh!

fkjagodzinski commented Dec 13, 2019

Uh oh!

adbridge commented Dec 24, 2019

Uh oh!

0xc0170 commented Jan 3, 2020

Uh oh!

mbed-ci commented Jan 3, 2020

Test run: SUCCESS

Uh oh!

0xc0170 commented Jan 3, 2020

Uh oh!

maciejbocianski commented Jan 8, 2020

Uh oh!

fkjagodzinski commented Jan 9, 2020

Uh oh!

0xc0170 commented Jan 9, 2020

Uh oh!

fkjagodzinski commented Jan 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamesbeyond commented Jan 10, 2020

Uh oh!

mbed-ci commented Jan 10, 2020

Test run: SUCCESS

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

fkjagodzinski commented Sep 6, 2019 •

edited

Loading

jamesbeyond commented Oct 14, 2019 •

edited

Loading

fkjagodzinski commented Oct 14, 2019 •

edited

Loading

fkjagodzinski commented Jan 9, 2020 •

edited

Loading