Skip to content

Fix initending and mmtable#511

Closed
snozawa wants to merge 3 commits intostart-jsk:masterfrom
snozawa:fix_initending_mmtable
Closed

Fix initending and mmtable#511
snozawa wants to merge 3 commits intostart-jsk:masterfrom
snozawa:fix_initending_mmtable

Conversation

@snozawa
Copy link
Copy Markdown
Collaborator

@snozawa snozawa commented Apr 18, 2017

Update version of #508.

修正点

  • robot.l (hrp2jsk.lなど)の:init-endingをなくして、:init-endingは常にutils.lにあるようにする。min/max tableもutils.lの:init-endingで作る
  • &rest argsが元々:init-endingの要所要所についてましたが、不要だったのと、jskeusの:init-endingの引数仕様をかえてしまってることになるので、削除しました。
  • checkコードをつけてみました

@YoheiKakiuchi
Copy link
Copy Markdown
Member

これだと、運用的にはロボットモデルを作るときには必ずrobotname-interface.lを呼ぶということになるけど、
そうするということでいいか。
これまでも、robotname-utils.lを呼ばないと完全に使えるモデルはできていなかったわけだし。

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 18, 2017

これだと、運用的にはロボットモデルを作るときには必ずrobotname-interface.lを呼ぶということになるけど、
そうするということでいいか。
これまでも、robotname-utils.lを呼ばないと完全に使えるモデルはできていなかったわけだし。

そうですね、もう何年も前からrobot.lを単体で呼ぶことはなく、想定もされてないと思います。
utilsファイルを作ってしまっている以上、utilsを使わないでロボットモデルを使うと不具合がでるはずなので。

@k-okada
Copy link
Copy Markdown
Member

k-okada commented Apr 18, 2017 via email

@YoheiKakiuchi
Copy link
Copy Markdown
Member

min-max-tableを追加するロボットは必ず、utils.lでinit-ending的なものをつくって :define-min-max-table しなくちゃならない気がしてきた。

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 19, 2017

#511 (comment)

util.l は irteusでもうごくけど、-interface.l は roseus 出ないと動かない、という切り分けになりそうだけど、 実際にpr2-utils.lとかみると、行頭に (require :pr2 "package://pr2eus/pr2.l") とかありますね.これは (require :pr2 "pr2.l") でいけそう.

