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

Implemented spline interpolation in joint space #103

Closed
wants to merge 23 commits into from

Conversation

urmahp
Copy link
Collaborator

@urmahp urmahp commented Jun 9, 2022

No description provided.

fmauch and others added 3 commits January 9, 2023 10:48
Use a constant for stopj time and make sure the robot actually stops when trying to stop the servoj thread

servoj internally uses a step time that takes the current velocity scaling
into account. Therefore, a servoj command might block longer than one
control cycle. If we join the thread this operation will effectively wait
until that servoj command is finished, while the robot is still moving.

By explicitly killing and calling stopj we make sure the robot actually stops
This enables the possibility to have both spline interpolation and movej points in on trajectory
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Patch coverage: 20.75% and project coverage change: -1.67 ⚠️

Comparison is base (8dbe2a1) 78.36% compared to head (588e4c2) 76.69%.

❗ Current head 588e4c2 differs from pull request most recent head dafbdb8. Consider uploading reports for the commit dafbdb8 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
- Coverage   78.36%   76.69%   -1.67%     
==========================================
  Files         100       95       -5     
  Lines        4409     3987     -422     
  Branches      449      394      -55     
==========================================
- Hits         3455     3058     -397     
- Misses        690      706      +16     
+ Partials      264      223      -41     
Impacted Files Coverage Δ
...lude/ur_client_library/control/reverse_interface.h 100.00% <ø> (ø)
...lient_library/control/trajectory_point_interface.h 100.00% <ø> (ø)
include/ur_client_library/ur/ur_driver.h 33.33% <ø> (-26.67%) ⬇️
src/ur/ur_driver.cpp 51.11% <0.00%> (+2.13%) ⬆️
src/control/trajectory_point_interface.cpp 40.00% <8.10%> (-31.80%) ⬇️
tests/test_trajectory_point_interface.cpp 95.53% <100.00%> (+0.25%) ⬆️

... and 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@urrsk urrsk left a comment

Choose a reason for hiding this comment

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

It needs to use the target values to avoid unnecessary noise in interpolation of the spline

resources/external_control.urscript Outdated Show resolved Hide resolved
resources/external_control.urscript Outdated Show resolved Hide resolved
urrsk and others added 6 commits June 7, 2023 13:04
Use target values for the starting point of the spline to avoid unnecessary noise
The num_retries wouldn't be reset within each function, but it will now so the correct number of retries is performed in each function, before throwing an exception.
* Added tests for the comm classes

This add tests for
* bin parser
* package serializer
* TCP socket
* stream
* producer
* pipeline

Fixed check size in bin parser, so that it works with starndard data types

Added recconection time variable to TCP socket, this will control the time in between connection attempts if the server is unavailable.

Co-authored-by: Felix Exner <[email protected]>
The file in question got removed from master, so it got replaced with a
permalink.
- Optimized the code
- Added unittest for the trajectory point
- Added an example for spline
@urrsk
Copy link
Member

urrsk commented Jun 28, 2023

The rebase to master was not so easy, so I think we should verify the latest features like freedrive and tool contact before merging

@fmauch
Copy link
Collaborator

fmauch commented Jun 28, 2023

Thank you for the updates. I'll try to have a look into this tomorrow.

Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

First of all: Thank you both @urmahp and @urrsk for the tremendous work put into this. I've raised a couple of discussion points and it looks like we have to make the tests a bit more stable, but apart from that it seems very good.

I've tested this with UniversalRobots/Universal_Robots_ROS_Driver#543 and it seems to be working fine.


void SendTrajectory(const std::vector<vector6d_t>& p_p, const std::vector<vector6d_t>& p_v,
const std::vector<vector6d_t>& p_a, const std::vector<double>& time, bool use_spline_interpolation_)
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

There could be an assertion here that p_p has the same size as time, as this is a hard requirement.

examples/spline_example.cpp Outdated Show resolved Hide resolved
examples/spline_example.cpp Outdated Show resolved Hide resolved
Comment on lines +134 to +146
// Power it off
if (!g_my_dashboard->commandPowerOff())
{
URCL_LOG_ERROR("Could not send Power off command");
return 1;
}

// Power it on
if (!g_my_dashboard->commandPowerOn())
{
URCL_LOG_ERROR("Could not send Power on command");
return 1;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What exactly is the motivation behind this? I found it rather annoying while testing with a real robot. If we reduce this to only switching the robot an and releasing the brakes, it would have the same effect that we know the robot will be running afterwards without the additional switch off / switch on thing.

// otherwise we will get pipeline overflows. Therefore, do this directly before starting your main
// loop.

g_my_driver->startRTDECommunication();
Copy link
Collaborator

Choose a reason for hiding this comment

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

With having this line so far away from the main loop it is probably not really deterministic whether we will be able to read once the second data package is sent.

As the current joint positions are used in assembling the trajectory we can't simply move it down. One approach could be to start RTDE communication in a separate thread.

include/ur_client_library/ur/ur_driver.h Outdated Show resolved Hide resolved
include/ur_client_library/ur/ur_driver.h Outdated Show resolved Hide resolved
include/ur_client_library/ur/ur_driver.h Outdated Show resolved Hide resolved
return false;
}

control::TrajectorySplineType spline_type = control::TrajectorySplineType::SPLINE_QUINTIC;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be cleaned up a bit in such a way that we initialize the spline_type as SPLINE_QUADRATIC and change it to a higher order in the if clauses when velocity / acceleration information is given.

}

const unsigned number_of_points = 5;
const double s_pos[number_of_points] = { 4.15364583e-03, 4.15364583e-03, -1.74088542e-02, 1.44817708e-02,
Copy link
Collaborator

@fmauch fmauch Jun 30, 2023

Choose a reason for hiding this comment

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

There seems to be an error in unit conversion. The motions done by the robot are in fact a lot larger than this:

0: 0.0331154
1: 0.0331154
2: 0.0115529
3: 0.0434435
4: 0.0434435

Edit: Nevermind. This seems to work correct. It's just that the position / velocity combination inside the test is a bit unintuitive. And in fact, it seems to move the robot into its joint limits inside the tests. I would suggest using a simpler motion instead.

Additionally, I would suggest using a fixed motion and no relative motion to the starting state as this will have a much more predictable output.

@urrsk urrsk mentioned this pull request Jun 30, 2023
@fmauch
Copy link
Collaborator

fmauch commented Jul 6, 2023

Closing this in favor of merged #151

@fmauch fmauch closed this Jul 6, 2023
@urmahp urmahp deleted the spline_interpolation_v2 branch August 29, 2023 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants