Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

roscpp: Adding AsyncSpinner::canStart to check if a spinner can be started#377

Closed
jbohren wants to merge 1 commit intoros:indigo-develfrom
jhu-lcsr-forks:async-spinner-can-start
Closed

roscpp: Adding AsyncSpinner::canStart to check if a spinner can be started#377
jbohren wants to merge 1 commit intoros:indigo-develfrom
jhu-lcsr-forks:async-spinner-can-start

Conversation

@jbohren
Copy link
Copy Markdown
Member

@jbohren jbohren commented Mar 11, 2014

This adds the introspection described in #277

Should a PR also be made against hydro?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please simplify the code:

return spinlock.owns_lock();

@jbohren
Copy link
Copy Markdown
Member Author

jbohren commented Mar 11, 2014

Corrections made.

@tfoote
Copy link
Copy Markdown
Member

tfoote commented Mar 11, 2014

+1

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This error message actually can't be this precise. It's possible that the other AsyncSpinner was created with a non-default callback queue, so not even the default queue will be serviced.

@jbohren
Copy link
Copy Markdown
Member Author

jbohren commented Mar 11, 2014

I've updated the warning message to better characterize the issue with starting two AsyncSpinners.

@dirk-thomas
Copy link
Copy Markdown
Member

+1 Manually merged in e6f48a8 with a minor whitespace change.

@dirk-thomas
Copy link
Copy Markdown
Member

I will cherry-pick the change to the hydro-devel branch once it passed CI - you don't have to create a PR for that.

@jbohren
Copy link
Copy Markdown
Member Author

jbohren commented Mar 11, 2014

I will cherry-pick the change to the hydro-devel branch once it passed CI - you don't have to create a PR for that.

Cool. Thanks, Dirk.

@dirk-thomas
Copy link
Copy Markdown
Member

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants