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

Convert the diff_drive_controller into a ChainableControllerInterface in the master branch #1457

Open
arthurlovekin opened this issue Dec 31, 2024 · 4 comments

Comments

@arthurlovekin
Copy link

I am currently working on designing a differential drive robot with advanced navigation and balancing capabilities using ROS2 Jazzy. I would like to configure my controllers in a chained configuration similar to the one described in the Controller Chaining Tutorial. However, in the ros2_controllers master branch it looks like the diff_drive_controller is still a standard ControllerInterface, not a ChainableControllerInterface, so I cannot put controllers that precede the diff_drive_controller.

I propose to convert the diff_drive_controller into a ChainableControllerInterface, and push these changes to the master branch.
I noticed that there is a chainable-diff-drive branch from last year that converts the diff_drive_controller into a ChainableControllerInterface as I desire. The problem is that this branch is many commits behind master at this point, and lacks functionality such as using TwistStamped instead of Twists for the velocity_command subscriber, among others. I would like to see the chainable-diff-drive branch merged with master (or, since this would be a complex merge, simply start from scratch using chainable-diff-drive as a guide).

I have not contributed to ros2_controllers before, but would be interested in taking this on as a challenge. My questions are:
Why was this merge not made before? What are the blockers or nuances that I should watch out for?

@bmagyar
Copy link
Member

bmagyar commented Dec 31, 2024

I created this for the 2023 ROSCon workshop but it never made it upstream, hence why it is out of date.

Let me put it here in link form for posterity: https://github.com/ros-controls/ros2_controllers/tree/chainable-diff-drive

You'd be welcome to follow up on this by creating a branch from this branch in your fork and creating a pull request that passes all CI checks, tests and adds new tests for the chainable functionality. Sounds good?

@arthurlovekin
Copy link
Author

Sounds good. I will let you know if I have more questions.

@arthurlovekin
Copy link
Author

Issue: I'm struggling to build the master branch and pass all tests.
How do you normally build things and pass all of the tests successfully?
Is there a good place where I can get nitty-gritty help with this kind of problem?

Context: My fork for making the diff_drive_controller into a chainable controller is here. Right now I'm at the point where I've naively merged master into into the chainable-diff-drive branch. I did this in steps to try to understand the merge conflicts better, but at the end of the day I have build errors (see bottom) so I will have to look at this more closely.

However, before debugging that I thought it would be a good idea to make sure that the master branch builds successfully and passes all tests. Turns out I'm having issues here as well: my clone of the master branch is not passing all of the tests.

I found that even getting things to build was non-trivial so I include my steps. Here's what I've done so far:
Make a new workspace. In the src directory, run
git clone [email protected]:ros-controls/ros2_controllers.git
I also had to clone the control_toolbox and realtime_tools to avoid missing header errors
git clone [email protected]:ros-controls/control_toolbox.git
git clone [email protected]:ros-controls/realtime_tools.git
(When only the ros2_controllers were installed I tried to run rosdep install --from-paths src/ros2_controllers/diff_drive_controller/ but this didn't resolve the missing header errors. Why doesn't this work?)
After removing my old build and install folders, I could successfully build:
colcon build --symlink-install --packages-select diff_drive_controller control_toolbox realtime_tools --allow-overriding diff_drive_controller control_toolbox realtime_tools
(--allow-overriding is because I'm getting a warning that these packages are in my underlay workspace which is true, and I think it's not an issue. I also get a few deprecation warnings as described in issue 1442, but this is also ok.)

Now in a new terminal, source and run tests:
source install/local_setup.bash
colcon test --ctest-args tests --packages-select diff_drive_controller
colcon test-result --all --verbose

