Skip to content

convert candidate shared ptrs to unique ptrs#1234

Merged
SteveMacenski merged 1 commit intoros-navigation:masterfrom
SteveMacenski:audit_shared
Oct 14, 2019
Merged

convert candidate shared ptrs to unique ptrs#1234
SteveMacenski merged 1 commit intoros-navigation:masterfrom
SteveMacenski:audit_shared

Conversation

@SteveMacenski
Copy link
Member

@SteveMacenski SteveMacenski commented Oct 12, 2019

Basic Info

Info Please fill out this column
Ticket(s) this addresses #1114
Primary OS tested on Ubuntu
Robotic platform tested on CI*
  • since just ptr type changes

Description of contribution in a few bullet points

  • changed any shared ptr that could be unique to unique.
  • Now only shared ptrs are needed as such or TF
  • frankly, we were in better shape than I suspected

Copy link

@bpwilcox bpwilcox left a comment

Choose a reason for hiding this comment

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

Just thinking, should we consider having the recoveries accept a pointer from the RecoveryServer to the CostmapSubscriber and FootprintSubscriber as opposed to created one themselves?

@SteveMacenski
Copy link
Member Author

SteveMacenski commented Oct 14, 2019

That's a good thought. If you file the ticket I'll probably get to it this week

I love it when different and random tests fail in both test runs :)

@bpwilcox
Copy link

That's a good thought. If you file the ticket I'll probably get to it this week

Filed #1239

@codecov
Copy link

codecov bot commented Oct 14, 2019

Codecov Report

Merging #1234 into master will increase coverage by 0.3%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #1234     +/-   ##
========================================
+ Coverage    37.7%     38%   +0.3%     
========================================
  Files         243     243             
  Lines       10993   10993             
  Branches     4288    4288             
========================================
+ Hits         4145    4178     +33     
+ Misses       4161    4118     -43     
- Partials     2687    2697     +10
Flag Coverage Δ
#project 38% <0%> (+0.3%) ⬆️
Impacted Files Coverage Δ
...v2_recoveries/include/nav2_recoveries/recovery.hpp 33.33% <0%> (ø) ⬆️
nav2_controller/src/nav2_controller.cpp 25.17% <0%> (+7.69%) ⬆️
nav2_dwb_controller/dwb_critics/src/path_align.cpp 60% <0%> (-25%) ⬇️
nav2_map_server/src/map_server.cpp 23.25% <0%> (-2.33%) ⬇️
nav2_recoveries/plugins/spin.cpp 59.64% <0%> (-1.76%) ⬇️
nav2_lifecycle_manager/src/lifecycle_manager.cpp 12.09% <0%> (ø) ⬆️
nav2_controller/src/progress_checker.cpp 60% <0%> (ø) ⬆️
nav2_amcl/src/amcl_node.cpp 35.26% <0%> (+0.16%) ⬆️
nav2_system_tests/src/planning/planner_tester.cpp 45.49% <0%> (+0.45%) ⬆️
nav2_costmap_2d/src/observation_buffer.cpp 40.59% <0%> (+0.99%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e09cabd...9a62d5a. Read the comment docs.

@SteveMacenski SteveMacenski merged commit 63afa48 into ros-navigation:master Oct 14, 2019
@Pana1v Pana1v mentioned this pull request Oct 8, 2025
8 tasks
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