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

Test: deepsleep() API replacement #5147

Merged
merged 1 commit into from Oct 13, 2017

Conversation

Projects
None yet
7 participants
@0xc0170
Member

0xc0170 commented Sep 20, 2017

Use sleep() as entry function + check to be certain we
are entering deepsleep when required by test (should be allowed as we use LowPower object).

This has dependency on fixing this issue #5076 (the test now fails, and will be fixed as today).

Tested using K64F and GCC ARM , these 2 tests affected, one failure at the moment

@jeromecoutant

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Sep 20, 2017

In these tests, we should not allow deepsleep when sleep() is called ?

/* Make sure deepsleep is allowed, to go to deepsleep */
bool deep_sleep_allowed = sleep_manager_can_deep_sleep();
TEST_ASSERT_TRUE_MESSAGE(deep_sleep_allowed, "Deep sleep should be allowed");
sleep();

This comment has been minimized.

@0xc0170

0xc0170 Sep 20, 2017

Member

this is assuming that using LowPower object does allow deepsleep, and using sleep() should go into deepsleep, but would it? :-) As this test is for drivers, this looks fine, what about the below test for hal? Same? no direct deepsleep hal call ?

This comment has been minimized.

@jeromecoutant

jeromecoutant Sep 20, 2017

Contributor

I was speaking about lp_timeout_1s_sleep function.

This comment has been minimized.

@0xc0170

0xc0170 Sep 20, 2017

Member

I was speaking about lp_timeout_1s_sleep function.

Should have lock there.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 20, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 20, 2017

In these tests, we should not allow deepsleep when sleep() is called ?

Both changes are changes for deepsleep test cases, so deepsleep should be allowed and should be entered?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 28, 2017

Depends on #5148

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

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 4, 2017

As the dependency was merged, this is opened for reviews

@mprse

@mprse

This comment has been minimized.

Member

mprse commented Oct 5, 2017

Provided updates for handling deep-sleep mode are valid.
Missing test case description.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 5, 2017

Missing test case description.

That's true, they should be documented. I can add a commit here, however might be better as a new patch

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Oct 5, 2017

Hi
For me, there are 3 parts in this test:
1- LPT test in run mode
2- LPT test in sleep mode
3- LPT test in deep sleep mode

I assume current update is OK to force part 3.
For part 2, we should also forbid deepsleep ?

@0xc0170 0xc0170 force-pushed the 0xc0170:fix_deepsleep_tests branch 2 times, most recently Oct 5, 2017

TESTS/mbed_drivers/lp_timeout/main.cpp Outdated
@@ -75,6 +78,8 @@ void lp_timeout_1s_sleep(void)
sleep_manager_lock_deep_sleep();
timestamp_t start = us_ticker_read();
lpt.attach(&cb_done, 1);
bool deep_sleep_allowed = sleep_manager_can_deep_sleep();
TEST_ASSERT_FALSE_MESSAGE(deep_sleep_allowed, "Deep sleep should be allowed");

This comment has been minimized.

@fkjagodzinski

fkjagodzinski Oct 5, 2017

Member

Since deepsleep() is not allowed now (as it should) the message is invalid.

@0xc0170 0xc0170 force-pushed the 0xc0170:fix_deepsleep_tests branch Oct 5, 2017

Test: deepsleep() API replacement
Use sleep() as entry function + check to be certain we
are entering deepsleep when required by test (should be allowed)

@0xc0170 0xc0170 force-pushed the 0xc0170:fix_deepsleep_tests branch to b30c622 Oct 5, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 6, 2017

@fkjagodzinski Can you review again pls?

@fkjagodzinski

fkjagodzinski approved these changes Oct 6, 2017 edited

Missing test case description.

Please don't bother with comments for LowPowerTimeout test cases. I'll add them in #5106.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 9, 2017

/morph test-nightly

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Oct 9, 2017

@mbed-bot

This comment has been minimized.

mbed-bot commented Oct 9, 2017

Result: SUCCESS

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

/morph test-nightly

Output

mbed Build Number: 1551

All builds and test passed!

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 10, 2017

retest uvisor

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Oct 11, 2017

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 11, 2017

Build : SUCCESS

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

Triggering tests

/test mbed-os

@mbed-ci

This comment has been minimized.

@theotherjimmy theotherjimmy merged commit 84f2d08 into ARMmbed:master Oct 13, 2017

6 checks passed

Cam-CI uvisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-test test 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment