Remove front_steering from steering library

[qinqon]

To Accommodate controllers that are not only steering at front or rear this change remove the front_steering variable from steering_controller_library, as a byproduct of that the notion of front or rear wheel radius is also removed from dependant controllers and the library has know "traction_joints_names" and "steering_joints_names" instead of "front_wheels_names" and "rear_wheels_names".

Depends on:

• https://github.com/ros-controls/ros2_controllers/pull/1150

[christophfroehlich]

Have you read and considered #692? Ideally, we should solve it with this PR. I'm not sure if your proposal works for 4WD + 4x steering?

[qinqon]

Have you read and considered #692? Ideally, we should solve it with this PR.

The idea is the same, is all about generalize the steering library a little more.

I'm not sure if your proposal works for 4WD + 4x steering?

Next PR will allow that, I am thinking about extending the current ackermann instead of new controller, if you add a pair of traction and steering joints and define the instantaneus center of robot you can have this by modifyting a little the IK and odometry. Also you can keep the current ackermann by configuring the instantaneus centor or robot at the traction joints and using the same IK and odometry.

[mergify[bot]]

This pull request is in conflict. Could you fix it @qinqon?

[qinqon]

Tests working

$ colcon test 
Starting >>> steering_controllers_library
Finished <<< steering_controllers_library [0.44s]          
Starting >>> ackermann_steering_controller
Starting >>> bicycle_steering_controller
Starting >>> tricycle_steering_controller                                                                    
Finished <<< bicycle_steering_controller [1.50s]                                                                                                   
Finished <<< ackermann_steering_controller [1.58s]                                                            
Finished <<< tricycle_steering_controller [1.52s]

Summary: 4 packages finished [3.01s]

[mergify[bot]]

This pull request is in conflict. Could you fix it @qinqon?

[bmagyar]

@destogl please add your take

[qinqon]

@destogl I have rebase the PR, the idea is to prepare a PR based on this one to allow four steering at the ackerman controller.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 65.31365% with 94 lines in your changes missing coverage. Please review.

Project coverage is 84.77%. Comparing base (646b7b3) to head (8c48e10).

Files with missing lines	Patch %	Lines
...llers_library/src/steering_controllers_library.cpp	39.58%	41 Missing and 17 partials :warning:
...g_controller/src/ackermann_steering_controller.cpp	23.07%	14 Missing and 6 partials :warning:
...ng_controller/src/tricycle_steering_controller.cpp	33.33%	6 Missing and 4 partials :warning:
...ing_controller/src/bicycle_steering_controller.cpp	33.33%	4 Missing and 2 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1166      +/-   ##
==========================================
- Coverage   85.22%   84.77%   -0.46%     
==========================================
  Files         127      127              
  Lines       12006    12096      +90     
  Branches     1010     1036      +26     
==========================================
+ Hits        10232    10254      +22     
- Misses       1458     1503      +45     
- Partials      316      339      +23     

Flag	Coverage Δ
unittests	84.77% <65.31%> (-0.46%)	:arrow_down:

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

Files with missing lines	Coverage Δ
...roller/test/test_ackermann_steering_controller.cpp	100.00% <100.00%> (ø)
...roller/test/test_ackermann_steering_controller.hpp	87.61% <100.00%> (ø)
...t/test_ackermann_steering_controller_preceding.cpp	100.00% <100.00%> (ø)
...ntroller/test/test_bicycle_steering_controller.cpp	100.00% <100.00%> (ø)
...ntroller/test/test_bicycle_steering_controller.hpp	84.70% <100.00%> (+0.36%)	:arrow_up:
...est/test_bicycle_steering_controller_preceding.cpp	100.00% <100.00%> (ø)
...steering_controllers_library/steering_odometry.hpp	100.00% <100.00%> (ø)
...ring_controllers_library/src/steering_odometry.cpp	83.42% <100.00%> (+0.89%)	:arrow_up:
...library/test/test_steering_controllers_library.cpp	100.00% <100.00%> (ø)
...library/test/test_steering_controllers_library.hpp	97.22% <100.00%> (ø)
... and 7 more

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ARK3r]

@christophfroehlich I have a suggestion for arbitrarily long robots with an arbitrary number of traction and steering wheels in each "row". So for example if we have the following setups:

