Skip to content

feat(nebula_continental, nebula_hesai): apply agnocast_components for CIE#418

Open
atsushi421 wants to merge 7 commits intotier4:mainfrom
atsushi421:feat/nebula-ros/apply-cie
Open

feat(nebula_continental, nebula_hesai): apply agnocast_components for CIE#418
atsushi421 wants to merge 7 commits intotier4:mainfrom
atsushi421:feat/nebula-ros/apply-cie

Conversation

@atsushi421
Copy link
Contributor

@atsushi421 atsushi421 commented Mar 17, 2026

Description

Apply agnocast_components to enable CallbackIsolatedAgnocastExecutor (CIE) for nebula_continental and nebula_hesai nodes, as part of the effort to achieve middleware-transparent scheduling and optimize end-to-end response time across Autoware.

Changes

nebula_continental

  • CMakeLists.txt:
    • Added conditional agnocast_components support guarded by ENABLE_AGNOCAST environment variable
    • When ENABLE_AGNOCAST is set: uses agnocast_components_register_node with EXECUTOR CallbackIsolatedAgnocastExecutor for continental_ars548_ros_wrapper, and default executor for continental_srr520_ros_wrapper
    • When ENABLE_AGNOCAST is not set: falls back to rclcpp_components_register_node
    • Added conditional ament_export_dependencies(agnocast_components)
    • SRR520 is also registered with agnocast_components_register_node when ENABLE_AGNOCAST is set, because agnocast_components_register_node and rclcpp_components_register_node cannot coexist in the same package (both generate the same resource index file via different mechanisms)
  • package.xml:
    • Added <depend condition="$ENABLE_AGNOCAST == 1">agnocast_components</depend>

nebula_hesai

  • CMakeLists.txt:
    • Added conditional agnocast_components availability check guarded by ENABLE_AGNOCAST (alongside existing agnocastlib check)
    • When ENABLE_AGNOCAST is set: uses agnocast_components_register_node with EXECUTOR CallbackIsolatedAgnocastExecutor for hesai_ros_wrapper
    • When ENABLE_AGNOCAST is not set: falls back to rclcpp_components_register_node
    • Added conditional ament_export_dependencies(agnocast_components)
    • Existing agnocastlib dependency (for publisher/subscriber wrapping via nebula_agnocast_wrapper) is preserved
  • package.xml:
    • Added <depend condition="$ENABLE_AGNOCAST == 1">agnocast_components</depend>

Related links

How was this PR tested?

Notes for reviewers

Please review the following aspects:

  • Verify that all relevant CMakeLists.txt for nebula_continental and nebula_hesai nodes have been updated (no missed files).

    • Our current target is X2 v4.4.0, so please let us know if we have applied CIE to any nodes that do not run with that version.
  • If the node meets all of the following conditions, please check for potential race conditions (see Effects on system behavior for the reason):

    1. The node was originally running on a rclcpp::executors::SingleThreadedExecutor or rclcpp_components::component_container
    2. The node has multiple callback groups (the default is one)
    3. Shared variables are accessed across callback groups without proper mutex protection

    Note: Nodes already running on MultiThreadedExecutor (or component_container_mt) are not affected, as their callbacks are already executed concurrently.

Interface changes

None.

Effects on system behavior

Internally, CIE creates a dedicated rclcpp::executors::SingleThreadedExecutor for each callback group, so the fundamental scheduling behavior within each group remains almost identical. The key difference is that, for nodes originally using rclcpp::executors::SingleThreadedExecutor, callback groups now run in parallel across separate threads, similar to MultiThreadedExecutor.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR integrates autoware_agnocast_wrapper into nebula_continental so the ARS548 node can switch between the standard ROS 2 executor and CallbackIsolatedAgnocastExecutor (CIE) based on the ENABLE_AGNOCAST environment variable, supporting middleware-transparent scheduling efforts in Autoware.

Changes:

  • Added autoware_agnocast_wrapper as a package dependency.
  • Added find_package(autoware_agnocast_wrapper REQUIRED) in CMake.
  • Switched ARS548’s component executable registration to autoware_agnocast_wrapper_register_node with explicit ROS2/agnocast executor selections.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/nebula_continental/nebula_continental/package.xml Adds autoware_agnocast_wrapper dependency to the package manifest.
