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

reset moveit_msgs::RobotState.is_diff to false #968

Merged
merged 2 commits into from Jul 4, 2018

Conversation

Projects
None yet
3 participants
@simonschmeisser
Copy link
Contributor

commented Jul 2, 2018

Currently once robot_state.is_diff has been set to true it will stay so. This breaks detaching attached objects. This is a fix for a regression introduced in #939

reset moveit_msgs::RobotState.is_diff to false
Currently once robot_state.is_diff has been set to true it will stay so. This breaks detaching attached objects. This is a fix for a regression introduced in #939
@v4hn

This comment has been minimized.

Copy link
Member

commented Jul 2, 2018

I approve this patch with reservations (see below).
Thank you for cleaning up after me @simonschmeisser ... 😞

On the other hand this wouldn't even be a problem if robotStateToRobotStateMsg used here and here would clear the is_diff field, but apparently it does not.
Isn't this the real bug you observe here?

@simonschmeisser

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2018

I think we should actually have both fixes. Recycling the message does not give any relevant performance wins but relying on the default value is also hard to follow.

@v4hn v4hn requested a review from rhaschke Jul 3, 2018

@v4hn

v4hn approved these changes Jul 3, 2018

Copy link
Member

left a comment

Thanks for adding the explicit is_diff=false too.

@rhaschke rhaschke merged commit 54d516a into ros-planning:kinetic-devel Jul 4, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

rhaschke added a commit to ubi-agni/moveit that referenced this pull request Jul 4, 2018

reset moveit_msgs::RobotState.is_diff to false (ros-planning#968)
Reset moveit_msgs::RobotState.is_diff to false whenever actually filling a RobotState msg.
This fixes a regression introduced in ros-planning#939.

@simonschmeisser simonschmeisser deleted the isys-vision:reset_robot-state-msg_is-diff branch Jul 4, 2018

dg-shadow added a commit to shadow-robot/moveit that referenced this pull request Jul 30, 2018

reset moveit_msgs::RobotState.is_diff to false (ros-planning#968)
Reset moveit_msgs::RobotState.is_diff to false whenever actually filling a RobotState msg.
This fixes a regression introduced in ros-planning#939.

MohmadAyman added a commit to MohmadAyman/moveit that referenced this pull request Aug 25, 2018

reset moveit_msgs::RobotState.is_diff to false (ros-planning#968)
Reset moveit_msgs::RobotState.is_diff to false whenever actually filling a RobotState msg.
This fixes a regression introduced in ros-planning#939.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.