jsk-ros-pkg / jsk_robot

jsk-ros-pkg/jsk_robot
https://github.com/jsk-ros-pkg/jsk_robot
73 stars 97 forks source link

add single panda #1799

Closed yuki-asano closed 10 months ago

yuki-asano commented 1 year ago

単腕のpandaを追加したものです。

Screenshot from 2023-05-09 16-07-56

yuki-asano commented 1 year ago

@pazeshun Thank you for your detail review at first. I'll check your comments.

yuki-asano commented 1 year ago

@pazeshun chould you approve changes?

pazeshun commented 1 year ago

@pazeshun chould you approve changes?

I should not approve until CI passes (https://github.com/jsk-ros-pkg/jsk_robot/pull/1208#issuecomment-1135488176). And currently, CI seems to fail due to apt server problem:

  + sudo apt-get install -y --force-yes -q -qq python-rosdep python-wstool python-catkin-tools
  W: --force-yes is deprecated, use one of the options starting with --allow instead.
  E: Package 'python-rosdep' has no installation candidate
  E: Package 'python-wstool' has no installation candidate
  E: Package 'python-catkin-tools' has no installation candidate

https://github.com/jsk-ros-pkg/jsk_robot/actions/runs/5150618620/jobs/9276407913?pr=1799 In this situation, we should wait for some time while sometimes restarting CI (too frequent restarting has a negative influence, though). I'll take care about CI, so please wait.

yuki-asano commented 1 year ago

OK. I missed the failed CI in the last. It is hidden and appears by scroling..

pazeshun commented 1 year ago

@k-okada Kindly ping

k-okada commented 1 year ago

@pazeshun sorry for being late,

However, we already have some demo codes depending on dual_panda's end-coords, so it is not easy to change it.

which demo uses this feature? I think we want to change the end coords near future(1-3 years), so how about add warning message to source code too,

(:end-coords (&rest args)
  (warning-message 2?3? (yeallow) "the endo coords of dual_pand is differnet from single panda and might be change in near future.)
  (send-super* :end-coords args))

in https://github.com/jsk-ros-pkg/jsk_robot/pull/1799/files#diff-da9673f8d3480112a7dbd60e430a179c2df50bf7f12437ae7419bd9777e0f357 if this is not so difficult.

pazeshun commented 1 year ago

which demo uses this feature?

For example, https://github.com/jsk-ros-pkg/jsk_demos/pull/1363/files#diff-e69875d71091c4531bb74bf07af5b703bc585828d41f8fea5aa8fbde9f4ef1e0R22. Also I used end-coords directly in my previous demo: https://gitlab.jsk.imi.i.u-tokyo.ac.jp/hasegawa/hasegawa_moonshot/-/blob/7b71edf5a7cb1c6ada46214f5aafb8d03f9d6cfa/hasegawa_moonshot/euslisp/lib/drilling-interface.l#L64-L66. Perhaps @haraduka may also use end-coords directly.

I think we want to change the end coords near future(1-3 years)

If you might allow this, then I think we should change end-coords right now, otherwise we will break too many demos because the number of demos will increase. But just now, we do not have enough time to test previous demos with the new end-coords. Do you OK to keep this PR open until testing finishes? Or should we just add a warning message and merge this PR? (But I'm afraid too many warning messages may be shown because :end-coords may be called everywhere. Also I may forget to fix end-coords...)

yuki-asano commented 1 year ago

I prefer to this if this is allowed.

Or should we just add a warning message and merge this PR?

k-okada commented 10 months ago

@pazeshun LGTM, as for https://github.com/yuki-asano/jsk_robot/pull/1#discussion_r1212576855, can we fix by https://github.com/jsk-ros-pkg/jsk_roseus/pull/746 ??

pazeshun commented 10 months ago

@k-okada Thank you, I confirmed https://github.com/jsk-ros-pkg/jsk_roseus/pull/746 fixes https://github.com/yuki-asano/jsk_robot/pull/1#discussion_r1212576855 even when :wait nil:

3.irteusgl$ send *ri* :get-start-grasp-result :rarm :wait nil
[ERROR] [1698643393.384314525]: [/franka_gripper/grasp] :get-result called without sending goal, returns null result
nil

Can we merge this PR as it is and consider https://github.com/jsk-ros-pkg/jsk_roseus/pull/746 as further improvement?

yuki-asano commented 10 months ago

Thank you!! @pazeshun @k-okada