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.

Sign up for GitHub

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

Jump to bottom

[jog_arm] SRDF velocity and acceleration limit enforcement #1863

Merged
merged 7 commits into from
Mar 9, 2020
Merged

[jog_arm] SRDF velocity and acceleration limit enforcement #1863

merged 7 commits into from
Mar 9, 2020

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Jan 23, 2020

Read velocity and acceleration limits from the SRDF and enforce them

@AndyZe AndyZe requested a review from tylerjw January 23, 2020 22:33
@AndyZe AndyZe changed the title [jog_arm] SRDF velocity and acceleration limit enforcement WIP [jog_arm] SRDF velocity and acceleration limit enforcement Jan 23, 2020
@AndyZe AndyZe changed the title WIP [jog_arm] SRDF velocity and acceleration limit enforcement [jog_arm] SRDF velocity and acceleration limit enforcement Jan 24, 2020
@AndyZe AndyZe mentioned this pull request Jan 24, 2020
Closed
@AndyZe AndyZe changed the title [jog_arm] SRDF velocity and acceleration limit enforcement WIP [jog_arm] SRDF velocity and acceleration limit enforcement Jan 26, 2020
@codecov-io
Copy link

codecov-io commented Jan 28, 2020 •
edited
Loading

Codecov Report

Merging #1863 into master will decrease coverage by 0.07%.
The diff coverage is 67.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1863      +/-   ##
==========================================
- Coverage   50.28%   50.21%   -0.08%     
==========================================
  Files         313      313              
  Lines       24647    24692      +45     
==========================================
+ Hits        12394    12398       +4     
- Misses      12253    12294      +41
Impacted Files Coverage Δ
.../moveit_jog_arm/include/moveit_jog_arm/jog_calcs.h 100% <ø> (ø) ⬆️
...veit_experimental/moveit_jog_arm/src/jog_calcs.cpp 68.96% <67.14%> (-1.04%) ⬇️
moveit_core/robot_model/src/joint_model.cpp 60.95% <0%> (-10.48%) ⬇️
moveit_ros/move_group/src/move_group.cpp 83.87% <0%> (-6.46%) ⬇️
...t_setup_assistant/src/tools/moveit_config_data.cpp 21.21% <0%> (-0.67%) ⬇️
moveit_ros/manipulation/pick_place/src/place.cpp 0.68% <0%> (ø) ⬆️
.../collision_detection_fcl/src/collision_env_fcl.cpp 89.83% <0%> (ø) ⬆️
...ene/include/moveit/planning_scene/planning_scene.h 50% <0%> (ø) ⬆️
moveit_core/planning_scene/src/planning_scene.cpp 46.13% <0%> (ø) ⬆️
...ning/robot_model_loader/src/robot_model_loader.cpp 77.77% <0%> (ø) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8a4072...da29e90. Read the comment docs.

@AndyZe AndyZe changed the title WIP [jog_arm] SRDF velocity and acceleration limit enforcement [jog_arm] SRDF velocity and acceleration limit enforcement Jan 28, 2020
@henningkayser
Copy link
Member

@AndyZe Please rebase

@AndyZe AndyZe changed the title [jog_arm] SRDF velocity and acceleration limit enforcement WIP [jog_arm] SRDF velocity and acceleration limit enforcement Mar 4, 2020
felixvd
felixvd reviewed Mar 4, 2020
View reviewed changes
moveit_experimental/moveit_jog_arm/include/moveit_jog_arm/jog_calcs.h
@@ -142,7 +143,7 @@ class JogCalcs

robot_state::RobotStatePtr kinematic_state_;

sensor_msgs::JointState joint_state_, original_joint_state_;
sensor_msgs::JointState incoming_joint_state_, internal_joint_state_, original_joint_state_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the internal_joint_state_ correspond to a "current" joint state?

Copy link
Member Author

Choose a reason for hiding this comment

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

What I mean is that it's used in calculations, it shouldn't be relied on to be accurate. Would there be a better way to say that?

I'm gonna try to get this list down to 2. It seems like incoming_joint_state_ and original_joint_state_ should be the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

It turns out that original_joint_state_ is a little bit different from incoming_joint_state_ so I left a comment to explain it. incoming_joint_state_ has every joint, but passive joints might get dropped from original_joint_state_

@AndyZe
Rename and comment joint_state variables better
c541dfa 
Add a placeholder test for vel/accel limits
@AndyZe
Copy link
Member Author

AndyZe commented Mar 4, 2020 •
edited
Loading

The test that was added (test_vel_accel_limits.py) will make more sense when I can check for specific error codes. For now, it's just a placeholder. PR #1915 adds the error codes

@AndyZe AndyZe changed the title WIP [jog_arm] SRDF velocity and acceleration limit enforcement [jog_arm] SRDF velocity and acceleration limit enforcement Mar 4, 2020
henningkayser
henningkayser approved these changes Mar 9, 2020
View reviewed changes
Copy link
Member

@henningkayser henningkayser left a comment

Choose a reason for hiding this comment

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

I tested this and it works without any issues for me

@henningkayser henningkayser merged commit 5327e41 into moveit:master Mar 9, 2020
@tylerjw tylerjw mentioned this pull request May 8, 2020
Closed
20 tasks
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 12, 2020
@AndyZe @tylerjw
[jog_arm] SRDF velocity and acceleration limit enforcement (moveit#1863)
bbda579 
* Loosen timing-based test tolerances

* Rename and comment joint_state variables better

* Add a placeholder test for vel/accel limits

* Remove useless update of kinematic_state_

* Fix suddenHalt()

* Improve Python integration tests
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 12, 2020
@AndyZe @tylerjw
[jog_arm] SRDF velocity and acceleration limit enforcement (moveit#1863)
e77f8c8 
* Loosen timing-based test tolerances

* Rename and comment joint_state variables better

* Add a placeholder test for vel/accel limits

* Remove useless update of kinematic_state_

* Fix suddenHalt()

* Improve Python integration tests
@AndyZe AndyZe mentioned this pull request May 18, 2020
Merged
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 20, 2020
@AndyZe @tylerjw
[jog_arm] SRDF velocity and acceleration limit enforcement (moveit#1863)
d2a6fe7 
* Loosen timing-based test tolerances

* Rename and comment joint_state variables better

* Add a placeholder test for vel/accel limits

* Remove useless update of kinematic_state_

* Fix suddenHalt()

* Improve Python integration tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixvd felixvd felixvd left review comments

@henningkayser henningkayser henningkayser approved these changes

@tylerjw tylerjw Awaiting requested review from tylerjw

@rhaschke rhaschke Awaiting requested review from rhaschke

@v4hn v4hn Awaiting requested review from v4hn

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@AndyZe @codecov-io @henningkayser @felixvd