build/diff_drive_controller/Testing/20250102-2334/Test.xml: 2 tests, 0 errors, 1 failure, 0 skipped
- test_load_diff_drive_controller
  <<< failure message
    -- run_test.py: invoking following command in '/home/arthur/Projects/test_ws2/build/diff_drive_controller':
     - /home/arthur/Projects/test_ws2/build/diff_drive_controller/test_load_diff_drive_controller --gtest_output=xml:/home/arthur/Projects/test_ws2/build/diff_drive_controller/test_results/diff_drive_controller/test_load_diff_drive_controller.gtest.xml
    [==========] Running 1 test from 1 test suite.
    [----------] Global test environment set-up.
    [----------] 1 test from TestLoadDiffDriveController
    [ RUN      ] TestLoadDiffDriveController.load_controller
    [INFO] [1735860863.167963905] [test_controller_manager.resource_manager]: Loading hardware 'TestActuatorHardwareLeft' 
    [INFO] [1735860863.168259532] [test_controller_manager.resource_manager]: Loaded hardware 'TestActuatorHardwareLeft' from plugin 'test_actuator'
    [INFO] [1735860863.168277520] [test_controller_manager.resource_manager]: Initialize hardware 'TestActuatorHardwareLeft' 
    /home/arthur/Projects/test_ws2/build/diff_drive_controller/test_load_diff_drive_controller: symbol lookup error: /opt/ros/jazzy/lib/libtest_components.so: undefined symbol: _ZN18hardware_interface34parse_state_interface_descriptionsERKSt6vectorINS_13ComponentInfoESaIS1_EERSt13unordered_mapINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEENS_20InterfaceDescriptionESt4hashISC_ESt8equal_toISC_ESaISt4pairIKSC_SD_EEE
    -- run_test.py: return code 127
    -- run_test.py: generate result file '/home/arthur/Projects/test_ws2/build/diff_drive_controller/test_results/diff_drive_controller/test_load_diff_drive_controller.gtest.xml' with failed test
    -- run_test.py: verify result file '/home/arthur/Projects/test_ws2/build/diff_drive_controller/test_results/diff_drive_controller/test_load_diff_drive_controller.gtest.xml'
  >>>
build/diff_drive_controller/test_results/diff_drive_controller/test_diff_drive_controller.gtest.xml: 18 tests, 0 errors, 0 failures, 0 skipped
build/diff_drive_controller/test_results/diff_drive_controller/test_load_diff_drive_controller.gtest.xml: 1 test, 1 error, 0 failures, 0 skipped
- diff_drive_controller test_load_diff_drive_controller.gtest.missing_result
  <<< error message
    The test did not generate a result file:
    
    [==========] Running 1 test from 1 test suite.
    [----------] Global test environment set-up.
    [----------] 1 test from TestLoadDiffDriveController
    [ RUN      ] TestLoadDiffDriveController.load_controller
    [INFO] [1735860863.167963905] [test_controller_manager.resource_manager]: Loading hardware 'TestActuatorHardwareLeft' 
    [INFO] [1735860863.168259532] [test_controller_manager.resource_manager]: Loaded hardware 'TestActuatorHardwareLeft' from plugin 'test_actuator'
    [INFO] [1735860863.168277520] [test_controller_manager.resource_manager]: Initialize hardware 'TestActuatorHardwareLeft' 
    /home/arthur/Projects/test_ws2/build/diff_drive_controller/test_load_diff_drive_controller: symbol lookup error: /opt/ros/jazzy/lib/libtest_components.so: undefined symbol: _ZN18hardware_interface34parse_state_interface_descriptionsERKSt6vectorINS_13ComponentInfoESaIS1_EERSt13unordered_mapINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEENS_20InterfaceDescriptionESt4hashISC_ESt8equal_toISC_ESaISt4pairIKSC_SD_EEE
  >>>

Summary: 21 tests, 1 error, 1 failure, 0 skipped

Separate problem: the build errors I was encountering for my naive merge of the chainable-diff-drive branch can be summarized as:
TwistStamped_<std::allocator<void> > >, std::mutex>’} has no member named ‘reset’, 'readFromNonRT', 'writeFromNonRT', or 'readFromRT'

@christophfroehlich
Copy link
Contributor

christophfroehlich commented Jan 3, 2025

I'd suggest that you start from the current master branch, create a new branch and cherry-pick dea3d3c on top.

To cleanly install the rolling version of ros2_control stack, see the instructions for the "Development Version" here.
Then this should do the job (you either can add the allow overriding or uninstall your jazzy binary packages)

colcon build --symlink-install --packages-up-to diff_drive_controller
colcon test --packages-select diff_drive_controller

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants