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

Add force mode controller #1049

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

URJala
Copy link
Collaborator

@URJala URJala commented Jul 8, 2024

Added force mode arguments: damping and gain scaling to be usable with the force mode controller. damping and gain scaling will be changeable every time you start force mode when PR is implemented.
This is an update to PR

@URJala
Copy link
Collaborator Author

URJala commented Jul 9, 2024

Also this is dependent on PR for the service definitiion.

Also add example of using force mode controller
@fmauch fmauch changed the title Force mode arguments Add force mode controller Oct 10, 2024
@fmauch
Copy link
Collaborator

fmauch commented Oct 10, 2024

Current ToDos:

  • Make sure the controller is only able to activate, when no conflicting controllers are running.
  • Add documentation
  • Decide on the interface
    • Wrench / WrenchStamped?
    • Split limits vector into two? (Speed and deviation)
  • Add interfaces to ros2_control xacro

@domire8
Copy link

domire8 commented Oct 14, 2024

Hello, exciting to see this work in progress, we from AICA have already achieved force mode through ros2_control. My question is why you are implementing this through the async IOs and not as a full command mode in the write function? Is your goal to be able to have force mode active while another joint velocity controller is running, for instance?

Also, this PR is missing the updates in the xacro macros for the relevant command interface of the force_mode.

Happy to have a discussion, if there is interest from your side

@fmauch
Copy link
Collaborator

fmauch commented Oct 14, 2024

Hello, exciting to see this work in progress, we from AICA have already achieved force mode through ros2_control. My question is why you are implementing this through the async IOs and not as a full command mode in the write function? Is your goal to be able to have force mode active while another joint velocity controller is running, for instance?

Yes, exactly.

Also, this PR is missing the updates in the xacro macros for the relevant command interface of the force_mode.

Yes, this is still active work. But thanks for the hint I'll add it to the list.

Happy to have a discussion, if there is interest from your side

You don't happen to be at ROSCon next week?

@fmauch
Copy link
Collaborator

fmauch commented Oct 16, 2024

My question is why you are implementing this through the async IOs and not as a full command mode in the write function? Is your goal to be able to have force mode active while another joint velocity controller is running, for instance?

I've been thinking about this comment and in all honesty, I don't think there is a valid reason why communication is managed outside of write. Especially, since we are doing the communication from the controller to the HW interface in the update loop, it seems that this is even a problem in terms of thread safety...

Currently, this isn't working as expected and we don't really have a method
to counteract the robot "jumping" back to the joint configuration without
force mode.
Since it isn't compatible with streaming controllers, we deactivate it
for now. It can be activated at runtime.
This should be more thread-safe, as the buffers are filled from the same thread
as the write function is called.
@fmauch fmauch marked this pull request as draft October 16, 2024 17:48
@domire8
Copy link

domire8 commented Oct 17, 2024

I've been thinking about this comment and in all honesty, I don't think there is a valid reason why communication is managed outside of write. Especially, since we are doing the communication from the controller to the HW interface in the update loop, it seems that this is even a problem in terms of thread safety...

I can't say about thread safety, but I guess at the end the question is what you want to achieve. Our main goal for a force controller was to run force mode with an impedance controller as a primary control mode, while the robot has no other setpoints. For that, definitely the write function is the right place since the AsyncIO runs at a fixed (?) lower frequency. If however, you have applications in mind where one just enables force mode in one axis once and then uses joint velocity setpoints to drive the robot, the asyncio approach could work as well.

@urfeex urfeex marked this pull request as ready for review November 12, 2024 14:24
@urfeex urfeex requested a review from VinDp November 12, 2024 14:24
@urfeex urfeex self-assigned this Nov 14, 2024
@urfeex urfeex linked an issue Nov 14, 2024 that may be closed by this pull request
5 tasks
Copy link
Collaborator

@VinDp VinDp left a comment

Choose a reason for hiding this comment

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

It looks good and works as intended, I just have few suggestions/questions :)

{
// Reject if controller is not active
if (get_lifecycle_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE) {
RCLCPP_ERROR(get_node()->get_logger(), "Can't accept new action goals. Controller is not running.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
RCLCPP_ERROR(get_node()->get_logger(), "Can't accept new action goals. Controller is not running.");
RCLCPP_ERROR(get_node()->get_logger(), "Can't accept new requests. Controller is not running.");

}) ||
std::any_of(control_modes[0].begin(), control_modes[0].end(), [this](auto& item) {
return (item == hardware_interface::HW_IF_VELOCITY ||
item == hardware_interface::HW_IF_POSITION /*|| item == PASSTHROUGH_GPIO*/);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be uncommented?

Suggested change
item == hardware_interface::HW_IF_POSITION /*|| item == PASSTHROUGH_GPIO*/);
item == hardware_interface::HW_IF_POSITION || item == PASSTHROUGH_GPIO);

robot.call_service(
"/controller_manager/switch_controller",
SwitchController.Request(
activate_controllers=["force_mode_controller", "scaled_joint_trajectory_controller"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are they both in activate_controllers?

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

Successfully merging this pull request may close these issues.

Added service to control force mode from ROS2
6 participants