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(agnocastlib): implement Agnocast-ROS2 functionality #326

Merged
merged 8 commits into from
Jan 20, 2025
Merged

Conversation

veqcc
Copy link
Contributor

@veqcc veqcc commented Jan 20, 2025

Description

Implemented Agnocast -> ROS 2 bridge functionality.

Major changes are:

Related links

Design Document (TIER IV internal)

How was this PR tested?

  • sample application (required)
    Confirmed that transient_local are subscribed by both Agnocast and ROS 2, and transient_local_with_flag are subscribed only by Agnocast.
[agnocast_component_container-1] [INFO] [1737351272.964023617] [listener_node]: I heard transient_local message through Agnocast
[agnocast_component_container-1] [INFO] [1737351272.964755617] [listener_node]: I heard transient_local_with_flag message through Agnocast
[agnocast_component_container-1] [INFO] [1737351273.074912651] [listener_node]: I heard transient_local message through ROS

Notes for reviewers

Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Co-authored-by: atsushi yano <55824710+atsushi421@users.noreply.github.com>
@atsushi421
Copy link
Contributor

Is it better to unify the publisher/subscription interface from node_interfaces to node even in subscriptions?

I think this is better.

Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@atsushi421
Copy link
Contributor

@veqcc
pre-commitがfailしているみたいです
https://github.com/tier4/agnocast/actions/runs/12861802701/job/35855726630?pr=326

Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc
Copy link
Contributor Author

veqcc commented Jan 20, 2025

Is it better to unify the publisher/subscription interface from node_interfaces to node even in subscriptions?

こちらの件、subscription もインターフェースを統一しました。

atsushi421
atsushi421 previously approved these changes Jan 20, 2025
Copy link
Contributor

@atsushi421 atsushi421 left a comment

Choose a reason for hiding this comment

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

LGTM!

@veqcc veqcc added the run-build-test Run build-test in CI label Jan 20, 2025
Copy link

------------------------------------------------------------------------------
                           GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
File                                       Lines    Exec  Cover   Missing
------------------------------------------------------------------------------
src/agnocastlib/include/agnocast.hpp           2       2   100%   
src/agnocastlib/include/agnocast_executor.hpp
                                               1       0     0%   38
src/agnocastlib/include/agnocast_publisher.hpp
                                              33      25    75%   59,81-82,91-93,97-98
src/agnocastlib/include/agnocast_smart_pointer.hpp
                                              53      50    94%   77,80-81
src/agnocastlib/include/agnocast_topic_info.hpp
                                              13      13   100%   
src/agnocastlib/src/agnocast.cpp              82      15    18%   22,24,27,30,32-34,36,39,42,44-50,54-55,58-62,66-67,71-74,77,80,82-85,88,91,93-94,96-97,102,104-106,109-112,115,117-123,125,138-139,150-151,154-156,161
src/agnocastlib/src/agnocast_component_container.cpp
                                              12       0     0%   5,7-8,11-13,16,18-20,22-23
src/agnocastlib/src/agnocast_component_container_mt.cpp
                                              28       0     0%   7,11-12,14-16,18-20,22-24,26-28,30,32-33,35-37,40,42,44-46,48-49
src/agnocastlib/src/agnocast_executor.cpp
                                              86       0     0%   10,12,14-16,18-20,24,26,29,31-32,35-39,42-45,47-48,51-53,55-58,61-62,67,70,73,75-79,82,86-87,90-91,94,96-100,103,106,110-115,118,121-124,126-129,132,135-137,140,142,145,149-150,153-156,159,162,164-165
src/agnocastlib/src/agnocast_multi_threaded_executor.cpp
                                              51       0     0%   9,13,15-17,19-20,25-26,29-33,36,38-41,44,48,51,53-55,58-60,64,66-67,71,74,77,79-80,82-83,87-88,91,96,100,103-104,107,114-117,120
src/agnocastlib/src/agnocast_publisher.cpp
                                              73       0     0%   5,7,9-15,19-20,27,32,34-39,42-49,54,58-65,68-69,71-74,76-79,81,84-85,89-90,96,100-109,112-113,116,119,121-126,129
src/agnocastlib/src/agnocast_single_threaded_executor.cpp
                                              19       0     0%   10-12,16,18-21,24,28-30,33,35-37,40-42
src/agnocastlib/src/agnocast_smart_pointer.cpp
                                              20       0     0%   5,7-15,19,21-29
src/agnocastlib/src/agnocast_subscription.cpp
                                              90       9    10%   10-12,14,17,21,24-26,28,30,32,34-39,41-46,48,51-58,61-64,66,68,71,74,78-81,83-89,92-93,100,105,107-110,113,116,120-125,127-132,134,136,139,143-144,146-147
src/agnocastlib/src/agnocast_topic_info.cpp
                                              16       0     0%   12,16-17,20-24,28-31,34-37
src/agnocastlib/src/agnocast_utils.cpp        25      23    92%   48,50
------------------------------------------------------------------------------
TOTAL                                        604     137    22%
------------------------------------------------------------------------------

Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc veqcc added run-build-test Run build-test in CI and removed run-build-test Run build-test in CI labels Jan 20, 2025
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc veqcc added run-build-test Run build-test in CI and removed run-build-test Run build-test in CI labels Jan 20, 2025
Copy link

------------------------------------------------------------------------------
                           GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