• bicycle setup 1: 2 rows, one row has a steerable wheel, second row has a traction wheel with the distance between row1 and row2 equal to 1.0 meter (wheelbase)
• bicycle setup 2: 2 rows, one row has a steerable and traction wheel, second row has a steerable and a traction wheel with the distance between row1 and row2 equal to 1.0 meter (wheelbase)
• 6 wheel rover (#1243): 3 rows, row1 has two steerable wheels, row2 only has two steerable + traction wheels, row3 has 1 steerable + traction wheel with (1) row1 having a track of 40 cm and (2) row1 and row2 having a distance of 50 cm (wheelbase) and (3) row2 having a track of 30 cm and (4) row2 and row3 having a distance of 20 cm (wheelbase) and (5) row3 having only one wheel.

Then the way each of these can be relayed to the controller could be as follows:

a list (an arbitrarily long number of rows) of lists (info for each row) of two lists and two doubles (steerable joints list, traction joints list, this row's track double, distance of this row to the previous row double) 🫠 where each row's list contains the following info:

[ 
   <list of steerable joints in this row: List>,
   <list of traction joints in this row: List>,
   <row track: Union[None, double]>,
   <wheelbase from prev row: Union[None, double]>
]

so examples of this would be:

• bicycle setup 1:

[
   [
      ["row1_steering_joint"],
      [ ],
      None,
      None
   ],
   [
      [ ],
      ["row2_traction_joint"],
      None,
      1.0
   ]
]

• bicycle setup 2:

[
   [
      ["row1_steering_joint"],
      ["row1_traction_joint"],
      None,
      None
   ],
   [
      ["row2_steering_joint" ],
      ["row2_traction_joint"],
      None,
      1.0
   ]
]

• 6 wheel rover:

[
   [
      ["row1_left_steering_joint", "row1_right_steering_joint"],
      [],
      0.4,
      None
   ],
   [
      ["row2_left_steering_joint", "row2_right_steering_joint" ],
      ["row2_left_traction_joint", "row2_right_traction_joint"],
      0.3,
      0.5
   ],
   [
      ["row3_steering_joint" ],
      ["row3_traction_joint"],
      None,
      0.2
   ]
]

EDIT: Alternatively, for more readability, something like robot_localization's handling of multiple sources of a single message type (like IMU here) can be implemented for row1, row2, ....

[christophfroehlich]

@ARK3r I'm not sure if this is the right place to discuss this (but it would make this PR here obsolete) Let's discuss this in #692

[Timple]

It's hard to see on mobile. But I see a separation between tracked and steered wheels. Can steered wheels also have traction with this pr? We have a vehicle with an active driven front wheel. Would be nice to use this 👍

[christophfroehlich]

It's hard to see on mobile. But I see a separation between tracked and steered wheels. Can steered wheels also have traction with this pr? We have a vehicle with an active driven front wheel. Would be nice to use this 👍

No, this is is not possible yet. I made a proposal once to configure this more freely, but haven't started going in that direction.

[qinqon]

I unearthed this PR, I changed my mind and think it makes sense to merge this before a possible rewrite of the library.

Thanks @qinqon for the initial work, on top of that I did

• add proper backwards compatibility

• updated the docs

• added migration and release notes

• fixed a bug, that with Ackermann kinematics a different wheel track of the steering and traction axle was not supported (although the parameters were there from the beginning).

@christophfroehlich happy that the PR did help with the effort, let me know on whatever you need.

[skasperski]

Hello, I am interested in the work here and would like to see it merged. I am currently testing this branch on jazzy and face some issues, probably due to parameterization. Is there any specific order required for traction_joint_names and steering_joint_names lists? Doesn't it need to know which joints are right and which are left?

@qinqon Are you still working on the 4WD patch? I'd be also willing to contribute or help with testing.

[christophfroehlich]

Hello, I am interested in the work here and would like to see it merged. I am currently testing this branch on jazzy and face some issues, probably due to parameterization. Is there any specific order required for traction_joint_names and steering_joint_names lists? Doesn't it need to know which joints are right and which are left?

Does your config work with the current version without this PR? After merging, we have to fix the ackermann_drive_controller of gz_ros2_control_demos, @skasperski maybe you want to work on that and create a PR there? then we can troubleshoot together if something is not working properly.

[skasperski]

Ok, I will look into this. For now the parameters are misspelled in the deprecation messages. Can I somehow add commits to this PR?

[christophfroehlich]

Ok, I will look into this. For now the parameters are misspelled in the deprecation messages. Can I somehow add commits to this PR?

you can review in the "Files changed" tab, and I can commit the changes.

[skasperski]

I am currently unable to compile these packages, because all tests fail to build with this error:

/ros_ws/src/ros2_controllers/ackermann_steering_controller/test/test_ackermann_steering_controller.cpp:34:26: error: ‘struct steering_controllers_library::Params’ has no member named ‘traction_joints_names’
   34 |     controller_->params_.traction_joints_names, testing::ElementsAreArray(traction_joints_names_));

I have no idea why that is, because the generated struct does have this member. Could this be some compatibility issue between jazzy and rolling?

Edit: Never mind, the binary installed version of this package had leaked into the workspace.

[skasperski]

I get this error when trying to launch the ackermann controller. It complains about the interface name, but as far as I can see it does start with the controller's name, so I am a bit clueless here:

[gz-1] [FATAL] [1739788697.287076537] [controller_manager]: Export of the state or reference interfaces failed with 
  following error: The name of the interface ackermann_steering_controller/linear/velocity does not begin with the 
  controller's name. This is mandatory for reference interfaces. Please correct and recompile the controller with name 
  ackermann_steering_controller and try again.
[gz-1] [INFO] [1739788697.287198080] [controller_manager]: Activating controllers: [ joint_state_broadcaster ]
[spawner-6] [ERROR] [1739788697.287824884] [spawner_ackermann_steering_controller]: Failed to configure controller

[christophfroehlich]

have you compiled ros2_control from source? there was a recent breaking change in the way how the interfaces get exported

[qinqon]

Hello, I am interested in the work here and would like to see it merged. I am currently testing this branch on jazzy and face some issues, probably due to parameterization. Is there any specific order required for traction_joint_names and steering_joint_names lists? Doesn't it need to know which joints are right and which are left?

@qinqon Are you still working on the 4WD patch? I'd be also willing to contribute or help with testing.

@skasperski I didn't have time to continue with the four wheels steering changes for ros2 controllers, but the idea was just to add another parameter to define where the instantaneous center of curvature point as a factor from wheel base, so you define the steering for front and rear wheels.

Like for example at a four wheels steering drive if front and rear wheel do same angle of steering the instantaneous center would be at 50% of the wheel base, but if their angle is not symmetric you can change that parameter and be different, like for example big trucks where rear wheels do steering but with less angle than front wheels.

[mergify[bot]]

This pull request is in conflict. Could you fix it @qinqon?