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] [jsk_robot_utils] Add robot-interface-utils.l to resolve name and file where robot-interface is defined #401

Closed
wants to merge 6 commits into from

Conversation

garaemon
Copy link
Member

@garaemon garaemon commented Aug 3, 2015

(load (robot-file (ros::get-param "/robot")))
(init-robot-from-name (ros::get-param "/robot"))

@garaemon garaemon force-pushed the robot-interface-utils branch from 64e8f18 to c0faa42 Compare August 3, 2015 04:02
@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

  • loading robot-interface file is heavy because it load robot model definition
    • ➡️ need to load one file rather than load all the files
  • it is difficult to know where the robot-interface class is defined
    • ➡️ write down the relationship between name of robots and files
    • ➡️ from a viewpoint of class hierarchy, writing robot-file function in robot-interface.l is not straight forward.

@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

Original definition is here

@k-okada
Copy link
Member

k-okada commented Aug 3, 2015

wehre do we define init-robot-from-name

is it something related to make-robot-model-from-name https://github.com/euslisp/jskeus/blob/master/irteus/irtutil.l#L378

do we still need xx-init function? https://github.com/start-jsk/rtmros_tutorials/blob/master/hrpsys_ros_bridge_tutorials/euslisp/hrp2jsk-interface.l#L44

@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

init-robot-from-name is defined in robot-interface.l

@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

pr2-init has small hook in pr2-init but I think we can remove the hooks and just use (instance pr2-interface :init)

@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

make-robot-model-from-name and make-robot-interface-from-name is a simple function to create
robot model and robot interface from string rather than class object.

User should load proper model files before using the functions.

@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

We have several alternatives to archive this goal:

  1. write robot-file function in robot-interface.l
  2. write robot-file function in somewhere else as this PR does
  3. All the robot should have ${ROBOT_NAME}eus package like pr2eus and peppereus and programs can load proper interface.l file.

@k-okada
Copy link
Member

k-okada commented Aug 3, 2015

so how about moving make-robot-model-from-name and make-robot-interface-from-name to robot-interface-utils.l so we can understand how to use them

Another question is : are you suggesting people to use

(load (robot-file (ros::get-param "/robot")))
(init-robot-from-name (ros::get-param "/robot"))
(make-robot-model-from-name)
(make-robot-interface-from-name)

instead of calling (hrp2-init) ?

