Skip to content

Make CPU stats test conditional on new LP ticker #7004

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

Closed
wants to merge 1 commit into from

Conversation

kjbracey
Copy link
Contributor

@kjbracey kjbracey commented May 24, 2018

Description

Pending the new LP ticker HAL changes, we're still seeing test issues on
the NRF51.

On the assumption that the LP ticker PR will fix it, and knowing that it
is currently lacking the necessary change to DEVICE_LPTICKER for this
test, change the test now.

This deactivates the test, and it will be reenabled when the LP ticker
PR lands, rather than being deactivated by it.

Pull request type

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

0xc0170
0xc0170 previously approved these changes May 24, 2018
@kjbracey kjbracey dismissed stale reviews from 0xc0170 and deepikabhavnani via 746d682 May 24, 2018 11:55
@kjbracey kjbracey changed the title CPU idle test: further increase sleep Make CPU stats test conditional on new LP ticker May 24, 2018
@kjbracey
Copy link
Contributor Author

kjbracey commented May 24, 2018

Redone - no longer convinced 100ms will help. Flip configuration so test goes off until #6995 lands.

(PS @bulislaw - you may want to rebase and check for other things that need to be updated to DEVICE_LPTICKER. Either don't edit this file, or make sure you edit it identically to avoid conflicts ).

@kjbracey kjbracey requested a review from bulislaw May 24, 2018 11:58
Pending the new LP ticker HAL changes, we're still seeing test issues on
the NRF51.

On the assumption that the LP ticker PR will fix it, and knowing that it
is currently lacking the necessary change to DEVICE_LPTICKER for this
test, change the test now.

This deactivates the test, and it will be reenabled when the LP ticker
PR lands, rather than being deactivated by it.
@kjbracey
Copy link
Contributor Author

We've confirmed the CI failure was from a build that completed before #6993 was merged in, so I'm now satisfied there's no remaining problem and #6993 fixed it. The 1ms was clearly wrong, as that could be a basically zero delay if the next tick happens as the call is made. 3ms should be fine.

@bulislaw - Now do update this file along with everything else.

@kjbracey kjbracey closed this May 24, 2018
@kjbracey kjbracey deleted the idletest100 branch May 24, 2018 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants