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

Specify velocities for trajectory forwarding test #721

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

fmauch
Copy link
Collaborator

@fmauch fmauch commented Sep 9, 2024

Otherwise the trajectory points will never be sent.

Otherwise the trajectory points will never be sent.
@urrsk
Copy link
Member

urrsk commented Sep 9, 2024

Why does it never get sent?
Due to our new check on splines?

@fmauch
Copy link
Collaborator Author

fmauch commented Sep 10, 2024

Why does it never get sent? Due to our new check on splines?

Basically because of the logic in HardwareInterface::startJointInterpolation()

    if (!use_spline_interpolation_)
    {
      // write point for movej command
    }
    else  // Use spline interpolation
    {
      if (point.velocities.size() == 6 && point.accelerations.size() == 6)
      {
        // write quintic spline segment
      }
      else if (point.velocities.size() == 6)
      {
        // write cubic spline segment
      }
      else
      {
        // nothing being sent
        ROS_ERROR_THROTTLE(1, "Spline interpolation using positions only is not supported.");
      }
  }

In the test we use a position-only trajectory so we end up in the last case, where no trajectory point is being sent, as we default to spline-interpolated execution. Since in the past we used the movej interface by default, the test actually was working correctly and got missed when switching to spline interpolation by default, as it was falsely succeeding. UniversalRobots/Universal_Robots_Client_Library#184 implements actually failing if no trajectory point was received, so the trajectory execution in the test fails.

Things could tremendously improved as explained in #722 but I wanted to keep this one small to get it implemented, reviewed and merged quickly, so this isn't blocking UniversalRobots/Universal_Robots_Client_Library#184 anymore.

@urrsk
Copy link
Member

urrsk commented Sep 10, 2024

That makes sense.

@fmauch fmauch merged commit 999aa7e into UniversalRobots:master Sep 10, 2024
5 checks passed
@fmauch fmauch deleted the fix_trajectory_forward_test branch September 10, 2024 07:45
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