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

Remove boost shared ptrs #896

Closed

Conversation

Projects
None yet
4 participants
@bmagyar
Copy link
Member

commented May 15, 2018

Description

I've removed all actively used occurences of boost::shared_ptr.

It seems that the issue that kept us using it was the rviz API for fetching the tf listener. I have solved it by adding a conversion function (in separate commit) although it is questionable if we should stick to this rviz API at all. The updated API works with tf2. I think we should use that but it implies further refactoring of the PlanningSceneMonitor and everything related (which is pretty much everything else in MoveIt).

Checklist

  • Required: Code is auto formatted using clang-format
  • Optional: Decide if this should be cherry-picked to other current ROS branches (Indigo, Jade, Kinetic)
@bmagyar

This comment has been minimized.

Copy link
Member Author

commented May 15, 2018

I won't be able to adjust this PR in the coming week, feel free to touch it up if needed.

@bmagyar bmagyar requested a review from rhaschke May 15, 2018

@bmagyar bmagyar force-pushed the bmagyar:remove_boost_shared_ptrs branch from 8c5c862 to 970245c May 15, 2018

@IanTheEngineer

This comment has been minimized.

Copy link
Member

commented May 15, 2018

+1 for removing all instances of boost::shared_ptr. However, I see substantial overlap between the this PR and the tf2 migration PR that’s still under review: #830
That PR was the impetus for modifying the TransformListener API :)

@bmagyar

This comment has been minimized.

Copy link
Member Author

commented May 15, 2018

Awesome! Thanks for the heads up, I'll diff against that PR and see if there's anything left ;)

@bmagyar bmagyar force-pushed the bmagyar:remove_boost_shared_ptrs branch from 970245c to 3c27ed3 May 15, 2018

@bmagyar

This comment has been minimized.

Copy link
Member Author

commented May 15, 2018

Rebased on top of #830 !

@v4hn

This comment has been minimized.

Copy link
Member

commented May 15, 2018

looks good after #830 has been merged.

@rhaschke

This comment has been minimized.

Copy link
Collaborator

commented May 15, 2018

+1 from me too. Merge is blocked by #830.

@IanTheEngineer IanTheEngineer referenced this pull request May 17, 2018

Closed

Release to ROS Melodic #806

19 of 25 tasks complete

@rhaschke rhaschke added the Melodic label May 17, 2018

@bmagyar bmagyar force-pushed the bmagyar:remove_boost_shared_ptrs branch 2 times, most recently from 0cf3d4d to 4a67db1 May 17, 2018

@rhaschke
Copy link
Collaborator

left a comment

@bmagyar Please rebase this onto latest melodic-devel. Then I will merge.

@bmagyar bmagyar force-pushed the bmagyar:remove_boost_shared_ptrs branch from 4a67db1 to 949b883 May 18, 2018

@rhaschke

This comment has been minimized.

Copy link
Collaborator

commented May 20, 2018

Added to #906. Closing here.

@rhaschke rhaschke closed this May 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.