Skip to content
This repository has been archived by the owner on Mar 27, 2023. It is now read-only.

[ROS2 Porting] perception_launch #6

Merged
merged 5 commits into from
Nov 30, 2020

Conversation

esteve
Copy link
Contributor

@esteve esteve commented Nov 6, 2020

No description provided.

@esteve esteve marked this pull request as ready for review November 11, 2020 21:20
Copy link
Collaborator

@mitsudome-r mitsudome-r left a comment

Choose a reason for hiding this comment

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

I noticed quite a few number of changes required to run the launch file.
I have created PR to your branch with the fixes I had to make. esteve#1.

Please consider merging it to your branch.

@@ -31,7 +31,7 @@ jobs:
- name: Build
run: |
. /opt/ros/foxy/setup.sh
colcon build --event-handlers console_cohesion+
colcon build --event-handlers console_cohesion+ --packages-up-to control_launch integration_launch localization_launch map_launch perception_launch planning_launch sensing_launch system_launch vehicle_launch
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we don't need this until all packages are ported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that if we don't filter the packages, the whole workspace will be built, including all the dependencies.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But that would mean CI will always fail because it fails to find packages that are not ported.
I would suggest either update CI as we do the port, or always build whole workspace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that once a package is ported, move it from --packages-ignore to --packages-up-to. Building the entire workspace takes a lot of time now that Pilot.Auto has been added to the build_depends.repos file in this repository.

@mitsudome-r mitsudome-r changed the title Initial port to ROS 2 [ROS2 Porting] perception_launch Nov 19, 2020
@esteve
Copy link
Contributor Author

esteve commented Nov 19, 2020

I've rebased this on top of #16 to pick up the new dependencies.

@esteve esteve force-pushed the port-perception-launch branch 2 times, most recently from bcc2dbb to a26ba2f Compare November 24, 2020 11:14
<arg name="image_raw7" default="$(arg image_raw7)"/>
<arg name="image_number" default="$(arg image_number)"/>
<!-- <include file="$(find-pkg-share tensorrt_yolo3)/launch/yolo3.launch.xml">
<let var="image_raw0" value="$(var image_raw0)"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I think I was the one to change arg to let, but I later found out that setting variables using let wouldn't pass them to included file.
Could you switch it back to arg?
You have to use arg with value as shown in the suggestion. (This sounds weird since the migration guide states that value for arg is not supported anymore, but that was the only way to pass arguments to the included launch file)

Suggested change
<let var="image_raw0" value="$(var image_raw0)"/>
<arg var="image_raw0" value="$(var image_raw0)"/>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 7ae696e

Copy link
Collaborator

@mitsudome-r mitsudome-r left a comment

Choose a reason for hiding this comment

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

LGTM

@mitsudome-r mitsudome-r merged commit bc0aab8 into tier4:ros2 Nov 30, 2020
isamu-takagi pushed a commit that referenced this pull request Feb 7, 2022
* autoware_iv_auto_msgs_converter -> tier4_auto_msgs_converter

* autoware_external_api_msgs -> tier4_external_api_msgs

* autoware_api_utils -> tier4_api_utils

* autoware_vehicle_msgs -> tier4_vehicle_msgs

* fix format
tier4-autoware-public-bot bot pushed a commit that referenced this pull request Apr 12, 2022
Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>
kazuki527 pushed a commit to kazuki527/autoware_launch that referenced this pull request Apr 25, 2022
takayuki5168 pushed a commit that referenced this pull request Mar 13, 2023
* fix(planning_launch): enable blind spot launch and change its params

* fix(planning_launch): change stop_margin param

* fix(planning_launch): revert stop_margin param

* fix(planning_launch): change ego_pass_first_margin param
takayuki5168 pushed a commit that referenced this pull request Mar 13, 2023
takayuki5168 pushed a commit that referenced this pull request Mar 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants