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

feat(tier4_autoware_api_launch): add rosbridge #779

Merged
merged 5 commits into from
Jun 6, 2022

Conversation

shmpwk
Copy link
Contributor

@shmpwk shmpwk commented Apr 24, 2022

Signed-off-by: Shumpei Wakabayashi shumpei.wakabayashi@tier4.jp

Description

web_controller needs rosbridge server.
Previously, autoware universe does not launch rosbridge because of #658 (comment).
So I add launching rosbridge following original tier4/autoware_launch.

Related discussion: autowarefoundation/autoware#208

Note that #318 removes rosbridge for fear of conflicting rosbridge in #175 as they mentioned.
However #175 is under tier4 repos, but not under autowarefoundation.
Under tier4 repos, rosbridge is launched in autoware_api_launch, which cause conflict if other code lanches rosbridge.
On the other hand, under autoware foundation repos, neither tier4_autoware_api_launch (same as autoware_api_launch) nor web_controller does not launch rosbridge for fear of duplication.
See #658 (comment) and #318.

Thus we can add launching rosbridge in autoware_api or web_controller under autoware foundation repos.

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.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@Sharrrrk
Copy link
Contributor

@shmpwk, in this case, I would propose to launch rosbridge in web_controller.launch.xml as the rosbridge is used by web_controller, launch it in autoware_api makes it confusing.

@Sharrrrk
Copy link
Contributor

What's more, as mentioned in ros2/launch#568 ,respawn fix has already been released into the launch packages in version 0.22.0.

@kosuke55
Copy link
Contributor

FYI: ros2/launch#569 was released in 0.22.0 but not in galactic.

@shmpwk
Copy link
Contributor Author

shmpwk commented Apr 25, 2022

@kosuke55 @Sharrrrk
Thank you for the infomation.
Then as for rospawn, leave <arg name="respawn" value="$(var respawn)"/>.

As for in which pkg should we launch rosbridge, I asked here

@shmpwk
Copy link
Contributor Author

shmpwk commented Apr 25, 2022

@Sharrrrk
Throughout discussion here,
we decided to launch rosbridge in tier4_autoware_api_launch.
Also I added docs in web_controller about rosbridge.

@yabuta
Could you review this PR?

h-ohta pushed a commit to h-ohta/autoware.universe that referenced this pull request Apr 26, 2022
* add drivable area visualizer

* add license

* modify pointed out in pre-commit

* modify pointed out in pre-commit

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>

Co-authored-by: Yukihiro Saito <yukky.saito@gmail.com>
@Sharrrrk
Copy link
Contributor

Sharrrrk commented Apr 26, 2022

@Sharrrrk Throughout discussion here, we decided to launch rosbridge in tier4_autoware_api_launch. Also I added docs in web_controller about rosbridge.

@shmpwk Thank you for the detailed explanation, may be we should consider add it directly in autoware.launch, as it is required by multiple nodes. So implement it in either module may not a proper way.
The other reason is that web_controller is now launched by autoware.launch in core and it's somehow a compulsory part to use autoware; while for API_launch, it's still in universe and under discussion. I think in this way, it's easier for current users to understand and also doesn't block the integration of API.

@shmpwk
Copy link
Contributor Author

shmpwk commented Apr 26, 2022

@Sharrrrk
Yes, I agree with you in that it is more straightforward to launch rosbridge in autoware.launch.xml.
My concern is rosbridge is just for backend support and if it is written in autoware.launch (also in planning_simulator,launch, logging_simulator.launch), it reduces readability. A new user don't have to know what is rosbridge.
If rosbridge is written in autoware.launch, do you think rosbridge should be launched independently not in the module?

@Sharrrrk
Copy link
Contributor

If rosbridge is written in autoware.launch, do you think rosbridge should be launched independently not in the module?

Yes, as far as rosbridge is a common requried component, I think it's better to be added to autoware.launch, in the tools section. Maybe with some comment like this to explain why:

<!-- This component allows webpages to talk with ROS, which is required by Web Controller and External API component-->

@shmpwk
Copy link
Contributor Author

shmpwk commented Apr 27, 2022

@Sharrrrk That seems to be a good way if rosbridge is launched with autoware_launch.
Still, I think it would be easier to use api_launch to launch rosbridge.
How about the concept that api_launch is an interface including rosbridge and web_controller is its use case?

@shmpwk
Copy link
Contributor Author

shmpwk commented May 2, 2022

@Sharrrrk What do you think? (It is a trivial matter though.)

@shmpwk shmpwk requested a review from Sharrrrk May 10, 2022 00:59
@shmpwk shmpwk mentioned this pull request May 27, 2022
4 tasks
@shmpwk
Copy link
Contributor Author

shmpwk commented May 27, 2022

@Sharrrrk
Looking at tier4 repos (not autowarefoundation repos), currently we put rosbridge in autoware_api.launch.
https://github.com/tier4/autoware_launch/blob/awf-latest/autoware_api_launch/launch/autoware_api.launch.xml

So my suggestion is

  1. following it as the current PR is
  2. if this PR have problem after this PR merged, make another PR.

@codecov
Copy link

codecov bot commented Jun 1, 2022

Codecov Report

