Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions design_drafts/refactor_controller_manager.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
extract ros2 pub/sub/services into a `ros2_communication` helper files.

## Motivation:
In a project, we're wrapping `controller_manager` in a custom scheduler task, essentially re-writing the `ros2_control_node`

- **Conceptual ambiguity**: `ros2_control_node` is not a node, but an executable specific to the OS scheduler responsible for instantiating and running the `controller_manager`, which IS the node.
- **Code Maintainability**: `controller_manager.cpp` file is hitting 4k LoC, mixes core logic and ROS2 communicaiton (services, publishers, subscribers)

## Proposed changes
We propose a simple refactoring with two main objectives:

1. **Extract all ROS 2 interfaces** and their callback implementations into a helper file `controller_manager_ros.cpp/hpp` which current `controller_manager.hpp` would include.

2. **Rename the current `ros2_control_node`** to `ros2_control_executable` to accurately reflect its function as a system entry point rather than a Node
Copy link
Member

Choose a reason for hiding this comment

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

I personally don't see much value in renaming this and it'd definitely break a lot of people's code when we remove the old one.


## Notes:
naming is up for discussion, of course

#### Food for thought:
Outside of scope for this suggestion, but we could push it a step further and make `ControllerManager` not inherit from `Node` at all, and place all ROS-related stuff into `ControllerManagerNode` class.

Then, as we do with `ResourceManager`, we'd pass in the necessary node interfaces to it:
- executor (already passed)
- parameters
- logger
- clock

Then the `ControlNode` would be responsible for ros2 communication, controller manager for controllers and resource manager for hardware.