Skip to content

Refactor: base node#1637

Merged
mjcarroll merged 12 commits into
ros2:rollingfrom
nadavelkabets:refactor/base-node
Apr 19, 2026
Merged

Refactor: base node#1637
mjcarroll merged 12 commits into
ros2:rollingfrom
nadavelkabets:refactor/base-node

Conversation

@nadavelkabets
Copy link
Copy Markdown
Contributor

@nadavelkabets nadavelkabets commented Mar 28, 2026

Part of #1620

Changes

  • Extracted BaseNode abstract base class from Node
    • BaseNode owns the C handle, logger, parameter management, graph discovery, name resolution and validation helpers
    • Node owns entity lists, callback groups, executor integration, Clock/TimeSource, entity creation, entity destruction, ParameterService / LoggingService / TypeDescriptionService initialization, wait_for_node, and context manager protocol
  • get_clock() is abstract in BaseNode (returns BaseClock), concrete in Node (returns Clock) — this lets AsyncNode provide its own AsyncClock
  • _parameter_event_publisher is typed as Optional[BasePublisher] in BaseNode and initialized to None, allowing subclasses to set it with their own publisher type

Notes

  • Node.__init__ signature is unchanged; BaseNode.__init__ drops start_parameter_services, parameter_overrides, automatically_declare_parameters_from_overrides, and enable_logger_service since those are executor-specific concerns

nadavelkabets and others added 8 commits March 16, 2026 13:37
@nadavelkabets
Feature: BaseClock
be0ddda 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@nadavelkabets
Fix: JumpHandle accept BaseClock, generic __repr__
a1ea926 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@nadavelkabets
Fix: Use BaseClock type in TimeSource, no __init__ parameters for ROS…
830b354 
…Clock

Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@nadavelkabets
Feature: add tests
a16824f 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@nadavelkabets
Feature: test ros time functions when clock type is not ros time
c0f4332 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@nadavelkabets @mjcarroll
Fix: called a property method
7e388d1 
Co-authored-by: Michael Carroll <carroll.michael@gmail.com>
Signed-off-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
@nadavelkabets
Refactor: initial extraction of BaseNode
d7c527b 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@nadavelkabets
Merge branch 'rolling' into refactor/base-node
4d819f1 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@nadavelkabets nadavelkabets changed the title Refactor/base node Refactor: base node Mar 28, 2026
@nadavelkabets
Move track_node to Node.__init__()
39873da 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@mjcarroll
Copy link
Copy Markdown
Member

mjcarroll commented Apr 10, 2026

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

mjcarroll
mjcarroll reviewed Apr 10, 2026
View reviewed changes
Comment thread rclpy/rclpy/node.py
mjcarroll
mjcarroll requested changes Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

we need to figure out what we are doing with the .__node member here

@nadavelkabets
Cr: add destroy_node method to BaseNode
e787f28 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@nadavelkabets nadavelkabets requested a review from mjcarroll April 11, 2026 10:59
@mjcarroll
Copy link
Copy Markdown
Member

mjcarroll commented Apr 11, 2026

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@nadavelkabets nadavelkabets mentioned this pull request Apr 11, 2026
Merged
@nadavelkabets
Merge branch 'rolling' into refactor/base-node
625ffc6 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@mjcarroll
Copy link
Copy Markdown
Member

mjcarroll commented Apr 12, 2026

Pulls: #1637
Gist: https://gist.githubusercontent.com/mjcarroll/edccf46592bf690ccf673b97c3eed0c6/raw/e89344bf8127a8edb2a07815cb84c4888bd0ebfc/ros2.repos
BUILD args: --packages-above-and-dependencies rclpy
TEST args: --packages-above rclpy
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18936

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@mjcarroll
Copy link
Copy Markdown
Member

mjcarroll commented Apr 14, 2026

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@mjcarroll
Copy link
Copy Markdown
Member

mjcarroll commented Apr 16, 2026

Pulls: #1637
Gist: https://gist.githubusercontent.com/mjcarroll/7b491a3c8fb5270aa16f76c232231eea/raw/e89344bf8127a8edb2a07815cb84c4888bd0ebfc/ros2.repos
BUILD args: --packages-above-and-dependencies rclpy
TEST args: --packages-above rclpy
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18980

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@mjcarroll
Copy link
Copy Markdown
Member

mjcarroll commented Apr 16, 2026 •
edited
Loading

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@mjcarroll
Copy link
Copy Markdown
Member

mjcarroll commented Apr 18, 2026

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@nadavelkabets
Merge remote-tracking branch 'upstream/rolling' into refactor/base-node
e6a5248 
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
@mjcarroll
Copy link
Copy Markdown
Member

mjcarroll commented Apr 18, 2026

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@mjcarroll mjcarroll merged commit 1ea7d02 into ros2:rolling Apr 19, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mjcarroll mjcarroll mjcarroll approved these changes

Assignees

@mjcarroll mjcarroll

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nadavelkabets @mjcarroll