Skip to content

Conversation

@Joschi3
Copy link
Member

@Joschi3 Joschi3 commented Oct 19, 2025

This PR introduces the SafetyPositionController, a chainable ROS 2 controller that ensures safe joint position commands.

Key features

  • Unwraps continuous joints to avoid 2π jumps
    • Assumes HardwareInterface expects joint commands in (-inf, inf) [example Gazebo]
  • Enforces joint limits based on URDF definitions
  • Operates only in chained mode (no direct subscribers)
  • Exports reference interfaces (safety_position_controller//position) for use by upstream controllers (e.g., trajectory controller)

@Joschi3 Joschi3 self-assigned this Oct 19, 2025
@Joschi3 Joschi3 requested a review from Excelsius314 October 19, 2025 21:58
@Joschi3 Joschi3 requested a review from Excelsius314 November 6, 2025 17:19
Comment on lines 444 to 464
bool SafetyPositionController::gather_interface_indices()
{
bool success = true;
for ( size_t i = 0; i < params_.joints.size(); ++i ) {
state_interface_index_[i] = -1;
for ( size_t s = 0; s < state_interfaces_.size(); ++s ) {
auto name = state_interfaces_[s].get_name();
auto interface_name = state_interfaces_[s].get_interface_name();
if ( state_interfaces_[s].get_name() == params_.joints[i] + "/position" &&
state_interfaces_[s].get_interface_name() == "position" ) {
state_interface_index_[i] = static_cast<int>( s );
break;
}
}
if ( state_interface_index_[i] < 0 )
RCLCPP_WARN( get_node()->get_logger(), "No state interface 'position' found for joint '%s'.",
params_.joints[i].c_str() );
success &= ( state_interface_index_[i] >= 0 );
}
return success;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This propably can be omitted. State interfaces will be loaded in the same order as they are provided in state_interface_configuration() . If indices are required in a separate array, this can be handled in the respective loop from the state interface configuration.

bool success = true;
for ( size_t i = 0; i < params_.joints.size(); ++i ) {
if ( !std::isnan( commands[i] ) ) {
success &= command_interfaces_[i].set_value( commands[i] );
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also get an opt first and check for a value. Otherwise compiler complains && it is already implemented properly for reading operations

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case, set_value() returns a bool, so using &= works as intended and doesn’t trigger any compiler warnings. The optional/value check is only required when reading from interfaces (to ensure a value is available), not when setting them. I haven't encountered any compiler warnings.

const auto &limit = in_compliant_mode_
? params_.current_limits.joints_map[params_.joints[i]].compliant_limit
: params_.current_limits.joints_map[params_.joints[i]].stiff_limit;
success &= command_interfaces_[i + params_.joints.size()].set_value( limit );
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also get an opt first and check for a value. Otherwise compiler complains && it is already implemented properly for reading operations

@Excelsius314 Excelsius314 merged commit f22e953 into jazzy Nov 12, 2025
2 checks passed
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