Merging #779 (ce8d2f5) into main (4e18fe0) will increase coverage by 0.40%.
The diff coverage is n/a.

❗ Current head ce8d2f5 differs from pull request most recent head 963f608. Consider uploading reports for the commit 963f608 to get more accurate results

@@           Coverage Diff            @@
##            main    #779      +/-   ##
========================================
+ Coverage   9.23%   9.63%   +0.40%     
========================================
  Files        981     907      -74     
  Lines      61221   56544    -4677     
  Branches   11193    6694    -4499     
========================================
- Hits        5655    5450     -205     
+ Misses     50646   46599    -4047     
+ Partials    4920    4495     -425     
Flag Coverage Δ *Carryforward flag
differential ∅ <ø> (?)
total 9.63% <ø> (+0.40%) ⬆️ Carriedforward from 5e9ab8c

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
...lision_checker_node/obstacle_collision_checker.cpp 0.00% <0.00%> (-8.14%) ⬇️
...anning_evaluator/src/metrics/stability_metrics.cpp 78.57% <0.00%> (-3.58%) ⬇️
...nning/behavior_velocity_planner/test/src/utils.hpp 85.71% <0.00%> (-2.10%) ⬇️
control/trajectory_follower/src/pid.cpp 91.48% <0.00%> (ø)
perception/shape_estimation/src/node.cpp 0.00% <0.00%> (ø)
perception/tensorrt_yolo/src/nodelet.cpp 0.00% <0.00%> (ø)
perception/lidar_centerpoint/src/node.cpp 0.00% <0.00%> (ø)
planning/obstacle_stop_planner/src/node.cpp 0.00% <0.00%> (ø)
map/lanelet2_extension/lib/visualization.cpp 0.00% <0.00%> (ø)
planning/route_handler/src/route_handler.cpp 0.00% <0.00%> (ø)
... and 281 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 4e18fe0...963f608. Read the comment docs.

Copy link
Contributor

@yukkysaito yukkysaito left a comment

Choose a reason for hiding this comment

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

LGTM
pls get @kenji-miyake 's aprrove as well

wep21 and others added 4 commits June 1, 2022 18:44
…towarefoundation#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>
…toware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>
Copy link
Contributor

@kenji-miyake kenji-miyake left a comment

Choose a reason for hiding this comment

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

@shmpwk shmpwk merged commit 7ec4534 into autowarefoundation:main Jun 6, 2022
@shmpwk shmpwk deleted the feat/add-rosbridge branch June 6, 2022 05:35
ktro2828 pushed a commit to ktro2828/autoware.universe that referenced this pull request Jun 7, 2022
* fix(image_projection_based_fusion): modify build error in rolling (autowarefoundation#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* feat(tier4_autoware_api_launch): add rosbridge

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* Update launch/tier4_autoware_api_launch/launch/autoware_api.launch.xml

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
SoohyeokPark-MORAI pushed a commit to SoohyeokPark-MORAI/autoware.universe that referenced this pull request Jun 15, 2022
* fix(image_projection_based_fusion): modify build error in rolling (autowarefoundation#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* feat(tier4_autoware_api_launch): add rosbridge

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* Update launch/tier4_autoware_api_launch/launch/autoware_api.launch.xml

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: SoohyeokPark-MORAI <shpark.morai@gmail.com>
SoohyeokPark-MORAI pushed a commit to SoohyeokPark-MORAI/autoware.universe that referenced this pull request Jun 15, 2022
* fix(image_projection_based_fusion): modify build error in rolling (autowarefoundation#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* feat(tier4_autoware_api_launch): add rosbridge

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* Update launch/tier4_autoware_api_launch/launch/autoware_api.launch.xml

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Jul 1, 2022
* fix(image_projection_based_fusion): modify build error in rolling (tier4#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* feat(tier4_autoware_api_launch): add rosbridge

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* Update launch/tier4_autoware_api_launch/launch/autoware_api.launch.xml

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Sep 28, 2022
* fix(image_projection_based_fusion): modify build error in rolling (tier4#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* feat(tier4_autoware_api_launch): add rosbridge

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* Update launch/tier4_autoware_api_launch/launch/autoware_api.launch.xml

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Oct 3, 2022
* fix(image_projection_based_fusion): modify build error in rolling (tier4#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* feat(tier4_autoware_api_launch): add rosbridge

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* Update launch/tier4_autoware_api_launch/launch/autoware_api.launch.xml

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Oct 3, 2022
* fix(image_projection_based_fusion): modify build error in rolling (tier4#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* feat(tier4_autoware_api_launch): add rosbridge

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* Update launch/tier4_autoware_api_launch/launch/autoware_api.launch.xml

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Oct 19, 2022
* fix(image_projection_based_fusion): modify build error in rolling (tier4#775)

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* feat(tier4_autoware_api_launch): add rosbridge

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* docs(web_controller): rosbridge is automatically launched in tier4_autoware_api_launch

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* Update launch/tier4_autoware_api_launch/launch/autoware_api.launch.xml

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

Co-authored-by: Daisuke Nishimatsu <42202095+wep21@users.noreply.github.com>
Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
kyoichi-sugahara pushed a commit to kyoichi-sugahara/autoware.universe that referenced this pull request Sep 3, 2023
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.

6 participants