[pr2_base_trajectory_action] remove dependency to pr2_controller_msgs #623

furushchev · 2016-06-22T03:43:12Z

What I did

refactor codes
remove pr2 related packages from dependencies
use control_msgs/FollowJointTrajectoryAction instead of pr2_controller_msgs
support publishing feedback message
Write test codes
Further Test on real robot / gazebo

k-okada · 2016-06-29T15:46:56Z

did you finished

Write test codes
Further Test on real robot / gazebo
??

Cc: @chiwunau

furushchev · 2016-07-01T05:34:11Z

@k-okada Sorry for late response. All has been done.

furushchev · 2016-07-01T05:35:19Z

@k-okada If this is merged, we can remove pr2 related packages from robot-interface.l (not pr2-interface.l). Should we move robot-interface.l to jsk_roseus?
( https://github.com/jsk-ros-pkg/jsk_pr2eus/pull/237/files#diff-15f6920b924513f6027d36b361f85bbdL5 )

furushchev · 2016-07-01T09:12:42Z

travis failed at test/pr2_base_trajectory_action.test.
I doubt that gazebo's base_controller does not work correctly. (at least different behavior from real robot)
@k-okada Can I disable gazebo test in this point?

furushchev · 2016-07-10T11:54:06Z

@k-okada How do you think of it?

k-okada · 2016-07-10T12:18:34Z

Why you have to disable gazebo test?

On Sunday, 10 July 2016, Furushchev [email protected] wrote:

@k-okada https://github.com/k-okada How do you think of it?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#623 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAeG3KrWqYFjQ5QUKeCToMjKXnFKMA1Vks5qUN1fgaJpZM4I7YTc
.

◉ Kei Okada

furushchev · 2016-07-10T12:33:11Z

@k-okada maybe gazebo's realtime controller does not follow the actual /cmd_vel (slower than command), and it always causes failure of following trajectory in this action server.
But I could not yet inspect codes of pr2_gazebo.

furushchev · 2016-07-10T12:34:47Z

oh, closed wrongly...

k-okada · 2016-07-10T12:46:50Z

In that case you can send command with large error threshold

On Sunday, 10 July 2016, Furushchev [email protected] wrote:

oh, closed wrongly...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#623 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAeG3HUyZLC7h4LuRAhJKjgGCblsfjvMks5qUObogaJpZM4I7YTc
.

◉ Kei Okada

furushchev · 2016-07-10T13:14:22Z

@k-okada Yes, we can increase error threshold, but I think it actually does not test anything...just for check if publish /cmd_vel, we dont need gazebo..

k-okada · 2016-07-11T04:07:56Z

even so, it is important to check the code actually publish something and
simulated robot receive them

◉ Kei Okada

On Sun, Jul 10, 2016 at 10:14 PM, Furushchev [email protected]
wrote:

@k-okada https://github.com/k-okada Yes, we can increase error
threshold, but I think it actually does not test anything...just for check
if publish /cmd_vel, we dont need gazebo..

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#623 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAeG3Nkgne8vnnVS4sNFKzAZK0OEa9YOks5qUPAugaJpZM4I7YTc
.

furushchev · 2016-07-11T04:50:06Z

@k-okada OK. I added note about this problem to test code and enabled test with broader thresholds.

furushchev · 2016-08-11T03:13:25Z

please merge after #644

* refactor codes * remove pr2 related packages from dependencies * use control_msgs/FollowJointTrajectoryAction * support feedback message * added test codes * some bugfixes of old codes * checked on real robot * update launch files

furushchev · 2016-08-11T06:42:49Z

@k-okada I used pr2eus to test with gazebo, and it does not work until jsk-ros-pkg/jsk_pr2eus#237 is merged.
There are two choices.

merge [pr2eus/robot-interface.l] use follow joint trajectory action jsk_pr2eus#237
release pr2eus to pass test with USE_DEB=true on this repo.
restart travis on this PR to test with new pr2eus

or.

merge this PR with temporally disable gazebo test
merge [pr2eus/robot-interface.l] use follow joint trajectory action jsk_pr2eus#237
release pr2eus to pass test with USE_DEB=true on this repo.
create new PR to enable gazebo test

how do you think of it? I recommend the latter.

k-okada · 2016-08-12T05:37:37Z

ok, I'll merge jsk-ros-pkg/jsk_pr2eus#237, before that please explain more
detail, because this will be a big change

which method has been changed, specially in robot-interface
since robot-interface is used in other robots, specially rtmros robots,
so why this change did not affect that robot, for example, I assume they
have override the information (
https://github.com/start-jsk/rtmros_common/blob/master/hrpsys_ros_bridge/euslisp/rtm-ros-robot-interface.l#L314),
so please list up such case in all method described in 1), and then they
will remove that code from rtm-ros-robot-interface in future.

Cc:@mmurooka

◉ Kei Okada

On Thu, Aug 11, 2016 at 3:42 PM, Furushchev [email protected]
wrote:

@k-okada https://github.com/k-okada I used pr2eus to test with gazebo,
and it does not work until jsk-ros-pkg/jsk_pr2eus#237
jsk-ros-pkg/jsk_pr2eus#237 is merged.
There are two choices.

merge [pr2eus/robot-interface.l] use follow joint trajectory action jsk_pr2eus#237
[pr2eus/robot-interface.l] use follow joint trajectory action jsk_pr2eus#237

restart travis on this PR to test with new pr2eus

or.

merge this PR with temporally disable gazebo test

merge [pr2eus/robot-interface.l] use follow joint trajectory action jsk_pr2eus#237
[pr2eus/robot-interface.l] use follow joint trajectory action jsk_pr2eus#237

create new PR to enable gazebo test

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#623 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAeG3ISXH5IbxnsMcp40Sd-9utGV7fd9ks5qesRqgaJpZM4I7YTc
.

snozawa · 2016-08-12T06:04:19Z

furushchev · 2016-08-27T03:08:00Z

@k-okada Sorry for late reply.

which method has been changed, specially in robot-interface

in robot-interface.l, I only changed to use control_msgs/FollowJointTrajectoryAction instead of pr2_controller_msgs/JointTrajectoryAction. And moved pr2_* messages to pr2-interface.l from robot-interface.l.
changed methods are :action-feedback-cb in controller-action-client class, :add-controller and:default-controller in robot-interface class and :init, :move-trajectory in robot-move-base-interface class.

since robot-interface is used in other robots, specially rtmros robots,
so why this change did not affect that robot, for example, I assume they
have override the information (
https://github.com/start-jsk/rtmros_common/blob/master/hrpsys_ros_bridge/euslisp/rtm-ros-robot-interface.l#L314),
so please list up such case in all method described in 1), and then they
will remove that code from rtm-ros-robot-interface in future.

Is there any robots that uses pr2_controller_msgs other than PR2?
In my thought, by this change, we will support wider variations of robot by using control_msgs, generalized messages, and removing pr2_* messages only can affect PR2 users.
And this PR removes dependency to pr2_ messages for even PR2.
(Sorry basically I could not understand your second question)

k-okada · 2016-08-27T06:15:24Z

old robots uses pr2_controller_msgs, for example, hrp2
https://github.com/start-jsk/rtmros_common/blob/master/hrpsys_ros_bridge/src/HrpsysJointTrajectoryBridge.cpp#L223-L228
now they accept both pr2_controller_msgs and controller_msgs.
This is why @snozawa 's originall PR at pr2eus depneds on pr2 -> https://github.com/jsk-ros-pkg/jsk_pr2eus/pull/197/files#diff-15f6920b924513f6027d36b361f85bbdR1066,
but newer PR uses https://github.com/start-jsk/rtmros_common/pull/887/files#diff-d9351e39dede27bec86e5a1c49dcfaa0R312
So, I hope they now switched to controller_msgs, but also afraid that someone still using old message.

furushchev · 2016-10-11T13:35:48Z

Why not merge this pull request? If there is problem, please complain.
Merging only jsk-ros-pkg/jsk_pr2eus#237 breaks pr2 unsafe moving.

furushchev · 2016-10-13T08:33:54Z

@k-okada seems answered at jsk-ros-pkg/jsk_pr2eus#237

k-okada · 2016-10-13T14:28:15Z

I think this PR is ok to merge, because it is only base_trajectry_action

furushchev · 2016-10-13T15:06:14Z

@k-okada Thank you for review and merging!

furushchev mentioned this pull request Jun 22, 2016

[pr2eus/robot-interface.l] use follow joint trajectory action jsk-ros-pkg/jsk_pr2eus#237

Merged

furushchev force-pushed the pr2-base-traj branch from 5f427d9 to 3efdf08 Compare July 1, 2016 05:33

furushchev changed the title ~~[WIP] [pr2_base_trajectory_action] remove dependency to pr2_controller_msgs~~ [pr2_base_trajectory_action] remove dependency to pr2_controller_msgs Jul 1, 2016

furushchev force-pushed the pr2-base-traj branch from b75b395 to 7a2ec3e Compare July 9, 2016 05:25

furushchev closed this Jul 10, 2016

furushchev reopened this Jul 10, 2016

furushchev force-pushed the pr2-base-traj branch from 7a2ec3e to b076b10 Compare July 11, 2016 04:49

furushchev force-pushed the pr2-base-traj branch 3 times, most recently from 7440014 to 1390b12 Compare July 14, 2016 14:31

k-okada added the MergeOK label Jul 19, 2016

[.travis.yml] use travis for building all distro

9588cfc

furushchev mentioned this pull request Aug 11, 2016

[.travis.yml] use travis for building all distro #644

Merged

furushchev force-pushed the pr2-base-traj branch 2 times, most recently from 68e8a7a to 2ee4457 Compare August 11, 2016 03:09

furushchev force-pushed the pr2-base-traj branch from 2ee4457 to 7bdd1c0 Compare August 11, 2016 03:55

[pr2_base_trajectory_action/CMakeLists.txt] temp disable test

9332f9c

Merge branch 'master' into pr2-base-traj

69cfd39

k-okada merged commit c48c304 into jsk-ros-pkg:master Oct 13, 2016

furushchev deleted the pr2-base-traj branch October 13, 2016 15:06

nakane11 mentioned this pull request Oct 15, 2022

[pr2_base_trajectory_action] Fix parameter names in pr2_base_link.yaml #1673

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pr2_base_trajectory_action] remove dependency to pr2_controller_msgs #623

