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

[WIP] Tell getting new goal from cancel #10

Closed
wants to merge 3 commits into from

Conversation

pazeshun
Copy link

@pazeshun pazeshun commented Jun 18, 2018

Not tested in real robot yet.
Not tested in real robot yet.

This PR solves the issue written in start-jsk#765 (comment) (this issue is specific to HIRONX as start-jsk#765 (comment)).
With this PR, clearOfGroup or clearJointAnglesOfGroup is not called when new goal comes, while that is called when cancel topic comes.
Though this PR is not necessary for robots with new hrpsys (e.g., HRP2JSKNTS), this PR makes start-jsk#765 more similar behavior to current master branch (difference appears only when cancel topic comes).
I think this is more acceptable.

@pazeshun pazeshun changed the title [WIP] Tell getting new goal from cancel Tell getting new goal from cancel Jun 18, 2018
@pazeshun
Copy link
Author

pazeshun commented Jun 18, 2018

Tested in HIRONX with rebasing on origin/master (https://github.com/pazeshun/rtmros_common/tree/add-cancel-smooth-master)

@pazeshun
Copy link
Author

Tested in HRP2JSKNTS

@Affonso-Gui
Copy link

@k-okada We have been using this without problems in Hiro, and it was also tested in HRP2. Can we merge it?

@k-okada
Copy link
Owner

k-okada commented Sep 3, 2019 via email

@pazeshun pazeshun changed the title Tell getting new goal from cancel [WIP] Tell getting new goal from cancel Oct 25, 2019
@pazeshun
Copy link
Author

could you add test code to catch this?

@k-okada I created start-jsk#1070 to test cancelling and overwriting.
In terms of overwriting test, however, it always passes due to start-jsk#765 (comment).
I'll explain detail in start-jsk#1070.

for example send cancel to the actionlib while monitoring /joint_states and check the velocity.

In fact, we cannot get velocity without start-jsk#1069.
At start-jsk#1070, I use position only and that is enough.

@pazeshun
Copy link
Author

pazeshun commented Oct 25, 2019

@k-okada I added one commit (8881caa) to this PR because I thought clear() and clearJointAngles() is swapped in one place.
Is that fix correct for you?

The reason why we couldn't notice it until now is that the wrong part is in fullbody commander of HrpsysJointTrajectoryBridge and that commander is not used.
Instead, fullbody command from roseus is passed to commander of HrpsysSeqStateROSBridge.

@pazeshun
Copy link
Author

@Affonso-Gui I created start-jsk#1071. Please help me to test it next week.

@pazeshun
Copy link
Author

The contents of this PR were merged to https://github.com/start-jsk/rtmros_common/tree/master via start-jsk#1071

@pazeshun pazeshun closed this Dec 18, 2019
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