Skip to content
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

Add pluginlib support #41

Merged
merged 2 commits into from
Mar 10, 2017
Merged

Add pluginlib support #41

merged 2 commits into from
Mar 10, 2017

Conversation

jubeira
Copy link
Contributor

@jubeira jubeira commented Mar 9, 2017

This PR adds the plugin libraries to the static linkage list to use pluginlib support (valid since this PR was merged in roscpp_android).
It also proposes a fix to the race condition when loading parameters (move_base consumes the parameters before they are loaded into the server without this fix). This resolves #42.

Note: ParameterLoaderNode can be upstreamed once again and removed from the project once this is merged. This modification doesn't break the original API for the node.

-Added plugin libraries to static linkage in native node.
@jubeira jubeira requested a review from adamantivm March 9, 2017 19:17
Copy link
Member

@adamantivm adamantivm left a comment

Choose a reason for hiding this comment

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

couple nits then LGTM

CountDownLatch latch = new CountDownLatch(1);
startParameterLoaderNode(latch);

while (latch.getCount() > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

why a busy look here? Use the latch wait mechanism instead.

try {
Thread.sleep(1000);
} catch (InterruptedException ie) {

Copy link
Member

Choose a reason for hiding this comment

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

maybe print some warning?

@jubeira
Copy link
Contributor Author

jubeira commented Mar 10, 2017

Thanks for the review; comments addressed!
Merging now

@jubeira jubeira merged commit f6c67c8 into ekumenlabs:master Mar 10, 2017
@jubeira jubeira deleted the fix/parameterloader_sync branch March 10, 2017 13:33
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.

Racing condition for Parameter Loader
2 participants