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

Fix some targets fail to pass ticker overflow test #7548

Merged
merged 1 commit into from Jul 20, 2018

Conversation

Projects
None yet
6 participants
@ccli8
Contributor

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

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

@mprse

mprse approved these changes Jul 19, 2018

The fix looks good. Nice catch! Thanks!

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Jul 19, 2018

@cmonr cmonr requested a review from deepikabhavnani Jul 19, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 19, 2018

/morph build

@deepikabhavnani

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

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Jul 19, 2018

Contributor

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

This comment has been minimized.

@ccli8

ccli8 Jul 20, 2018

Contributor

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

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Jul 20, 2018

Contributor

No problem

@mbed-ci

This comment has been minimized.

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

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr added ready for merge and removed needs: CI labels Jul 19, 2018

@cmonr cmonr merged commit c1c5d89 into ARMmbed:master Jul 20, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 793 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10256 cycles (+99 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9960B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@ccli8 ccli8 deleted the OpenNuvoton:nuvoton_fix_ticker_overflow branch Jul 20, 2018

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018

Merge pull request ARMmbed#7548 from OpenNuvoton/nuvoton_fix_ticker_o…
…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