Skip to content

Fix some targets fail to pass ticker overflow test #7548

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
merged 1 commit into from
Jul 20, 2018

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Jul 19, 2018

Description

In mbed-os-tests-mbed_hal-common_tickers/Microsecond ticker overflow test, some targets would fail to catch specified ticker value near overflow in time and so fail. This commit alleviates the issue by checking ticker value range rather than one exact ticker value near overflow.

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@deepikabhavnani

In mbed-os-tests-mbed_hal-common_tickers/Microsecond ticker overflow test, some targets
would fail to catch specified ticker value near overflow in time and so fail. This commit
alleviates the issue by checking ticker value range rather than one exact ticker value near
overflow.
@0xc0170 0xc0170 requested a review from mprse July 19, 2018 06:14
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.

The fix looks good. Nice catch! Thanks!

@cmonr
Copy link
Contributor

cmonr commented Jul 19, 2018

/morph build

Copy link

@deepikabhavnani deepikabhavnani 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 to me

#define LP_TICKER_OVERFLOW_DELTA1 0 // this will allow to detect that ticker counter rollovers to 0
#define LP_TICKER_OVERFLOW_DELTA2 0
#define US_TICKER_OVERFLOW_DELTA1 50
#define US_TICKER_OVERFLOW_DELTA2 60

Choose a reason for hiding this comment

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

It will be good to name the defines as min/max instead 1/2 to have better understanding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@deepikabhavnani Because this PR has merged, I would make no modification for it.

Choose a reason for hiding this comment

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

No problem

@mbed-ci
Copy link

mbed-ci commented Jul 19, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Jul 19, 2018

@mbed-ci
Copy link

mbed-ci commented Jul 19, 2018

@cmonr cmonr merged commit c1c5d89 into ARMmbed:master Jul 20, 2018
@ccli8 ccli8 deleted the nuvoton_fix_ticker_overflow branch July 20, 2018 02:00
pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018
…verflow

Fix some targets fail to pass ticker overflow test
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