-
Notifications
You must be signed in to change notification settings - Fork 650
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
refactor(mission_planner): prepare to support ad api #1561
refactor(mission_planner): prepare to support ad api #1561
Conversation
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Codecov Report
@@ Coverage Diff @@
## main #1561 +/- ##
==========================================
- Coverage 10.74% 10.73% -0.01%
==========================================
Files 1115 1117 +2
Lines 78815 78883 +68
Branches 18559 18559
==========================================
Hits 8465 8465
- Misses 61630 61698 +68
Partials 8720 8720
*This pull request uses carry forward flags. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -15,7 +15,7 @@ | |||
<!-- mission planning module --> | |||
<group> | |||
<push-ros-namespace namespace="mission_planning"/> | |||
<include file="$(find-pkg-share tier4_planning_launch)/launch/mission_planning/mission_planning.launch.py"/> | |||
<include file="$(find-pkg-share tier4_planning_launch)/launch/mission_planning/mission_planning.launch.xml"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will stop using ComposableNodeContainer
for mission_planner::MissionPlannerLanelet2
and mission_planner::GoalPoseVisualizer
.
That may cause some performance degradation.
@yukkysaito @tkimura4 Is that okay for you?
FYI, from Humble, I believe we can use ComposableNode
in launch.xml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal pose visualizer node only subscribes to the route topic. This topic is only published once per setting route, so I don't think it will affect performance much.
@@ -12,7 +12,7 @@ | |||
// See the License for the specific language governing permissions and | |||
// limitations under the License. | |||
|
|||
#include "mission_planner/goal_pose_visualizer.hpp" | |||
#include "goal_pose_visualizer.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you move the header file? Please let me know if there are any references.
Do you want to create a guideline that says "all node header files should be under src/"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you are building a library, put all headers which should be usable by clients and therefore must be installed into a subdirectory of the include folder named like the package, while all other files (.c/.cpp and header files which should not be exported) are inside the src folder.
This is from guide of ament_cmake. This is not library header, so it should be placed under src
. Now, many packages in Autoware put their private headers in include
, so I think that should be a guideline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thanks!
I'll write some documents based on ament_cmake
's guidelines.
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
* refactor(mission_planner): prepare to support ad api Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> * fix node name Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
* refactor(mission_planner): prepare to support ad api Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> * fix node name Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
* refactor(mission_planner): prepare to support ad api Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> * fix node name Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
…ion#1561) * refactor(mission_planner): prepare to support ad api Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> * fix node name Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
* refactor(mission_planner): prepare to support ad api Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp> * fix node name Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Signed-off-by: Takagi, Isamu isamu.takagi@tier4.jp
Description
Move the file for #1495
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.