-
Notifications
You must be signed in to change notification settings - Fork 116
Add functionality to send MoveP and MoveC instructions to the robot #303
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #303 +/- ##
==========================================
- Coverage 76.05% 76.02% -0.03%
==========================================
Files 87 87
Lines 3684 3738 +54
Branches 416 414 -2
==========================================
+ Hits 2802 2842 +40
- Misses 665 679 +14
Partials 217 217
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cbb4b0d
to
cf2a8f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks good, I only have a couple of small suggestions and comments.
|
||
bool TrajectoryPointInterface::writeTrajectoryPoint(const vector6d_t* positions, const float acceleration, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is kept for backwards compatibility, would it make sense to mark it as deprecated and then moving forward we want people to use writeMotionPrimitive?
Otherwise if we still want to keep writeTrajectoryPoint, then we should make it possible from the UrDriver to writeTrajectorypoints with moveC and MoveP as well, because currently it is only possible with MoveL and MoveJ.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't necessarily see a reason to mark this deprecated, but I would also not see the necessity to add MoveC and MoveP to the UrDriver, as this is handled inside the InstructionExecutor.
Maybe this would be an argument to deprecated those...
This removes code duplication
This should help changing specific fields in the future.
Co-authored-by: Mads Holm Peters <79145214+urmahp@users.noreply.github.com>
That makes the whole interface more consistent. The public-facing API isn't affected by that.
I will merge this after #307 and include the communication protocol changes in the migration notes. |
Thank you for the integration of moveC! I just wanted to give quick feedback that I was able to use moveC via the instruction executer, both as a single command and within a motion sequence (with UR5e in URSim). I used it within my custom hardware interface: https://github.com/StoglRobotics-forks/Universal_Robots_ROS2_Driver_MotionPrimitive/blob/main/ur_robot_driver/src/motion_primitives_ur_driver.cpp |
This PR aims at supporting to send MoveP and MoveC commands to the robot directly through the TrajectoryPointInterface and the InstructionExecutor.