[WIP] Enforce joint limits #223

hsd-dev · 2020-07-07T13:33:20Z

Related to #198
[WIP] since have not tested.

ur_robot_driver/include/ur_robot_driver/ros/hardware_interface.h

gavanderhoorn · 2020-07-07T14:20:31Z

I'll let @fmauch do the detailed review, but two high-level comments:

was this code copied from somewhere? I seem to recognise it as the code used in the ros_control boilerplate for loading limits from the parameter server. If it was copied (doesn't matter from where), this fact should be recorded somewhere -- probably with a copyright statement and attribution somewhere
to make the diff easier to follow: try to avoid whitespace and layout changes (unless clang format asks/demands it)

Edit: looks like the format_check test failed.

hsd-dev · 2020-07-07T14:23:21Z

was this code copied from somewhere?

Yes from the boilerplate. There are very few examples out there :)

gavanderhoorn · 2020-07-07T15:05:05Z

was this code copied from somewhere?

Yes from the boilerplate. There are very few examples out there :)

Then we should mention that somewhere.

It might even be a good idea to put that code in a separate .cpp. That would make it even easier to add a license block at the top of the file.

gavanderhoorn · 2020-07-07T15:27:24Z

Will you do that @ipa-hsd ? (just making sure)

hsd-dev · 2020-07-07T15:30:01Z

Will you do that @ipa-hsd ?

Yes working on it.

hsd-dev · 2020-07-07T15:50:46Z

All checks passed, but not tested yet and updating as per @gavanderhoorn's comments.
Do Not Merge.

fmauch

This roughly looks quite good.

Because of the missing vector initialization I cannot test this currently, but I'll be happy to test this once that's done.

fmauch · 2020-07-07T15:42:39Z

ur_robot_driver/src/ros/hardware_interface.cpp

+                                            std::size_t joint_id)
+{
+  // Default values
+  joint_position_lower_limits_[joint_id] = -std::numeric_limits<double>::max();


As far as I can see, this vector doesn't get resized anywhere, which would making this call illegal.

fmauch · 2020-07-07T15:44:00Z

ur_robot_driver/src/ros/hardware_interface.cpp

+  bool has_soft_limits = false;
+
+  // Get limits from URDF
+  if (urdf_model_ == NULL)


Suggested change

if (urdf_model_ == NULL)

if (urdf_model_ == nullptr)

fmauch · 2020-07-07T15:44:40Z

ur_robot_driver/src/ros/hardware_interface.cpp

+  urdf::JointConstSharedPtr urdf_joint = urdf_model_->getJoint(joint_names_[joint_id]);
+
+  // Get main joint limits
+  if (urdf_joint == NULL)


Suggested change

if (urdf_joint == NULL)

if (urdf_joint == nullptr)

fmauch · 2020-07-07T15:47:29Z

ur_robot_driver/src/ros/hardware_interface.cpp

+    if (urdf_joint->type != urdf::Joint::CONTINUOUS)
+      ROS_WARN_STREAM("Joint " << joint_names_[joint_id]
+                               << " does not have a URDF "
+                                  "position limit");


This isn't only referring to position limits, right? We could change the output that no limits are configured for that joint in the URDF.

fmauch · 2020-07-07T15:57:54Z

ur_robot_driver/src/ros/hardware_interface.cpp

+    {
+      ROS_INFO_STREAM("Waiting for model URDF on the ROS param server at location: " << nh.getNamespace()
+                                                                                     << param_name);
+      nh.getParam(param_name, urdf_string);
+    }


This block doesn't make sense to me. In which situation should searchParam fail but getParam work? Am I missing anything?

You are right. Removed it for the next commit.

fmauch · 2020-07-20T12:29:43Z

@ipa-hsd Any plans / resources to continue this?

hsd-dev · 2020-07-21T12:54:54Z

@fmauch sorry for the delay. Was occupied last week. I have separated the code from the boilerplate example in a separate file. Please take a look if that make sense because I have not made it as a separate class, but just a separate .cpp

@gavanderhoorn I have also added the original license from the example.

fmauch · 2020-07-23T09:14:19Z

ur_robot_driver/include/ur_robot_driver/ros/hardware_interface.h

+   * \brief Applies joint limits & soft joint limits on position, velocity & effort defined in URDF / parameter server
+   *
+   */
+  void registerJointLimits(ros::NodeHandle& robot_hw_nh, const hardware_interface::JointHandle& joint_handle_position,


Does this function apply the limits or register the limits? I guess, the docstring needs to be adapted.

fmauch · 2020-07-23T09:16:19Z

ur_robot_driver/src/ros/hardware_interface.cpp

@@ -72,6 +75,9 @@ bool HardwareInterface::init(ros::NodeHandle& root_nh, ros::NodeHandle& robot_hw
  std::string output_recipe_filename;
  std::string input_recipe_filename;

+  // Load URDF file from parameter server
+  loadURDF(robot_hw_nh, "robot_description");


This should probably be the root_nh

fmauch · 2020-07-23T09:17:49Z

ur_robot_driver/src/ros/joint_limit_interface.cpp

+ *********************************************************************/
+
+/* Author: Dave Coleman
+   Desc:   Helper ros_control hardware interface that loads configurations


I think a note where this file comes from and that you added modifications to it (if you did) might be nice to add here.

gavanderhoorn · 2020-08-27T18:42:07Z

@ipa-hsd: would you see a chance to finish this?

hsd-dev · 2020-10-09T10:10:08Z

@fmauch a1422f1 addresses your previous comments.

hsd-dev · 2020-10-09T10:17:18Z

Any idea about:

  Starting function 'before_init'
  E: Failed to fetch http://packages.ros.org/ros/ubuntu/dists/xenial/InRelease  Clearsigned file isn't valid, got 'NOSPLIT' (does the network require authentication?)
  E: Some index files failed to download. They have been ignored, or old ones used instead.

gavanderhoorn · 2020-10-09T12:51:17Z

@ipa-hsd: could be ros-infrastructure/answers.ros.org#209.

hsd-dev · 2020-10-12T09:12:03Z

The checks look good now.

ljden · 2021-06-21T00:25:14Z

What's the status on this?

github-actions · 2023-04-08T22:37:44Z

This PR hasn't made any progress for quite some time and will be closed soon. Please comment if it is still relevant.

fmauch · 2023-04-11T10:58:44Z

though not being updated in a while, this topic is still important.

github-actions · 2023-07-13T22:38:06Z

This PR hasn't made any progress for quite some time and will be closed soon. Please comment if it is still relevant.

loads URDF model from parameter server

8828a43

hsd-dev mentioned this pull request Jul 7, 2020

Add support for ros_control joint limits? #198

Open

gavanderhoorn reviewed Jul 7, 2020

View reviewed changes

ur_robot_driver/include/ur_robot_driver/ros/hardware_interface.h Outdated Show resolved Hide resolved

hsd-dev force-pushed the joint_limits branch from 189d744 to 3520361 Compare July 7, 2020 14:16

hsd-dev force-pushed the joint_limits branch from 3520361 to c99685c Compare July 7, 2020 15:35

enforcing joint limits from urdf / parameter server

72463b3

hsd-dev force-pushed the joint_limits branch from c99685c to 72463b3 Compare July 7, 2020 15:38

fmauch requested changes Jul 7, 2020

View reviewed changes

fmauch added the wrid20 label Jul 7, 2020

hsd-dev force-pushed the joint_limits branch from e2ad636 to 7d01042 Compare July 21, 2020 12:59

Separated boilterplate example in a separate cpp with license

7315fef

hsd-dev force-pushed the joint_limits branch from 7d01042 to 7315fef Compare July 21, 2020 13:11

fmauch reviewed Jul 23, 2020

View reviewed changes

Added note to document changes from original file

a1422f1

restarting CI checks

7a4fcfb

gavanderhoorn mentioned this pull request Sep 23, 2021

Parameter naming in joint_limits.yaml UniversalRobots/Universal_Robots_ROS2_Driver#168

Closed

github-actions bot added the Stale label Apr 8, 2023

github-actions bot removed the Stale label Apr 11, 2023

github-actions bot added the Stale label Jul 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Enforce joint limits #223

[WIP] Enforce joint limits #223

hsd-dev commented Jul 7, 2020

gavanderhoorn commented Jul 7, 2020 •

edited

Loading

hsd-dev commented Jul 7, 2020 •

edited

Loading

gavanderhoorn commented Jul 7, 2020

gavanderhoorn commented Jul 7, 2020

hsd-dev commented Jul 7, 2020 •

edited

Loading

hsd-dev commented Jul 7, 2020 •

edited

Loading

fmauch left a comment

fmauch Jul 7, 2020

fmauch Jul 7, 2020

fmauch Jul 7, 2020

fmauch Jul 7, 2020

fmauch Jul 7, 2020

hsd-dev Jul 21, 2020

fmauch commented Jul 20, 2020

hsd-dev commented Jul 21, 2020

fmauch Jul 23, 2020

fmauch Jul 23, 2020

fmauch Jul 23, 2020

gavanderhoorn commented Aug 27, 2020

hsd-dev commented Oct 9, 2020

hsd-dev commented Oct 9, 2020

gavanderhoorn commented Oct 9, 2020

hsd-dev commented Oct 12, 2020 •

edited

Loading

ljden commented Jun 21, 2021

github-actions bot commented Apr 8, 2023

fmauch commented Apr 11, 2023

github-actions bot commented Jul 13, 2023

[WIP] Enforce joint limits #223

Are you sure you want to change the base?

[WIP] Enforce joint limits #223

Conversation

hsd-dev commented Jul 7, 2020

gavanderhoorn commented Jul 7, 2020 • edited Loading

hsd-dev commented Jul 7, 2020 • edited Loading

gavanderhoorn commented Jul 7, 2020

gavanderhoorn commented Jul 7, 2020

hsd-dev commented Jul 7, 2020 • edited Loading

hsd-dev commented Jul 7, 2020 • edited Loading

fmauch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmauch commented Jul 20, 2020

hsd-dev commented Jul 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavanderhoorn commented Aug 27, 2020

hsd-dev commented Oct 9, 2020

hsd-dev commented Oct 9, 2020

gavanderhoorn commented Oct 9, 2020

hsd-dev commented Oct 12, 2020 • edited Loading

ljden commented Jun 21, 2021

github-actions bot commented Apr 8, 2023

fmauch commented Apr 11, 2023

github-actions bot commented Jul 13, 2023

gavanderhoorn commented Jul 7, 2020 •

edited

Loading

hsd-dev commented Jul 7, 2020 •

edited

Loading

hsd-dev commented Jul 7, 2020 •

edited

Loading

hsd-dev commented Jul 7, 2020 •

edited

Loading

hsd-dev commented Oct 12, 2020 •

edited

Loading