src/nebula_continental/nebula_continental/CMakeLists.txt Enables wrapper-based node registration for ARS548 and sets executor types for toggling CIE via env var.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atsushi421 atsushi421 changed the title feat(nebula_continental): apply autoware_agnocast_wrapper for CIE feat(nebula_continental): apply agnocast_components for CIE Mar 18, 2026
…nal on ENABLE_AGNOCAST

Align with the Hesai package pattern by guarding the agnocast_components
dependency behind the ENABLE_AGNOCAST environment variable, falling back
to rclcpp_components_register_node when disabled. This fixes CI failures
where rosdep cannot find ros-humble-agnocast-components.

Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
@codecov
Copy link

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.34%. Comparing base (601086a) to head (161ea6b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #418      +/-   ##
==========================================
- Coverage   48.35%   48.34%   -0.01%     
==========================================
  Files         156      156              
  Lines       12996    12997       +1     
  Branches     6901     6901              
==========================================
  Hits         6284     6284              
- Misses       5325     5326       +1     
  Partials     1387     1387              
Flag Coverage Δ
nebula_continental 30.37% <ø> (?)
nebula_hesai 30.37% <ø> (?)

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.

…n ENABLE_AGNOCAST

agnocast_components_register_node and rclcpp_components_register_node
cannot coexist in the same package because both generate the same
rclcpp_components resource index file via different mechanisms.
Wrap SRR520 registration with the same ENABLE_AGNOCAST conditional.

Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
@atsushi421 atsushi421 requested a review from mojomex March 19, 2026 00:44
@atsushi421 atsushi421 marked this pull request as ready for review March 19, 2026 00:44
Copy link
Collaborator

@mojomex mojomex left a comment

Choose a reason for hiding this comment

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

Hi, thank you for the PR! I'm really excited about our stack getting proper scheduling.

One question: Is there a reason you only targeted the Continental ARS548 here, and not the Hesai node as well? From what we saw internally, LiDARs would profit the most from proper real-time scheduling support.

@atsushi421
Copy link
Contributor Author

Hi, thank you for the PR! I'm really excited about our stack getting proper scheduling.

One question: Is there a reason you only targeted the Continental ARS548 here, and not the Hesai node as well? From what we saw internally, LiDARs would profit the most from proper real-time scheduling support.

@mojomex
Thank you for the warm and encouraging comment!
Regarding the Hesai node — the executor for hesai_ros_wrapper_node is planned to be changed in this separate PR: https://github.com/tier4/autoware_launch.x2/pull/2028
Please let me know if my understanding is off!

@mojomex
Copy link
Collaborator

mojomex commented Mar 19, 2026

@atsushi421 I see. That seems like a very Pilot Auto-centric approach to me. Since Nebula is used by other projects and products, I'd suggest we add the same changes to Hesai in Nebula directly as well. Other vendors are fine right now - we can change those on-demand.

@atsushi421
Copy link
Contributor Author

atsushi421 commented Mar 19, 2026

@atsushi421 I see. That seems like a very Pilot Auto-centric approach to me. Since Nebula is used by other projects and products, I'd suggest we add the same changes to Hesai in Nebula directly as well. Other vendors are fine right now - we can change those on-demand.

@mojomex
Thank you for the suggestion! We ideally wanted to apply CIE to all executors, but our initial plan was to start small and demonstrate results before expanding further considering this comment.
That said, if you're open to having the same changes applied to Hesai as well, we'd be happy to do so. Would it be okay to include the Hesai updates in this PR, or would you prefer a separate one?

@mojomex
Copy link
Collaborator

mojomex commented Mar 19, 2026

@atsushi421 I see, that makes sense. From a Nebula point of view, I would still prefer for you to add Hesai (in this PR is fine!).

If there's any unforseen roadblocks, we can split PRs, or do Hesai later as you suggested.
Thanks!

Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
@atsushi421 atsushi421 changed the title feat(nebula_continental): apply agnocast_components for CIE feat(nebula_continental, nebula_hesai): apply agnocast_components for CIE Mar 19, 2026
@atsushi421
Copy link
Contributor Author

@mojomex
I've applied the same changes to Hesai's CMakeLists.txt in this PR 👍 161ea6b
With ENABLE_AGNOCAST=1, if you launch hesai_ros_wrapper_node as a standalone executable, it will use CallbackIsolatedAgnocastExecutor. If you load it into a component container, you can use agnocast_component_container_cie to enable CIE.

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.

3 participants