これはおっしゃるとおりですね。
utilsは、package://の記法のみがroseusに依存しています。
同じpackage内ではpackage://の記法を使わずパスの扱いをいいかんじにしたら、
irteusでもいけるようになると思います。
utilsファイルをさらにどこか別の場所から読み込んで使うには、
いずれにしてもpackage://記法が必要になるかと思うので、
その段階ではroseusはいるようになるように思います。
(それに関して昔先生方が議論されてた名残りのこーど:https://github.com/jsk-ros-pkg/euslib/blob/master/rbrain/robots.l#L651-L667)

あとは、roscoreがないと-interface.lは動かないけど、utilは動く、というのは今でも真か.

そうですね。
もう少し細かわけると

  1. roscoreがなくてもutilsは動く(動くべき)
  2. roscoreがなくてもinterfaceはloadできる。そのため、(progn (load "package://pr2eus/pr2-interface.l") (pr2)) のようにモデル生成もできる
  3. rosocreがないと、pr2-initなどのri関係はできない(それでもroscoreさえあれば、何もなくてもkinematics simulatorが動いてくれる)

というかんじでしょうか。

もともとの@YoheiKakiuchiさんのコメントの
#511 (comment)
の(roscoreもなく)モデル生成のみやりたい場合はどうするか、は
現状は2のやり方で自分は行っています。

別なアレとしては、riに関する依存関係もloadしたくなくて、
roseusのみで動くファイルをloadしたい、という要望があった場合は、
pr2.lもしくはpr2-utils.lを直接loadすることになるかもしれません。
この場合は、

  • pr2-utils.lのようなrobot-utils.lを直接loadするようにする
  • pr2-utils.lの中でpr2.lをloadせず、pr2.lの中でpr2-utils.lがprobe-fileされればpr2.lの中でpr2-utils.lをよぶようにして、robot.lを直接loadする

などがありそうです。
後者は、utilsの有無にかかわらず、robot.lをloadするだけで良いので
ユーザの使い勝手としてはシンプルですが、
自動生成ファイルから非自動生成ふぁいるが暗黙のうちに呼ばれたり
なんとなくやばいかもしれないです

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 19, 2017

#511 (comment)

min-max-tableを追加するロボットは必ず、utils.lでinit-ending的なものをつくって :define-min-max-table しなくちゃならない気がしてきた。

そうですね。
#508
だと、生成されたjaxon.lにもjaxon-utils.lにも:init-endingがあって、かつ
前者はinit-endingが書いてあるけど呼ばれてない、という形になってて、
hrp2jskだとhrp2jsk-utils.lがこのPRまでは追加してないので、

  • 書いてあるけど呼ばれなくなるメソッドができてしまう
  • 同じようなことが書いてあるファイルが2種類でてきてしまい、かつロボットによってどっちが正解かがかわってしまう

の2点がアレだったので、
#509
ではどちらも呼ばれるようにして上記2点を解決しました。

このPRでは、robot.lの:init-endingを削除してutils.lにのみ:init-endingをかくことに
統一するようにしてみて、上記2点を解決しています。
このPRは、新しくロボットをつくったときにinit-endingをかかないとmin/maxのよび忘れがでてしまうのが
欠点ですが、その他の議論も含めて、大抵のロボットはまずはutils.lをかいておくので良いきもしています
(ロボットごとにutilsがある・なしがあると若干面倒)

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 19, 2017

超脱線してしまいますが、オブジェクト指向でオーバーライドは明確に単語の定義がありますが、
(多分eus特有の機能?)同じクラスのメソッドをあとで上書きするのって、
何か単語の定義ってあるんでしょうか。

(defclass hogehoge)
(defmethod hogehoge
  (:testtest () (print "hogehoge")))

(defclass fugafuga
  :super hogehoge) ;; サブクラス
(defmethod fugafuga
  (:testtest () (print "fugafuga1"))) ;; これはオーバーライド                                                                                                                                                                                                                                                             

(defmethod fugafuga ;; 同じクラスで定義
  (:testtest () (print "fugafuga2"))) ;; これは?overwrite??  呼び名がある?

@k-okada
Copy link
Copy Markdown
Member

k-okada commented Apr 19, 2017

同じpackage内ではpackage://の記法を使わずパスの扱いをいいかんじにしたら、

昨晩気がついたんですが、loadが書いてあるファイルと同じディレクトリにあるプログラムは優先して読み込んでくれるみたいです.試してみて下さい.
https://github.com/euslisp/EusLisp/blob/EusLisp-9.23/lisp/l/loader.l#L246
lisp:*loader-current-directory* というところにloadが呼ばれたファイルのパスが書いてある.

utils.l は良くない風習で、落ち着いて考えると、pr2.dae -> pr2-model.l として、pr2.lという名前でutils.l 的なものを作るべきだったね、という話ではあるかと思います.が、後の祭り.
言い訳すると、utils.lにあるものはいずれirteusに入れるべきだから、テンポラリのファイルで将来は無くしたい、ということでは有りますが、実際は難しそう.

現状、

bodyはmodelかえたらいいし、coordsはどうするんだろうね.リンクを作ればいいのかな.

が真の正解、ということでいいとすると、あまり見たくないrobotmodelの:init-ending を更にロボット毎に変えないと
モデルが動かない状況になっているところを前提に話を積み上げるのよりは、現状では新しいルールを追加しない方向で、
ここで変更したことで、これからjaxon系の:init-endingはこうあるべし、しからば、jskeusもこう直すべし、とならないよう注意しつつ、
上書きされた:init-ending を直していく、というのでいいともいます.

(defmethod fugafuga ;; 同じクラスで定義
  (:testtest () (print "fugafuga2"))) ;; これは?overwrite??  呼び名がある?

再定義?

k-okada@p40-yoga:~/catkin_ws/ws_fetch/src/jsk_robot/jsk_fetch_robot/fetcheus$ git diff
diff --git a/jsk_fetch_robot/fetcheus/fetch-utils.l b/jsk_fetch_robot/fetcheus/fetch-utils.l
index ea2be57..6532a84 100644
--- a/jsk_fetch_robot/fetcheus/fetch-utils.l
+++ b/jsk_fetch_robot/fetcheus/fetch-utils.l
@@ -1,7 +1,8 @@
 ;;
 ;;
 ;;
-(require :fetch "package://fetcheus/fetch.l")
+(require :fetch "fetch.l")
+(print lisp:*loader-current-directory*)
 
 (defmethod fetch-robot
   (:inverse-kinematics
k-okada@p40-yoga:~/catkin_ws/ws_fetch/src/jsk_robot/jsk_fetch_robot/fetcheus$ cd -
/tmp
k-okada@p40-yoga:/tmp$ unset ROS_PACKAGE_PATH
k-okada@p40-yoga:/tmp$ rospack find fetcheus
[rospack] Error: package 'fetcheus' not found
k-okada@p40-yoga:/tmp$ cat hoge.l
(load "/home/k-okada/catkin_ws/ws_fetch/src/jsk_robot/jsk_fetch_robot/fetcheus/fetch-utils.l")

k-okada@p40-yoga:/tmp$ irteusgl hoge.l
configuring by "/opt/ros/indigo/share/euslisp/jskeus/eus//lib/eusrt.l"
;; readmacro ;; object ;; packsym ;; common ;; constants ;; stream ;; string ;; loader ;; pprint ;; process ;; hashtab ;; array ;; mathtran ;; eusdebug ;; eusforeign ;; coordinates ;; tty ;; history ;; toplevel ;; trans ;; comp ;; builtins ;; par ;; intersection ;; geoclasses ;; geopack ;; geobody ;; primt ;; compose ;; polygon ;; viewing ;; viewport ;; viewsurface ;; hid ;; shadow ;; bodyrel ;; dda ;; helpsub ;; eushelp ;; xforeign ;; Xdecl ;; Xgraphics ;; Xcolor ;; Xeus ;; Xevent ;; Xpanel ;; Xitem ;; Xtext ;; Xmenu ;; Xscroll ;; Xcanvas ;; Xtop ;; Xapplwin
connected to Xserver DISPLAY=:0
X events are being asynchronously monitored.
;; pixword ;; RGBHLS ;; convolve ;; piximage ;; pbmfile ;; image_correlation ;; oglforeign ;; gldecl ;; glconst ;; glforeign ;; gluconst ;; gluforeign ;; glxconst ;; glxforeign ;; eglforeign ;; eglfunc ;; glutil ;; gltexture ;; glprim ;; gleus ;; glview ;; toiv-undefined ;; fstringdouble irtmath irtutil irtc irtgeoc irtgraph pgsql irtgeo euspqp pqp irtscene irtmodel irtdyna irtrobot irtsensor irtbvh irtcollada irtpointcloud irtx eusjpeg euspng png irtimage irtglrgb
;; extending gcstack 0x5fb3de0[16374] --> 0x6410120[32748] top=3d1f
irtgl irtglc irtviewer
EusLisp 9.22(e4e5a3b 1.1.0) for Linux64 created on p40-yoga(Fri Apr 14 23:29:23 JST 2017)
("/home/k-okada/catkin_ws/ws_fetch/src/jsk_robot/jsk_fetch_robot/fetcheus/" "./" "./")
1.irteusgl$ fetch

;; extending gcstack 0x6410120[32738] --> 0xcf54640[65476] top=7fe1
#<fetch-robot #X680fcb0 fetch  0.0 0.0 0.0 / 0.0 0.0 0.0>
2.irteusgl$ exit

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 19, 2017

言い訳すると、utils.lにあるものはいずれirteusに入れるべきだから、テンポラリのファイルで将来は無くしたい、ということでは有りますが、実際は難しそう.

そうですね。
:init-endingはbody, coordsの問題がありつつ、他にはutilsには

  • つま先つきHRP2ではデフォルトつま先軸のweightを0にしたり、
  • ハンドをくっつけたり、
  • hrp4rはvirtual force sensorに関するコードがあったり、

などがありそうで、それぞれ例えば
euslisp/jskeus#282
などのように、ちょっとずつ個別設定が少なくするように
しようとして、まだ途中というかんじですね。

昨晩気がついたんですが、loadが書いてあるファイルと同じディレクトリにあるプログラムは優先して読み込んでくれるみたいです.試してみて下さい.

たしかに、これ今まで積極的に同じディレクトリのファイルをloadする機能を使ってなかったですが、
この方がpackage://への依存がなくなって良いかんじがしますね
(実はむかし、roseusのpackage://のパス解決がやや遅いのでカレントディレクトリでloadするようにしてたころがあります。今はパス解決速度はどうなんでしょうかね)
package://を極力使わない形にして、PRを更新してみます

@k-okada
Copy link
Copy Markdown
Member

k-okada commented Apr 19, 2017 via email

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 19, 2017

#508と#509は
#511 (comment)
にも少しかきましたが、
#508のcons

  • 書いてあるけど呼ばれなくなるメソッドができてしまう(:init-ending)
  • 同じようなことが書いてあるファイルが2種類でてきてしまい、かつロボットによってどっちが正解かがかわってしまう

の2点があり、#509ではそれがないので、#509が良いと思っているのですがいかがでしょうか。
その場合は、このPRがcloseで、#509https://github.com/snozawa/rtmros_tutorials/blob/570f0444d7d90a294fcd07301887bea110e173c1/hrpsys_ros_bridge_tutorials/test/test-all-robots-check.l になります。

@k-okada
Copy link
Copy Markdown
Member

k-okada commented Apr 19, 2017 via email

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 19, 2017

ありがとうございます
argsはミスでしたので、戻しました。
(508と509で甲乙つけがたいですが)#509とtest codeを合わせて以下にPRしました。
#512

@snozawa snozawa closed this Apr 19, 2017
@k-okada
Copy link
Copy Markdown
Member

k-okada commented Apr 19, 2017 via email

@YoheiKakiuchi
Copy link
Copy Markdown
Member

新しいクラスをつくって、send-super するのは検討したんですが、やはり、class名が変わってしまうのが気になる。
(例えば、ここ https://github.com/YoheiKakiuchi/rtmros_tutorials/blob/master/hrpsys_ros_bridge_tutorials/euslisp/jaxon-interface.l#L9-L11 で classを渡しているので)

classのreplacaですが、

(defclass jaxon-robot-new
  :super jaxon-robot
  :slots ())
(unless (boundp 'jaxon-robot-org)
  (setf (symbol-value jaxon-robot-org) (symbol-value jaxon-robot))
  (setf (symbol-value jaxon-robot) (symbol-value jaxon-robot-new))
  ;; これは (setq jaxon-robot jaxon-robot-new) と同じはず
  )

とすると、jaxon-robotに #<metaclass #Xmemaddress jaxon-robot-new> バインドされる。
いまのところこれで困ることを思いついていません。

send #<metaclass #Xmemaddress jaxon-robot-new> :name "jaxon-robot" すると、
名前も変えられます。が、この名前はほとんど使っていないように思っています。
defclass myclass で与えられたmyclassのシンボルにmataclass(nameスロットがmyclass)
のインスタンスがバインドされるというのが大前提で、nameスロットは表示くらいにしか使っていない気もします。

ですが、メソッドの付け替えよりも禁じ手な気がします。

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 19, 2017

ぐらいが、#508から次の段階のスタート地点かな、と思いました.

#509が新しくルールを作ってるのと同様に、#508もutils.lで何かをするという意味で、
同様のルールが相当しますし、以下2点に関しても新たにルールを設定していることになります。
#511 (comment)
したがって、#508と#509は、ルールをもうける観点ではあまり優劣を個人的には感じてないです
(おっしゃるとおりいずれも良いルールでないので、「ルールのヤバさが同じくらい」という意味です)
それで#508では2点くらいやばそうな点がある他、
#511 (comment)
でコメントいただいた中の言葉をお借りすると、
#509がよさそうに思う理由のもう一つは「罪悪感が伝わって良い」からです。
#508はそれ自体みると、とくに何も問題なさそうですが、実はその背景には上記の2点の副作用を暗黙につかってるので、後々メンテナンスがしんどくなりそうです。
#509はぱっとみでヤバいので、(もちろんベストではないですが)#509でなく#508にする
理由もとくにないのかなと思います。

それで、
#511 (comment)
の他の部分(upstreamで行うべき)は同感です。
その方法をいくつかあげていただいてますが、以下自分の意見です。

「class名をかえる方法」
class名をあとで変えるのは、#509と同じくらいヤバイので、長期的には違う方向性がよさそうに思いました。

「classを特性ごとに継承させて新しく定義していく」
きちんとクラスをわけるのはややアリっぽいです。
rbrainが過去そうなっていて、クラス階層が深くなるのでアレかなと思いました。
また、rbrainの時代に思った致命的な点としては、ロボットの特性に関するものをclassにすると、
多重敬称できないので後々困ることがあります。
具体的には、特性(handの有無、min/maxの有無、足の有無...)は、いずれの組み合わせもありうるので、
単一継承だと実現できないのが難点です。

「upstreamでなんとか解決する/書き出しをなんとかする」
upstreamで解決するのが、個人的にはベストだと思います。
現状、(以下分け方がテキトーかもしれないです)

  • なんとかeuscolladaでモデルコンバート時にいいかんじにできないか
  • define-min-max-tableをjskeusにもっていき、:init-endingでよぶ
  • jointのinstance生成時に追加する

などを挙げていただいてます。

それでご提案いただいている、
「jointのinstance生成時に追加する」
が確かにいいかんじとは思ってきたので、例えば以下はいかがでしょうか。

robot.lには:name "RLEG_JOINT0"がかいてあるので、 make-min-max-table.lの中では、対応する関節名で:name "RLEG_JOINT0"を 検索して、そこをsedか何かで置き換えて、 :name "RLEG_JOINT0" :joint-min-max-target "RLEG_JOINT2" :joint-min-max-table xxxx`
をかきこむ。
ややeuscolladaの書き込み形式をhackする感があるのが難点ですが、
ユーザレベルでの新たなルールは必要なく、
:init-endingも必要なくなり、:define-min-max-tableもいらなくなります。

もしくは、upstream感をだして、euscolladaの引数で--join-min-max-tableが与えられたらこれをかくことにして

  • euscolladaでjoint-min-max tableのないロボットを生成する
  • make-min-max-tableする
  • 生成されたものを引数に与えてもう一回euscolladaを実行する

などもありうるのかと思います。

ただ、upstreamもっていく場合のupstreamは、euscolladaというよりは、jskeusなのかな、と思います。
たまたま今つかってるろぼっとがeuscolladaであって、min/maxの生成方、利用法もjskeusの機能であり、
euscolladaに依存したものではないので。
また、jskeusにmin-max tableがある以上、euscolladaでないロボットで同じ問題が再発するためです。

@k-okada
Copy link
Copy Markdown
Member

k-okada commented Apr 19, 2017 via email

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 21, 2017

すいません,遅くなりました.

jskeusにいれるには、hrp2/jaxon
以外のロボットでどういう使われ方になっているか、検討しましょう.例えばmin-max-tableはどうやって入れることが想定されているか.jskeusの段階だと、人手でつくるか、make-min-max-table
で作ることが想定されているから、init-endingで:default-min-max-tableする必要はないように見えます.:default-min-max-tableを使ったらいいjskeusのロボットの例をうまく作る必要があると思います.

なるほど.
やはりjskeusにいれるのが良いとは思いつつ,どうやったらいいかんじに考えてはみてましたが,
確かにこの点がうまくなってないとjskeusにはいれづらいですね.
とくに,事前生成するか,インスタンス生成時につくるか,なども違いがありそうですね.先日のjskeusの方は前者しか考えられてなかったです.
ありがとうございます,だいぶスッキリしてきました.

PR2のカメラなどもそうですが,キャリブ済みカメラモデルや,もしかしたらreachability mapとかも
その系統かもしれないですが,基本となるロボットモデルに後付けしたり,
オフライン計算した結果を持たせるようなものは他にもありそうですね.

また,コメントいただいている

(unless (assoc :init-ending-org (send euscollada-robot :methods))
   (rplaca (assoc :init-ending (send euscollada-robot :methods))
:init-ending-org))

  (defmethod euscollada-robot
    (:init-ending
    (&rest args)
    ()
     (prog1
       (send* self :init-ending-org args)
      (if (find-method self :define-min-max-table)
        (send self :define-min-max-table))))

の方法がかなりよさそうに思いました.
取り急ぎの解法である#508, #509と,系統は同じでありつつも,いくつかの問題点も#508, #509
より改善されてますね.

こちらでPRを更新してみたいと思います

@snozawa
Copy link
Copy Markdown
Collaborator Author

snozawa commented Apr 24, 2017

#511 (comment)
のeuscollada-robotの:init-endig-orgをつくる方法を試してみました。

  • min/max tableをもつロボットは、robot.lに元々の:init-ending, :init-ending-orgを使う:init-endingの2種類がかかれる
  • min/max tableを持たないロボットは、robot.lに元々の:init-endingのみもつ

なので、別なロボットのrobot.lがloadされると問題がおこりえることが分かりました。
具体的には、

  1. min/maxがあるorbot.lの次にないrobot.lをloadする
  2. HRP2JSKNTなどは、ハンドもロボットとしているので、HRP2JSKNTは単体をつくるだけでも1に該当する

となって、:init-ending-orgを使うものが上書きされるようでした。

itohdak pushed a commit to itohdak/rtmros_tutorials that referenced this pull request Jan 10, 2019
[jsk_hrp2_ros_bridge/jsk_hrp2_ros_bridge.launch] Add voice_text.launc…
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