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.

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

Jump to bottom

Nuvoton: Fix Greentea test common_tickers failed #8030

Merged
merged 4 commits into from
Sep 21, 2018
Merged

Nuvoton: Fix Greentea test common_tickers failed #8030

merged 4 commits into from
Sep 21, 2018

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Sep 7, 2018

Description

This PR tries to fix mbed-os-tests-mbed_hal-common_tickers test failed, especially on Nuvoton targets:

  1. Redeem from missing go-off ticker interrupt when us/lp ticker layers are uninstalled
  2. Disable ticker interrupt which would interfere with fire_interrupt speed test
  3. Enlarge test period to avoid timeout error

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

@0xc0170 0xc0170 requested review from mprse and a team September 7, 2018 09:07
@ccli8
[Nuvoton] Fix spurious us_ticker/lp_ticker interrupts
9ee83f5 
If us_ticker/lp_ticker is scheduled and then the interrupt is disabled, the originally scheduled
interrupt may still become pending. If this occurs, then an interrupt will fire twice on the next
call to us_ticker_set_interrupt/lp_ticker_set_interrupt - once immediately and then a second time
at the appropriate time.

This patch prevents the first interrupt by clearing interrupts in
us_ticker_set_interrupt/lp_ticker_set_interrupt before calling NVIC_EnableIRQ.
@ccli8
[Nuvoton] Fix lp_ticker_free cannot pass speed test
6639a92
@ccli8
[Nuvoton] Synchronize us_ticker to lp_ticker
ab7b93c 
This is to make implementations of us_ticker/lp_ticker consistent.
@ccli8 ccli8 force-pushed the nuvoton_fix_common_tickers_fail branch from 55284b8 to 9a003fc Compare September 10, 2018 06:19
@ccli8
Copy link
Contributor Author

ccli8 commented Sep 10, 2018

I do rebase and push some commits for Nuvoton targets:

  1. Fix spurious us_ticker/lp_ticker interrupts
  2. Fix lp_ticker_free cannot pass speed test

@ccli8 ccli8 changed the title Fix Greentea test common_tickers failed Nuvoton: Fix Greentea test common_tickers failed Sep 10, 2018
@ccli8 ccli8 mentioned this pull request Sep 10, 2018
Closed
mprse
mprse approved these changes Sep 10, 2018
View reviewed changes
Copy link
Contributor

@mprse mprse left a comment

Choose a reason for hiding this comment

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

Approved test changes.

@cmonr cmonr requested a review from c1728p9 September 10, 2018 18:37
c1728p9
c1728p9 reviewed Sep 17, 2018
View reviewed changes
Copy link
Contributor

@c1728p9 c1728p9 left a comment

Choose a reason for hiding this comment

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

I left comments about some code which is unnecessary with #7524. Otherwise this PR looks good.

TESTS/mbed_hal/common_tickers/main.cpp Outdated
* the armed alarm won't go off until another ticker interrupt goes off. Manually firing one ticker interrupt
* can alleviate the issue.
*/
us_ticker_fire_interrupt();
Copy link
Contributor

Choose a reason for hiding this comment

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

This call to us_ticker_fire_interrupt should no longer be needed with the fix #7524. The call to ticker_resume earlier in this function should restart the common ticker layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@c1728p9 I remove the unnecessary code with #7524 in 4888ff2.

TESTS/mbed_hal/common_tickers/main.cpp Outdated
@@ -563,13 +579,16 @@ utest::v1::status_t lp_ticker_teardown(const Case *const source, const size_t pa
ticker_resume(get_lp_ticker_data());
osKernelResume(0);

/* Redeem from missing go-off ticker interrupt (same as us ticker above) */
lp_ticker_fire_interrupt();
Copy link
Contributor

Choose a reason for hiding this comment

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

This call to us_ticker_fire_interrupt should no longer be needed with the fix #7524. The call to ticker_resume earlier in this function should restart the common ticker layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@c1728p9 Fixed as above.

@ccli8
Fix Greentea test common_tickers failed
4888ff2 
1. Disable ticker interrupt which would interfere with fire_interrupt speed test
2. Enlarge test period to avoid timeout error
@ccli8 ccli8 force-pushed the nuvoton_fix_common_tickers_fail branch from 9a003fc to 4888ff2 Compare September 18, 2018 06:59
@0xc0170
Copy link
Contributor

0xc0170 commented Sep 19, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Sep 19, 2018

Build : SUCCESS

Build number : 3108
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8030/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Sep 19, 2018

Exporter Build : SUCCESS

Build number : 2715
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8030/

@mbed-ci
Copy link

mbed-ci commented Sep 19, 2018

Test : FAILURE

Build number : 2897
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8030/2897

@cmonr
Copy link
Contributor

cmonr commented Sep 20, 2018

Failures were due to a faulty board providing a false enumerated device.
/morph test

@mbed-ci
Copy link

mbed-ci commented Sep 21, 2018

Test : FAILURE

Build number : 2914
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8030/2914

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 21, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Sep 21, 2018

Test : SUCCESS

Build number : 2917
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8030/2917

@0xc0170 0xc0170 merged commit ab882c3 into ARMmbed:master Sep 21, 2018
@ccli8 ccli8 deleted the nuvoton_fix_common_tickers_fail branch October 2, 2018 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mprse mprse mprse approved these changes

@c1728p9 c1728p9 c1728p9 approved these changes

Assignees
No one assigned
Labels
release-version: 5.10.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ccli8 @0xc0170 @mbed-ci @cmonr @c1728p9 @mprse