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

fix(agnocastlib): use worker thread for ros2 publisher #356

Merged
merged 21 commits into from
Feb 14, 2025

Conversation

veqcc
Copy link
Contributor

@veqcc veqcc commented Feb 4, 2025

Description

This PR changes to use worker thread for publishing ROS 2 messages, in order to reduce the latency of publish.

Major features are:

  • create a std::thread which executes ROS 2 publish
  • use mq to notify that ROS 2 publish should be done

Related links

TIER IV INTERNAL

How was this PR tested?

  • sample application (required)
  • Autoware (required)
  • bash scripts/e2e_test_1to1_with_ros2sub
  • bash scripts/e2e_test_2to2

Notes for reviewers

There is a problem around termination when std::thread are used.
It will be solved later by @Koichi98

Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc veqcc added the run-build-test Run build-test in CI label Feb 4, 2025
@veqcc veqcc requested a review from sykwer February 4, 2025 04:52
@veqcc veqcc marked this pull request as draft February 4, 2025 05:00
@veqcc veqcc removed the run-build-test Run build-test in CI label Feb 4, 2025
Copy link

github-actions bot commented Feb 4, 2025

------------------------------------------------------------------------------
                           GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
File                                       Lines    Exec  Cover   Missing
------------------------------------------------------------------------------
src/agnocastlib/include/agnocast.hpp           2       2   100%   
src/agnocastlib/include/agnocast_callback_info.hpp
                                              13      13   100%   
src/agnocastlib/include/agnocast_executor.hpp
                                               1       0     0%   38
src/agnocastlib/include/agnocast_publisher.hpp
                                              35      27    77%   71,113-114,118-122
src/agnocastlib/include/agnocast_smart_pointer.hpp
                                              56      56   100%   
src/agnocastlib/src/agnocast.cpp              82      15    18%   24,26,29,32,34-36,38,41,44,46-52,56-57,60-64,68-69,73-76,79,82,84-87,90,93,95-96,98-99,104,106-108,111-114,117,119-125,127,140-141,152-153,156-158,163
src/agnocastlib/src/agnocast_callback_info.cpp
                                              16       0     0%   12,16-17,20-24,28-31,34-37
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%   11,13,15-17,19-21,25,27,30,32-33,36-40,43-46,48-49,52-54,56-59,62-63,68,71,74,76-80,83,87-88,91-92,95,97-101,104,107,111-116,119,122-125,127-130,133,136-137,147,150,152,155,159-160,163-165,169,175,178,180-181
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
                                              75       7     9%   10,12,23,26,31,33,35-41,45-47,50-55,58-65,69,72,77-86,89-90,92-95,97-100,102,105-106,110-111,116-117,120,123,125-130,133
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
                                              18       0     0%   6,9-16,20,23-30
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,73,76,80-83,85-91,94-96,99-102,105,108,112-117,119-124,126,128,131,135-136,138-139
src/agnocastlib/src/agnocast_utils.cpp        25      23    92%   48,50
------------------------------------------------------------------------------
TOTAL                                        609     152    25%
------------------------------------------------------------------------------

@veqcc veqcc marked this pull request as ready for review February 4, 2025 05:31
@veqcc veqcc added the run-build-test Run build-test in CI label Feb 4, 2025
Copy link

github-actions bot commented Feb 4, 2025

------------------------------------------------------------------------------
                           GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
File                                       Lines    Exec  Cover   Missing
------------------------------------------------------------------------------
src/agnocastlib/include/agnocast.hpp           2       2   100%   
src/agnocastlib/include/agnocast_callback_info.hpp
                                              13      13   100%   
src/agnocastlib/include/agnocast_executor.hpp
                                               1       0     0%   38
src/agnocastlib/include/agnocast_publisher.hpp
                                              35      27    77%   71,113-114,118-122
src/agnocastlib/include/agnocast_smart_pointer.hpp
                                              56      56   100%   
src/agnocastlib/src/agnocast.cpp              82      15    18%   24,26,29,32,34-36,38,41,44,46-52,56-57,60-64,68-69,73-76,79,82,84-87,90,93,95-96,98-99,104,106-108,111-114,117,119-125,127,140-141,152-153,156-158,163
src/agnocastlib/src/agnocast_callback_info.cpp
                                              16       0     0%   12,16-17,20-24,28-31,34-37
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%   11,13,15-17,19-21,25,27,30,32-33,36-40,43-46,48-49,52-54,56-59,62-63,68,71,74,76-80,83,87-88,91-92,95,97-101,104,107,111-116,119,122-125,127-130,133,136-137,147,150,152,155,159-160,163-165,169,175,178,180-181
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
                                              75       7     9%   10,12,23,26,31,33,35-41,45-47,50-55,58-65,69,72,77-86,89-90,92-95,97-100,102,105-106,110-111,116-117,120,123,125-130,133
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
                                              18       0     0%   6,9-16,20,23-30
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,73,76,80-83,85-91,94-96,99-102,105,108,112-117,119-124,126,128,131,135-136,138-139
src/agnocastlib/src/agnocast_utils.cpp        25      23    92%   48,50
------------------------------------------------------------------------------
TOTAL                                        609     152    25%
------------------------------------------------------------------------------