(defun robot-file (name)
"You can get robot-interface file according to `name' argument.
You can create `*ri*' like
(progn (load (robot-file (ros::get-param \"/robot\")))
Copy link
Member

Choose a reason for hiding this comment

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

please follow https://github.com/ros-infrastructure/rep/pull/104/files and use standard param name for robot type, currently robot/type is suggested.

@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

Another question is : are you suggesting people to use

(load (robot-file (ros::get-param "/robot")))
(init-robot-from-name (ros::get-param "/robot"))
(make-robot-model-from-name)
(make-robot-interface-from-name)
instead of calling (hrp2-init) ?

My recommendation is:

(load (robot-file (ros::get-param "/robot")))
(init-robot-from-name (ros::get-param "/robot"))

rather than

(load "package://hrpsys_ros_bridge_tutorials/euslisp/hrp2jsknt-interface.l")
(hrp2jsknt-init)

make-robot-model-from-name should be useful to the people who want to use multiple robot models in planning and so on.
I supposed make-robot-interface-from-name should be enough to create *ri* for each robot but
it is a little bit convenient because it creates *robot*P andri` at the same time.

@k-okada
Copy link
Member

k-okada commented Aug 3, 2015

so can we remove

`(make-robot-interface-from-name)` from 
https://github.com/jsk-ros-pkg/jsk_pr2eus/blob/master/pr2eus/robot-interface.l#L1070

and all `xx-init` function from https://github.com/start-jsk/rtmros_tutorials/blob/master/hrpsys_ros_bridge_tutorials/euslisp/hrp2jsk-interface.l#L44, https://github.com/start-jsk/rtmros_tutorials/blob/master/hrpsys_ros_bridge_tutorials/euslisp/hrp2jsknt-interface.l#L67, and https://github.com/start-jsk/rtmros_tutorials/blob/master/hrpsys_ros_bridge_tutorials/euslisp/jaxon-interface.l#L112 ... ?

@garaemon garaemon force-pushed the robot-interface-utils branch from 355dd2f to c1b1c5f Compare August 3, 2015 04:37
@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

Yes,

@k-okada
Copy link
Member

k-okada commented Aug 3, 2015

humm, that's big change and I do not think people will switch from

hrp2-init

to

(load (robot-file (ros::get-param "/robot")))
(init-robot-from-name (ros::get-param "/robot"))

how about create

robot-init

that do every thing for you.

but still I'm afraid people using their custom xx-init function like
jsk-ros-pkg/jsk_pr2eus@284901f
...

◉ Kei Okada

On Mon, Aug 3, 2015 at 1:38 PM, Ryohei Ueda [email protected]
wrote:

Yes,


Reply to this email directly or view it on GitHub
#401 (comment)
.

@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

that's big change and I do not think people will switch from

The easiest solution is to remove xx-init functions.

how about create

robot-init

that do every thing for you.

sounds good but robot-init conflicts with a lot of existing codes.

@garaemon
Copy link
Member Author

garaemon commented Aug 3, 2015

but still I'm afraid people using their custom xx-init function like
jsk-ros-pkg/jsk_pr2eus@284901f

it was added 3 years ago and recently no robot has custom init function.
However I know all the code requires special initialization for each task.

@k-okada
Copy link
Member

k-okada commented Aug 3, 2015

So, question is most of people agree to use robot-init function instead of
using their xxx-init function. Anyone?

2015年8月3日月曜日、Ryohei [email protected]さんは書きました:

but still I'm afraid people using their custom xx-init function like
jsk-ros-pkg/jsk_pr2eus@284901f
jsk-ros-pkg/jsk_pr2eus@284901f

it was added 3 years ago and recently no robot has custom init function.
However I know all the code requires special initialization for each task.


Reply to this email directly or view it on GitHub
#401 (comment)
.

◉ Kei Okada

@garaemon
Copy link
Member Author

garaemon commented Aug 5, 2015

So, question is most of people agree to use robot-init function instead of
using their xxx-init function. Anyone?

any opinions?

@garaemon garaemon closed this Aug 7, 2015
@garaemon garaemon deleted the robot-interface-utils branch August 7, 2015 12:48
@garaemon garaemon restored the robot-interface-utils branch August 7, 2015 14:05
@garaemon garaemon reopened this Aug 7, 2015
@garaemon
Copy link
Member Author

特に意見が無いようですが,${robot}-initを廃止して,全ロボット対応版のrobot-initを導入して良いですか?

@snozawa
Copy link
Contributor

snozawa commented Aug 19, 2015

(話についてくのが遅くなりすいません。。。)

robot-initをよんだときに、各ロボットを指定する方法はどうするかんじになりますでしょうか?
get-param?
また、robots.lのようなものをつくることになるでしょうか?

@garaemon
Copy link
Member Author

(defun robot-init (&optional (robot-name (ros::get-param "/robot/type" (unix::getenv "ROBOT")))
  ...)

みたいな感じじゃないでしょうか。
デフォルトは/robot/typeで、なかったら$ROBOTになって、optionalで指定もできる。

robotの名前とファイルの対応付けはどこかに書かなくてはいけないですね。

@snozawa
Copy link
Contributor

snozawa commented Aug 19, 2015

環境変数つかいますか?

@garaemon
Copy link
Member Author

環境変数つかいますか?

個人的にはどちらでも良い気とおもっています。暗黙的な何かがはいってしまうからあえて環境変数から初期値はなくしたほうが良いかもしれません

@garaemon garaemon changed the title [jsk_robot_utils] Add robot-interface-utils.l to resolve name and file where robot-interface is defined [WIP] [jsk_robot_utils] Add robot-interface-utils.l to resolve name and file where robot-interface is defined Sep 17, 2015
@garaemon
Copy link
Member Author

作業し始めてやはりxx-initを消してrobot-initに強制的に移行させるというのはかなりラディカルな思想だなという気がしてきました。
warnを出すくらいの段階を考えるべきですね.

(defun pr2-init ()
  (load ...)
  (warn "YOU NEED TO USE ROBOT-INIT FUNCTION~%")
  (warn "PR2-INIT IS DEPRECATED AND IT WILL BE REMOVED IN NEAR FEATURE~%")
  (robot-init "pr2"))

@snozawa
Copy link
Contributor

snozawa commented Sep 17, 2015

作業し始めてやはりxx-initを消してrobot-initに強制的に移行させるというのはかなりラディカルな思想だなという気がしてきました。
warnを出すくらいの段階を考えるべきですね.

あ、それで消してたんですね。
僕は

warnを出すくらいの段階を考えるべきですね.

で移行期を設けるものかと思ってました。
移行期はあったほうがよいと思います。
できればwarning-messageでカラーになってるとオシャレ

@garaemon
Copy link
Member Author

sedスクリプトくらい用意しないと大変そうです。
一回プル陸を消したりして変更を整理します。

@garaemon
Copy link
Member Author

作業して気づきましたが、レポジトリをやはり変えるべきな気がしてきました。
robot-interface.lと同じレイヤで定義すべき問題な気がする。
ちょっと頭を冷やして落ち着いて整理したほうが良さそう。

@snozawa
Copy link
Contributor

snozawa commented Sep 17, 2015

robot-interface.lと同じレイヤで定義すべき問題な気がする。
ちょっと頭を冷やして落ち着いて整理したほうが良さそう。

リンクをたどるだけだったので今まで気づきませんでしたが、ここjsk_robotだったんですね。。。
robot-interface.lのファイルも読み込むので、robot-interface.lにあるのが良い気がします。

@garaemon
Copy link
Member Author

一旦closeして仕切り直します

@snozawa
Copy link
Contributor

snozawa commented Dec 25, 2015

こちらの議論ですが,ロボット名をenvでなくて引数or rosparamでということだったと思いますが,

  • 名前は/robot/typeでOKでしょうか.個人的にはよさそうに思います.genericすぎという声もあるかもしれません.
  • rosparam setするファイルはどのlaunchにしたら良いでしょうか.今JSKのロボット全員がincludeしてるlaunchファイルってあるんでしたっけ?

@k-okada
Copy link
Member

k-okada commented Dec 25, 2015

名前は/robot/typeでOKでしょうか.個人的にはよさそうに思います.genericすぎという声もあるかもしれません

This has been proposed and discussed at here ->
ros-infrastructure/rep#104

◉ Kei Okada

2015/12/25 18:35、Shunichi Nozawa [email protected] のメッセージ:

@snozawa
Copy link
Contributor

snozawa commented Dec 25, 2015

I see.

@YoheiKakiuchi
Copy link
Member

すごい蒸し返したはなしになるけど、環境変数はなんで無しなんだっけ?
rosparamにすると、ROSじゃないプログラムで使えなくてややこしくならないかな?

@snozawa
Copy link
Contributor

snozawa commented Dec 29, 2015

おおもとの議論は
https://github.com/start-jsk/rtmros_hrp2/issues/237
https://github.com/start-jsk/rtmros_hrp2/issues/268
にありますが、環境変数でなくrosparamからとってくるメリットは
https://github.com/start-jsk/rtmros_hrp2/issues/268#issuecomment-126547508
の、例えばROSのマスタが一環して情報を配る、などがあります。

環境変数が必要な部分もありそうで、hrp2のc、trans_systemのビルド系があります。

実際どっちにしてくかはケースバイケースですが、最近目にするロボット名解決のケースでは以下がありそうです。

  • launchにかいてあるものは、envでなくてargでよさそう。一番トップレベルではjaxon_hogehoge.launchのように呼ぶことがおおいので、このトップレベルからargで渡すことができそう。https://github.com/jsk-ros-pkg/trans_system/pull/575
  • Euslispでかつ(ros::roseus "hoge")してるようなファイルは、ロボット名はrosparamの/robot/nameなどをみるのでよさそう。ros::initしてるので。
  • EuslispでROS関係ないところは、比較的抽象化されていて、概ね知ってる限りそもそもロボット環境変数も/robot/nameもなくかけるところがほとんどのようにみえる。

@snozawa
Copy link
Contributor

snozawa commented Apr 25, 2016

ros-infrastructure/rep#104
で特に記載がなさそうでしたので質問なのですが、

  • robot/typeのロボットtypeは、hrp2jsknt, jaxon_red, ...etcのように、ROSの小文字、ハイフンでなくアンダーバー、などの命名速に則っていたらOKでしょうか。
  • robot/nameは何でしょうか。さっき@furuchichovからオフラインできいて、pr2はホスト名ではないとのことでしたが、例えばhrp2の17の場合、hrp2017、hrp2017c、hrp2017vのいずれが良いでしょうか。

@furushchev
Copy link
Member

PR2では
robot/typeがpr2で、robot/nameがpr1012,pr1040などのロボットを一意に特定できるものになっています。
pr1012はpr1012,pr1012s,pr1012basestationの3つのサーバからなるので、必ずしもhostnameと一致していないです。

@snozawa
Copy link
Contributor

snozawa commented Apr 25, 2016

pr1012はpr1012,pr1012s,pr1012basestationの3つのサーバからなるので、必ずしもhostnameと一致していないです。

一つはhostnameと一致してるやつがあるというかんじかな?

@YoheiKakiuchi
Copy link
Member

pr1012はpr1012,pr1012s,pr1012basestationの3つのサーバからなるので、必ずしもhostnameと一致してい> ないです。
一つはhostnameと一致してるやつがあるというかんじかな?

これは、どっちかというとhostnameの名付け規則の問題じゃないかな。
robot/typeはクラス名、robot/nameはインスタンス名で、それをどう使うかは、また別の問題。

@snozawa
Copy link
Contributor

snozawa commented Apr 25, 2016

rtmros_commonなロボットで自動でセットされるように、
hrpsys_ros_bridge.launchで
robot/type
robot/name
をデフォルトで設定されるようにしようと思ったのですが、
前者はロボット名でだいたい決まるのでOKそうですが、
後者はやめたほうがいいでしょうか(やめたほうがいいきがしてきました)。

@snozawa
Copy link
Contributor

snozawa commented Apr 25, 2016

rtmros_commonにPRしました
start-jsk/rtmros_common#944

@k-okada
Copy link
Member

k-okada commented Apr 25, 2016

通常はモデルファイルが違ったら,違うロボット(robot/type クラス名),とするのがやりやすいとは思います.
ただ,そうするとHRP2/Jaxonは全部違うロボットになっちゃうので,あまり意味がないのでクラス名={hrp2,
jaxon}みたいなのはしょうがない気はしますが.

◉ Kei Okada

On Mon, Apr 25, 2016 at 8:14 PM, Shunichi Nozawa [email protected]
wrote:

rtmros_commonにPRしました
start-jsk/rtmros_common#944
start-jsk/rtmros_common#944


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#401 (comment)

@snozawa
Copy link
Contributor

snozawa commented Apr 25, 2016

通常はモデルファイルが違ったら,違うロボット(robot/type クラス名),とするのがやりやすいとは思います.

そうですね。
start-jsk/rtmros_common#944
でもモデルファイルがちがったら別なrobot/typeとしてみました。
HRP2の場合、/robot/type=hrp2jsk, hrp2jsknt, hrp2jsknts, hrp2wで
JAXONの場合、/robot/type=jaxon, jaxon_redになります。
これで、VRMLおよびhrpsys_ros_bridge_tutorials/euslisp/xx-interface.lのファイルの分け方と同じになります。

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.

5 participants