Skip to content

feat(nebula_core_ros): make SyncToolingWorker compatible with agnocast_wrapper::Node - #469

Merged
mojomex merged 17 commits into
tier4:mainfrom
Koichi98:feat/agnocast-core-node-compat
Jul 29, 2026
Merged

mojomex merged 17 commits into
tier4:mainfrom
Koichi98:feat/agnocast-core-node-compat

Conversation

@Koichi98

@Koichi98 Koichi98 commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

PR Type

  • Improvement

Related Links

Description

Make the shared SyncToolingWorker compatible with agnocast_wrapper::Node.

Why hard-code agnocast_wrapper::Node * instead of a NodeT template: SyncToolingWorker holds a publisher member whose type depends on the backend. Unlike RateBoundStatus / LivenessMonitor, which take the node only in a templated constructor and keep no node-typed member, a templated constructor can't type that member — the only template option would be making the whole class a template with a decltype(node->create_publisher<...>(...))-deduced member. That would only add plain rclcpp::Node support, which no caller needs: SyncToolingWorker is used only by HesaiRosWrapper and ContinentalARS548RosWrapper, and both migrate to agnocast_wrapper::Node. Since the wrapper Node already abstracts the ENABLE_AGNOCAST=0/1 backend, agnocast_wrapper::Node * + NEBULA_PUBLISHER_PTR is simpler and sufficient.

⚠️ Must merge atomically with the driver migrations: without the templated constructor, SyncToolingWorker no longer accepts a plain rclcpp::Node *. On main the drivers are still rclcpp::Node and pass one, so merging this branch to main alone breaks the build; the migrations pass an agnocast_wrapper::Node * that the old constructor rejects. The hesai and continental migrations therefore merge into this branch first, and this branch goes to main only once both are integrated.

Review Procedure

Build with ENABLE_AGNOCAST=0 and =1 after the hesai / continental migrations are merged into this branch.

Remarks

None.

Pre-Review Checklist for the PR Author

  • Assign PR to reviewer

CI Checks

  • Build and test for PR: Required to pass before the merge.

…wrapper::Node-compatible

Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
…pper to agnocast_wrapper::Node (Method 2)

Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
Koichi98 and others added 6 commits July 27, 2026 13:29
…a publish(const &)

The ALLOCATE_OUTPUT + *out = std::move(*msg) pattern stole msg's buffer, which
was allocated on the normal heap (outside the borrow window), so the payload
never entered shared memory and agnocast zero-copy delivered an invalid pointer
to cross-process subscribers. publish(const &) borrows inside the window and
copies into shared memory, which is correct on both the rclcpp and agnocast
backends.

Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
…sync_tooling_worker

Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
… (Method 2)

Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
…, restore cie thread factory

- include the post-tier4#472 nebula_agnocast_wrapper.hpp (old autoware_agnocast_wrapper.hpp
  no longer exists)
- publish raw packets via publish(*msg) so the payload is materialized into shared memory
  (a plain std::move would leave it on the normal heap, invalid for cross-process subscribers)
- restore make_cie_thread_factory for the packets publish thread; the migration had dropped it,
  which would leave that non-ROS thread unmanaged by the Agnocast CIE thread configurator and
  lose its configured priority/affinity

Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
@Koichi98
Koichi98 requested a review from mojomex July 27, 2026 06:46
@Koichi98
Koichi98 marked this pull request as ready for review July 27, 2026 06:46
mojomex and others added 7 commits July 28, 2026 12:02
…gain

The migration replaced `packets_sub_->get_topic_name()` with the relative
topic literal, which hides the actual (namespaced/remapped) topic and makes
troubleshooting harder. The wrapper's Subscription has no get_topic_name(),
so resolve the name through the node's topics interface instead; this yields
the same fully-qualified name as before on both backends.

Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
The migration replaced `packets_sub_->get_topic_name()` with the relative
topic literal, which hides the actual (namespaced/remapped) topic and makes
troubleshooting harder. The wrapper's Subscription has no get_topic_name(),
so resolve the name through the node's topics interface instead; this yields
the same fully-qualified name as before on both backends.

Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
feat(nebula_hesai): migrate HesaiRosWrapper to agnocast_wrapper::Node
feat(continental_ars548_ros_wrapper): migrate ContinentalARS548RosWrapper to agnocast_wrapper::Node
@codecov

codecov Bot commented Jul 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 43.47826% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.06%. Comparing base (d28c7e8) to head (014790e).
⚠️ Report is 1 commits behind head on main.

❌ Your patch check has failed because the patch coverage (30.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #469      +/-   ##
==========================================
+ Coverage   40.00%   40.06%   +0.06%     
==========================================
  Files         132      135       +3     
  Lines       10284    10328      +44     
  Branches     5399     5397       -2     
==========================================
+ Hits         4114     4138      +24     
- Misses       3777     3806      +29     
+ Partials     2393     2384       -9     
Flag Coverage Δ
nebula_continental 29.77% <43.47%> (?)
nebula_core_ros 29.77% <43.47%> (?)
nebula_hesai 29.77% <43.47%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mojomex mojomex left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Confirmed Nebula compiles&runs with and without Agnocast enabled.

Tested with rosbags for both OT128 and ARS548.

@mojomex
mojomex merged commit 06e04fb into tier4:main Jul 29, 2026
91 of 95 checks passed
mojomex pushed a commit that referenced this pull request Oct 15, 2026
…ad configurator again (#498)

#460 registered two Hesai threads with the Agnocast CIE thread
configurator, the packet publisher and the UDP receiver. #469 migrated
HesaiRosWrapper to agnocast_wrapper::Node and dropped both thread
factories, but it restored only the factory of the packet publisher.
The UDP receiver has been a plain std::thread since then, so the
configurator cannot set its scheduling policy, priority, or affinity.

This change passes make_cie_thread_factory("nebula_hesai_udp_receiver@"
+ frame_id) to HesaiHwInterface again.

With launch_hw:=true and udp_only:=true, the prerun template now lists
nebula_hesai_udp_receiver@<frame_id> next to
nebula_hesai_packets_pub@<frame_id>. Before this change, it listed only
the packet publisher.

Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
@mojomex mojomex mentioned this pull request Oct 15, 2026
1 of 5 tasks
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.

2 participants