@sykwer
Copy link
Member

sykwer commented Feb 4, 2025

recommended: worker thread + conditional variable

@veqcc veqcc marked this pull request as draft February 5, 2025 01:39
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc veqcc changed the title fix(agnocastlib): use detached thread for ros2 publisher fix(agnocastlib): use worker thread for ros2 publisher Feb 5, 2025
@veqcc veqcc marked this pull request as ready for review February 5, 2025 04:05
@veqcc veqcc requested a review from Koichi98 February 5, 2025 04:05
@veqcc veqcc removed the run-build-test Run build-test in CI label Feb 5, 2025
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
veqcc added 5 commits February 5, 2025 20:09
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc veqcc requested review from atsushi421 and removed request for sykwer February 5, 2025 12:59
sykwer
sykwer previously approved these changes Feb 5, 2025
Co-authored-by: atsushi yano <55824710+atsushi421@users.noreply.github.com>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc veqcc marked this pull request as draft February 6, 2025 01:48
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc veqcc marked this pull request as ready for review February 13, 2025 08:44
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@Koichi98
Copy link
Contributor

Koichi98 commented Feb 13, 2025

There is a problem around termination when std::thread are used.
It will be solved later by @Koichi98

The problem was already fixed with your implementation on Publisher destructor!
I confirmed that ros2_publish thread go pass the if condition below, and there was nothing left in /dev/shm and /dev/mqueue.
Thanks!

    if (mq_msg.should_terminate) {

@atsushi421 atsushi421 self-requested a review February 14, 2025 03:04
@atsushi421
Copy link
Contributor

コンフリクト解決お願いします 🙇

@atsushi421 atsushi421 self-requested a review February 14, 2025 04:37
atsushi421
atsushi421 previously approved these changes Feb 14, 2025
Signed-off-by: veqcc <ryuta.kambe@tier4.jp>
@veqcc
Copy link
Contributor Author

veqcc commented Feb 14, 2025

@Koichi98
口頭で指摘いただいた、mq 作成の順番を修正しました 🙇

@Koichi98 Koichi98 added the run-build-test Run build-test in CI label Feb 14, 2025
Copy link

------------------------------------------------------------------------------
                           GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
File                                       Lines    Exec  Cover   Missing
------------------------------------------------------------------------------
src/agnocastlib/include/agnocast.hpp           3       3   100%   
src/agnocastlib/include/agnocast_callback_info.hpp
                                              13      10    76%   92,94-95
src/agnocastlib/include/agnocast_executor.hpp
                                               1       0     0%   40
src/agnocastlib/include/agnocast_publisher.hpp
                                              88      56    63%   88,106-108,119,125,129,137-139,142,146-148,155-156,159-161,164-165,168,173,201-202,207-208,211-213,217-218
src/agnocastlib/include/agnocast_smart_pointer.hpp
                                              34      34   100%   
src/agnocastlib/src/agnocast.cpp              59      12    20%   19,22,24-30,34-35,38-42,46-47,51-54,57,60,62,65,67-68,73,75-77,80-83,86,88-94,96,108-109
src/agnocastlib/src/agnocast_callback_info.cpp
                                              16       0     0%   12,16-17,20-24,28-31,34-37
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
                                              90       0     0%   11,13,15-17,19-21,25,27,30,32-33,36-40,43-46,48-49,52-54,56-59,62-63,68,71,74,76-80,83,87-88,91-92,95,97-101,104,111-116,119,122-125,127-130,134-138,141,144-145,155,158,160,163,167-168,171-173,177,183,186,188-189
src/agnocastlib/src/agnocast_multi_threaded_executor.cpp
                                              53       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,109-110,118-121,124
src/agnocastlib/src/agnocast_publisher.cpp
                                              54       7    13%   10,12,23,26,31,33,35-41,44,47,52-60,69-70,72-75,77-80,82,87,91-92,97,100,102-107,110
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
                                              18       0     0%   6,9-16,20,23-30
src/agnocastlib/src/agnocast_subscription.cpp
                                              51       9    17%   6-8,10,13,15,19-22,24-29,32-36,39,42,46-51,53-58,60,62,65,69-70,72-73
src/agnocastlib/src/agnocast_utils.cpp        29      26    89%   63,65-66
------------------------------------------------------------------------------
TOTAL                                        568     157    27%
------------------------------------------------------------------------------

@veqcc veqcc merged commit 71117f2 into main Feb 14, 2025
4 checks passed
@veqcc veqcc deleted the fix/use_detached_thread_for_ros2_pub branch February 14, 2025 06:14
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.

4 participants