Skip to content

Ticker: add dtor implementation #5217

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

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Sep 28, 2017

This depends on #5148.
I'll rebase once that PR is integrated (CI now fails !), and will tag for review/tests

This addresses the issue reported here (#5067). If Ticker locked deepsleep, it should unlock it (clean its resources in this case) in dtor.

Should it also invoke remove() in dtor?

@0xc0170
Copy link
Contributor Author

0xc0170 commented Sep 28, 2017

Similar issue is in SerialBase - dtor is empty thus can lead to keeping locked deep sleep - that is not the intention. However that class is a separate issue (as it has multiple options how it could lock). Will be dealt with via separate patch

@@ -22,6 +22,12 @@

namespace mbed {

void Ticker::~Ticker() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably call detach to ensure no more interrupts can fire.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Dtor should invoke detach
@0xc0170
Copy link
Contributor Author

0xc0170 commented Sep 29, 2017

Rebased (dtor invokes detach to clean its resources), resolved conflicts, now ready for CI

@0xc0170
Copy link
Contributor Author

0xc0170 commented Sep 29, 2017

/morph test

@mbed-bot
Copy link

Result: ABORTED

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

/morph test

@0xc0170
Copy link
Contributor Author

0xc0170 commented Sep 29, 2017

Not valid anymore, rebase resolved it !

@0xc0170 0xc0170 closed this Sep 29, 2017
@0xc0170 0xc0170 removed the needs: CI label Sep 29, 2017
@0xc0170 0xc0170 deleted the fix_ticker_dtor branch September 29, 2017 08:38
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