[pr2_base_trajectory_action] remove dependency to pr2_controller_msgs #623

furushchev commented Jun 22, 2016 •

edited

Loading

k-okada commented Jun 29, 2016

furushchev commented Jul 1, 2016

furushchev commented Jul 1, 2016 •

edited

Loading

furushchev commented Jul 1, 2016 •

edited

Loading

furushchev commented Jul 10, 2016

k-okada commented Jul 10, 2016

furushchev commented Jul 10, 2016

furushchev commented Jul 10, 2016

k-okada commented Jul 10, 2016

furushchev commented Jul 10, 2016

k-okada commented Jul 11, 2016

furushchev commented Jul 11, 2016

furushchev commented Aug 11, 2016

furushchev commented Aug 11, 2016 •

edited

Loading

k-okada commented Aug 12, 2016

snozawa commented Aug 12, 2016

furushchev commented Aug 27, 2016

k-okada commented Aug 27, 2016

furushchev commented Oct 11, 2016

furushchev commented Oct 13, 2016

k-okada commented Oct 13, 2016

furushchev commented Oct 13, 2016

[pr2_base_trajectory_action] remove dependency to pr2_controller_msgs #623

[pr2_base_trajectory_action] remove dependency to pr2_controller_msgs #623

Conversation

furushchev commented Jun 22, 2016 • edited Loading

k-okada commented Jun 29, 2016

furushchev commented Jul 1, 2016

furushchev commented Jul 1, 2016 • edited Loading

furushchev commented Jul 1, 2016 • edited Loading

furushchev commented Jul 10, 2016

k-okada commented Jul 10, 2016

furushchev commented Jul 10, 2016

furushchev commented Jul 10, 2016

k-okada commented Jul 10, 2016

furushchev commented Jul 10, 2016

k-okada commented Jul 11, 2016

furushchev commented Jul 11, 2016

furushchev commented Aug 11, 2016

furushchev commented Aug 11, 2016 • edited Loading

k-okada commented Aug 12, 2016

snozawa commented Aug 12, 2016

furushchev commented Aug 27, 2016

k-okada commented Aug 27, 2016

furushchev commented Oct 11, 2016

furushchev commented Oct 13, 2016

k-okada commented Oct 13, 2016

furushchev commented Oct 13, 2016

furushchev commented Jun 22, 2016 •

edited

Loading

furushchev commented Jul 1, 2016 •

edited

Loading

furushchev commented Jul 1, 2016 •

edited

Loading

furushchev commented Aug 11, 2016 •

edited

Loading