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

Remove direct use of us and lp ticker from tests #5096

Merged
merged 1 commit into from Sep 21, 2017

Conversation

Projects
None yet
5 participants
@c1728p9
Contributor

c1728p9 commented Sep 14, 2017

Remove the direct use of the microsecond and low power ticker from the tests. This enforces that sleep mode is properly locked when using timers. Furthermore, this prepares the codebase for new ticker changes which allow differing clock frequencies and timer bit widths.

Remove direct use of us and lp ticker from tests
Remove the direct use of the microsecond and low power ticker from
the tests. This enforces that sleep mode is properly locked when
using timers. Furthermore, this prepares the codebase for new ticker
changes which allow differing clock frequencies and timer bit widths.
@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Sep 14, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Sep 14, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1304

All builds and test passed!

@0xc0170 0xc0170 removed the needs: review label Sep 19, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 19, 2017

@pan-

In the lp_ticker test, it is a bit strange to use a combination of the C and C++ API. I'd suggest to explain when one has to be used over, that would help future maintainers.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 20, 2017

In the lp_ticker test, it is a bit strange to use a combination of the C and C++ API. I'd suggest to explain when one has to be used over, that would help future maintainers.

+1. how shall this be suggested? In the test, explaine why Timer is used instead of directly tickers?

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Sep 20, 2017

In general (not including the code under test) we need to encourage tests to be written using the C++ API rather than the HAL API. This is because the C++ API's provide thread and sleep safety and mbed-os is committed to keeping these APIs backwards compatible. With the HAL APIs there is more flexibility to change over time.

In this PR the HAL API is used only in TESTS/mbed_hal/lp_ticker/main.cpp. This is because it is the code being tested. The C++ API is also used to aid in the testing, but it itself is not the focus of the testing. All other files in this PR should be using the C++ APIs only.

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Sep 20, 2017

/morph test-nightly

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 21, 2017

@c1728p9 Will this be needed for 5.6.1?

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Sep 21, 2017

nope

@mbed-bot

This comment has been minimized.

mbed-bot commented Sep 21, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1349

All builds and test passed!

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Sep 21, 2017

@0xc0170 0xc0170 merged commit 8acfcfb into ARMmbed:master Sep 21, 2017

5 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
ci/morph-test-nightly Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@c1728p9 c1728p9 deleted the c1728p9:remove_direct_ticker_use branch Sep 22, 2017

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