Skip to content

Allow updating robot gravity#1606

Open
AdamPettinger wants to merge 17 commits intoUniversalRobots:mainfrom
AdamPettinger:update_gravity
Open

Allow updating robot gravity#1606
AdamPettinger wants to merge 17 commits intoUniversalRobots:mainfrom
AdamPettinger:update_gravity

Conversation

@AdamPettinger
Copy link
Copy Markdown

@AdamPettinger AdamPettinger commented Dec 12, 2025

I have a UR20 mounted on another robot that can change its orientation enough to cause problems for the arm's mounting installation.

There has been some discussion on how to update the mounting orientation/gravity live (here and here), but these did not end up happening.

These changes expose the set_gravity URScript functionality through the ROS driver, allowing a user to update the gravity vector of the robot during live operation via a service. This is obviously risky if you calculate the gravity vector incorrectly.

There are corresponding changes in

I just rebased these commits on main today, although we have been running these changes forked from the jazzy branch for a few months now.

Copy link
Copy Markdown
Member

@urfeex urfeex left a comment

Choose a reason for hiding this comment

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

This all seems fair and well implemented. ros-industrial/ur_msgs#39 (comment) might influence this PR, however.

@urfeex urfeex added the enhancement New feature or request label Jan 20, 2026
Copy link
Copy Markdown
Member

@urfeex urfeex left a comment

Choose a reason for hiding this comment

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

I might have found the source of "fishyness". For all data processed by the control box, base is the correct one, not base_link.

On another topic: We would like to split up the gpio controller in the future, as it kind of became the dump place for all robot-specific functionalities. Would you mind moving this to a separate controller?

@AdamPettinger
Copy link
Copy Markdown
Author

I might have found the source of "fishyness". For all data processed by the control box, base is the correct one, not base_link.

Ah yes, thanks! Specifically, the urcl call is expecting the "up" (away from Earth) vector in the base frame. I have left the message as gravity = down, and negated it internally to make it up in this most recent commit. Sorry about the confusion! Our test hardware was mounted so that in the neutral base position, base_link and down was the same as base and up, which made it tricky to debug

Unfortunately, this confusion existed in the URCL merge too, just in the comments here. I can go update that if you want, or you can probably just drop that specific commit

@AdamPettinger
Copy link
Copy Markdown
Author

On another topic: We would like to split up the gpio controller in the future, as it kind of became the dump place for all robot-specific functionalities. Would you mind moving this to a separate controller?

I can give this a shot. Do you mean a gravity-specific controller, or moving some of the e.g. setPayload stuff to there as well? Or do gravity for now and others later? Based on those answers, opinion on name?

@urfeex
Copy link
Copy Markdown
Member

urfeex commented Jan 29, 2026

Unfortunately, this confusion existed in the URCL merge too, just in the comments here. I can go update that if you want, or you can probably just drop that specific commit

Oh, yes! I missed that while reviewing. If you could update that, that would be great, then I can review it.

I can give this a shot. Do you mean a gravity-specific controller, or moving some of the e.g. setPayload stuff to there as well? Or do gravity for now and others later? Based on those answers, opinion on name?

I've been thinking about this as well last evening. I think, there are arguments for both, one UR-specific functionality controller and one controller per functionality.
Since we will not rip apart the gpio_and_status controller for released distributions we would either have to put it in there or add a new controller. Hence, I think moving it to its own controller for now is the best trade-off: We can directly backport it and if we decide to merge functionality into one controller independent of I/Os, we have to break the ROS API, anyway, then we can also merge that one in.

Name wise, what would you think of "gravity update controller"?

@AdamPettinger
Copy link
Copy Markdown
Author

I'll split it off into a "gravity update controller". I don't know what you will do in the future, but exposing that and others (eg setPayload) as individual controllers seems reasonable in the sense that each user can tailor what controllers they want launched based on their needs. Maybe particularly relevant for setting gravity and payload as those have the potential to damage the robot if set wrong


It sounds like I will leave the gravity direction as is for now? The urscript docs for both the set_gravity and set_base_acceleration are generally confusing to me. For instance, set_gravity is actually expecting an acceleration, and the example calls in the set_base_acceleration don't account for gravity, so is it expecting acceleration that is in addition to gravity?

@urfeex
Copy link
Copy Markdown
Member

urfeex commented Jan 30, 2026

Yes, I can understand that. That's why I think, we should add a more consecutive example to the controller's documentation. If you want, I can contribute that. And yes, I think we can leave the vector as it is now.

set_base_acceleration() allows to compensate for a dynamic acceleration while leaving gravity untouched. The same thing can be achieved using set_gravity(), but there you'll always have to account for gravity, as well. Which one you would use probably depends a lot on the actual application. For example, in a mobile robot it might make sense to put IMU readings into set_gravity which will account for both, gravity and dynamic acceleration. On a linear axis you might not want to touch gravity once things are setup and use set_base_acceleration() to account for the linear axis acceleration.

However, as I said, I think the scope of this PR is very well established and we can focus on that.

@urfeex
Copy link
Copy Markdown
Member

urfeex commented Mar 2, 2026

@AdamPettinger just to be sure: I'm currently waiting for you to finish splitting this into its own controller. Or are you waiting for any input from the maintainers?

@AdamPettinger
Copy link
Copy Markdown
Author

@AdamPettinger just to be sure: I'm currently waiting for you to finish splitting this into its own controller. Or are you waiting for any input from the maintainers?

Ah sorry, I have had a busy few weeks, I will get to this soon! Not waiting on other input

AdamPettinger and others added 14 commits March 13, 2026 10:26
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
…uest

Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
@AdamPettinger
Copy link
Copy Markdown
Author

AdamPettinger commented Mar 16, 2026

Ok, broke it out into its own controller and tested with our hardware. This doesn't touch the GPIO controller at all now. I did go ahead and add the gravity_update_controller to the main launch file to start by default. I also updated the directionality comments in the other repo, here: UniversalRobots/Universal_Robots_Client_Library#462

Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Signed-off-by: AdamPettinger <adam.l.pettinger@gmail.com>
Copy link
Copy Markdown
Member

@urfeex urfeex left a comment

Choose a reason for hiding this comment

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

This is looking good now, thanks for iterating.

I'll need some time to test this and I'll merge the service first.

@urfeex
Copy link
Copy Markdown
Member

urfeex commented Mar 26, 2026

Tested and verified using a robot.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants