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

Use a shared_ptr for the dynamic reconfigure pointer, and create it w… #358

Merged

Conversation

lucasw
Copy link
Contributor

@lucasw lucasw commented Sep 22, 2018

…ith the private node handle so that the parameters for the dynamic reconfigure server are in the private namespace and two image publishers can coexist in the same manager #357

This may break parameters that were set in the nodelet manager namespace as a workaround in some launch files out there.

lucasw added a commit to lucasw/image_pipeline that referenced this pull request Oct 21, 2018
…ory> to make build system build it anyhow), also use typedef for callback ros-perception#358
@JWhitleyWork
Copy link
Collaborator

@lucasw - Thanks for the PR! I know this is a bit old but, would you mind removing your custom launch file and rebasing this on melodic? Also, I'm OK with the use of std::shared_ptr<>. We should be upgrading to it across the code-base for melodic anyway.

@JWhitleyWork JWhitleyWork changed the base branch from indigo to melodic April 25, 2019 14:19
@lucasw
Copy link
Contributor Author

lucasw commented Apr 25, 2019

Wouldn't you rather take this as a boost shared_ptr, then have a separate PR that switches out all the boost shared_ptrs for all of image_pipeline? (created an issue #407) It seems odd to just have one std shared_ptr and then 40 boost shared_ptrs.

@JWhitleyWork
Copy link
Collaborator

@lucasw - That's a good point. Thanks for the update. Would you mind rebasing this on melodic to grab the new CI? After, I'll have one other person review and then we'll merge.

@lucasw lucasw force-pushed the image_pub_dr_private_namespace branch from 1e67767 to b467ca1 Compare April 25, 2019 14:51
lucasw added a commit to lucasw/image_pipeline that referenced this pull request Apr 25, 2019
…ory> to make build system build it anyhow), also use typedef for callback ros-perception#358
lucasw added a commit to lucasw/image_pipeline that referenced this pull request Apr 25, 2019
…ith the private node handle so that the parameters for the dynamic reconfigure server are in the private namespace and two image publishers can coexist in the same manager ros-perception#357
@lucasw lucasw force-pushed the image_pub_dr_private_namespace branch from 295ae7c to 3427b70 Compare April 25, 2019 14:57
@lucasw
Copy link
Contributor Author

lucasw commented Apr 25, 2019

I've removed the launch file, rebased to melodic, and squashed. I left a backup of the launch file for myself in https://github.com/lucasw/image_pipeline/tree/indigo_image_pub_dr_private_namespace .

@JWhitleyWork JWhitleyWork merged commit eea2c59 into ros-perception:melodic Apr 25, 2019
generush pushed a commit to generush/image_pipeline that referenced this pull request May 9, 2019
…e_namespace

Use a shared_ptr for the dynamic reconfigure pointer, and create it w…
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.

None yet

3 participants