File                                       Lines    Exec  Cover   Missing
------------------------------------------------------------------------------
src/agnocastlib/include/agnocast.hpp           2       2   100%   
src/agnocastlib/include/agnocast_executor.hpp
                                               1       0     0%   38
src/agnocastlib/include/agnocast_publisher.hpp
                                              33      25    75%   59,81-82,91-93,97-98
src/agnocastlib/include/agnocast_smart_pointer.hpp
                                              53      50    94%   77,80-81
src/agnocastlib/include/agnocast_topic_info.hpp
                                              13      13   100%   
src/agnocastlib/src/agnocast.cpp              82      15    18%   22,24,27,30,32-34,36,39,42,44-50,54-55,58-62,66-67,71-74,77,80,82-85,88,91,93-94,96-97,102,104-106,109-112,115,117-123,125,138-139,150-151,154-156,161
src/agnocastlib/src/agnocast_component_container.cpp
                                              12       0     0%   5,7-8,11-13,16,18-20,22-23
src/agnocastlib/src/agnocast_component_container_mt.cpp
                                              28       0     0%   7,11-12,14-16,18-20,22-24,26-28,30,32-33,35-37,40,42,44-46,48-49
src/agnocastlib/src/agnocast_executor.cpp
                                              86       0     0%   10,12,14-16,18-20,24,26,29,31-32,35-39,42-45,47-48,51-53,55-58,61-62,67,70,73,75-79,82,86-87,90-91,94,96-100,103,106,110-115,118,121-124,126-129,132,135-137,140,142,145,149-150,153-156,159,162,164-165
src/agnocastlib/src/agnocast_multi_threaded_executor.cpp
                                              51       0     0%   9,13,15-17,19-20,25-26,29-33,36,38-41,44,48,51,53-55,58-60,64,66-67,71,74,77,79-80,82-83,87-88,91,96,100,103-104,107,114-117,120
src/agnocastlib/src/agnocast_publisher.cpp
                                              73       0     0%   5,7,9-15,19-20,27,32,34-39,42-49,54,58-65,68-69,71-74,76-79,81,84-85,89-90,96,100-109,112-113,116,119,121-126,129
src/agnocastlib/src/agnocast_single_threaded_executor.cpp
                                              19       0     0%   10-12,16,18-21,24,28-30,33,35-37,40-42
src/agnocastlib/src/agnocast_smart_pointer.cpp
                                              20       0     0%   5,7-15,19,21-29
src/agnocastlib/src/agnocast_subscription.cpp
                                              92       9     9%   10,12-15,17,20,24,27-29,31,33,35,37-42,44-49,51,54-61,64-67,69,71,74,77,81-84,86-92,95-96,103,108,110-113,116,119,123-128,130-135,137,139,142,146-147,149-150
src/agnocastlib/src/agnocast_topic_info.cpp
                                              16       0     0%   12,16-17,20-24,28-31,34-37
src/agnocastlib/src/agnocast_utils.cpp        25      23    92%   48,50
------------------------------------------------------------------------------
TOTAL                                        606     137    22%
------------------------------------------------------------------------------

@veqcc
Copy link
Contributor Author

veqcc commented Jan 20, 2025

@atsushi421 @Koichi98

  • トピック名の resolve なしでの publisher/subscription 作成を可能に(これに付随してテストも多少改変した)
  • universe での動作確認

まで完了したので、再度 approve お願いします!

universe の方も同時マージできると助かります
tier4/autoware.universe_tmp-agnocast#15

Copy link
Contributor

@Koichi98 Koichi98 left a comment

Choose a reason for hiding this comment

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

LGTM!

@veqcc veqcc merged commit 76722b3 into main Jan 20, 2025
4 checks passed
@veqcc veqcc deleted the feat/bridge branch January 20, 2025 09:20
@bdm-k
Copy link
Contributor

bdm-k commented Feb 7, 2025

@veqcc

The parameter do_always_ros2_publish will be unified to Agnocast::PublisherOptions later

struct PublisherOptions : rclcpp::PublisherOptionsBase
{
  bool do_always_ros2_publish;
};

このような構造体を想定していますか?

@veqcc
Copy link
Contributor Author

veqcc commented Feb 7, 2025

struct PublisherOptions : rclcpp::PublisherOptionsBase
{
  bool do_always_ros2_publish;
};

このような構造体を想定していますか?

@bdm-k
ですね!何を継承するか( rclcpp::PublisherOptionsBase なのか rclcpp::PublisherOptions なのかなど )は未検討ですが、イメージはそんな感じです 👍

@Koichi98
Copy link
Contributor

Koichi98 commented Feb 7, 2025

@bdm-k @veqcc
一旦rclcpp::PublisherOptionsBaseなどrclcpp側の継承をせずに、

struct PublisherOptions
{
  bool do_always_ros2_publish;
};

だけで作ってもらって機能性が保持されることを確認した上でPRをmergeしましょう。
そこからissue #306に見られるようなqos_overriding_optionsをはじめAutoware適用に必要なものを適宜追加していく形でどうでしょうか。
継承するのはPublisherOptionsBaseやPublisherOptionsなどで定義されているメンバ変数に関して、対応するAgnocastのPublisherでの実装を作ってからという方針でいくのがよさそうかなと!

@bdm-k bdm-k mentioned this pull request Feb 10, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-build-test Run build-test in CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_subscription_count が Agnocast の subscription しかカウントできない
4 participants