search

jsk-ros-pkg / jsk_robot

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

[pr2/fetch] Tweetのwarningの内容がaudible warningとtweet_client_warning.lとで被る #1562

Open iory opened 2 years ago

iory commented 2 years ago

PR2とfetchでテストしている機能としてdiagnosticsの内容を話すaudible_warningというノードがあります。 https://github.com/jsk-ros-pkg/jsk_common/pull/1607

このaudible warningでは、blacklistを与えることで話すwarningをフィルタリングしています。 https://github.com/knorth55/jsk_robot/blob/pr1040/jsk_pr2_robot/jsk_pr2_startup/jsk_pr2_warning/warning_blacklist.yaml

また、transformノードでtweetするtopicをpublishするようにしています。 https://github.com/knorth55/jsk_robot/blob/ceced0b80fa24e1cd73426f180703b9d47b30df5/jsk_pr2_robot/jsk_pr2_startup/pr2.launch#L215-L226

以前からあるtweet_client_warning.lでは簡易的な文字の対応をとることでdiagnosticsをtweetしていますが、warning_blacklist.yamlのようにパラメータで与えるものではなく、プログラムに直書きで指定する方法をとっています。 https://github.com/jsk-ros-pkg/jsk_robot/blob/25b9f6a44f7639624cf8859e80215e5020028fdc/jsk_robot_common/jsk_robot_startup/lifelog/tweet_client_warning.l#L8-L31

これらの内容が被るため、 https://twitter.com/pr2jsk/status/1560109698900103168 https://twitter.com/pr2jsk/status/1560109263707521024 これらのtweetのようにほぼ同じ内容のものをtweetしてしまうという問題があります。また、warning_blacklistでフィルタリングした内容をtweet_client_warning.lがtweetするということもあります。

k-okada commented 2 years ago

１）とにかくシステム構成図を書くことをおすすめします．何がどうなっているか，新しく作ったものは何がどう違うか，具体例としても，抽象的な説明としても伝わるものが良いです https://github.com/jsk-ros-pkg/jsk_robot/pull/1558#issuecomment-1209050229

２）新しい物を作った時は，古いものも包摂するインクルーシブなものがサステイナブルですよね．コードを増やして機能を増やすのは当たり前で，機能が増えるけどコードは同じ，というのが良いですよね．本当は，機能が増えてコードが減るのがサイコーです．

-- ◉ Kei Okada

2022年8月18日(木) 13:13 Iori Yanokura @.***>:

PR2とfetchでテストしている機能としてdiagnosticsの内容を話すaudible_warningというノードがあります。 jsk-ros-pkg/jsk_common#1607 https://github.com/jsk-ros-pkg/jsk_common/pull/1607

このaudible warningでは、blacklistを与えることで話すwarningをフィルタリングしています。

https://github.com/knorth55/jsk_robot/blob/pr1040/jsk_pr2_robot/jsk_pr2_startup/jsk_pr2_warning/warning_blacklist.yaml

また、transformノードでtweetするtopicをpublishするようにしています。

https://github.com/knorth55/jsk_robot/blob/ceced0b80fa24e1cd73426f180703b9d47b30df5/jsk_pr2_robot/jsk_pr2_startup/pr2.launch#L215-L226

以前からあるtweet_client_warning.lでは簡易的な文字の対応をとることでdiagnosticsをtweetしていますが、warning_blacklist.yamlのようにパラメータで与えるものではなく、プログラムに直書きで指定する方法をとっています。

https://github.com/jsk-ros-pkg/jsk_robot/blob/25b9f6a44f7639624cf8859e80215e5020028fdc/jsk_robot_common/jsk_robot_startup/lifelog/tweet_client_warning.l#L8-L31

これらの内容が被るため、 https://twitter.com/pr2jsk/status/1560109698900103168 https://twitter.com/pr2jsk/status/1560109263707521024

これらのtweetのようにほぼ同じ内容のものをtweetしてしまうという問題があります。また、warning_blacklistでフィルタリングした内容をtweet_client_warning.lがtweetするということもあります。

— Reply to this email directly, view it on GitHub https://github.com/jsk-ros-pkg/jsk_robot/issues/1562, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYNXBAVNQZ7E2NUJE6FNLVZWZYJANCNFSM563WJ4VQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

iory commented 2 years ago

ありがとうございます。 次のようなシステム構成図を作成し、処理の流れを可視化しました。 /tweet_client_warningと異なるのはtweetする内容をオレンジ色で囲ったParamによって指定できる点と、その指定するparamに正規表現を用いることができる点、フィルタする場合にdiagnostics内部のvalueも与えることができる点、/robotsoundをpublishするためロボットに喋らせることができるという点になります。

Screen Shot 2022-08-22 at 19 06 06

２）新しい物を作った時は，古いものも包摂するインクルーシブなものがサステイナブルですよね．コードを増やして機能を増やすのは当たり前で，機能が増えるけどコードは同じ，というのが良いですよね．本当は，機能が増えてコードが減るのがサイコーです．

こちらに関しては、上記の図を見ると古いものも包摂するインクルーシブなものがサステイナブルとなっています。 後者の本当は，機能が増えてコードが減るのがサイコーです．という点に関しては残念ながらそうはなっていないです。

k-okada commented 2 years ago

並列になっているのはインクルーシブではないので，tweet_cleint_warningの機能が取り込まれているなら，tweet_client_warningを消して，新しいものが使われるようにするのが良いでしょう．

少し，バックワードコンパチビリティを気にするなら，古い，tweet_client_warning は https://github.com/ros/xacro/blob/kinetic-devel/xacro.py みたいにして，audible_warning を呼ぶようにするんでしょう．

こういう時に参考になるのは，以下の図で，P3(HRP-1)があって，HRP2が作りたいときに一気に作るんではなくて，一回HRP-1Sを置くところですね． https://pc.watch.impress.co.jp/docs/2003/0227/hrp02.jpg

時間軸を考えると，あるロボットはaudible_warningを使って運用し，安定してリリースされた段階で，古いロボットも切り替えて，行く，という２段階の切り替えが可能ですね．

で，更に時間が巻き戻せるなら， tweet_client_warning で，subscribeするtopicをプログラマブルにする，などを対応したら，機能が増えてコードが減る，という状況になっていましたね．

そう考えると，最初はソースコード埋め込みのものをparam化しておけば，別のロボットでも使えるようになって，そのなかで正規表現などeusで対応が難しい時に中身を書き換えるという２段階ができますね．そう思うと，最初のparam化ではwhitelistを作って，その後，blackリストにするとか，

*
!/CPU/*

みたいな書き方になったのかなぁ，とも思いました．ツール作るのが目的ではなくて，開発を前に進めるのが目的なので，こういうことはしなくても良いと思うけど．

-- ◉ Kei Okada

2022年8月22日(月) 19:07 Iori Yanokura @.***>:

ありがとうございます。 次のようなシステム構成図を作成し、処理の流れを可視化しました。 /tweet_client_warning と異なるのはtweetする内容をオレンジ色で囲ったParamによって指定できる点と、その指定するparamに正規表現を用いることができる点、フィルタする場合にdiagnostics内部のvalueも与えることができる点、 /robotsoundをpublishするためロボットに喋らせることができるという点になります。 [image: Screen Shot 2022-08-22 at 19 06 06] https://user-images.githubusercontent.com/4690682/185896079-b1a0b2f6-76a3-4349-9faa-42c94ceac64a.png

２）新しい物を作った時は，古いものも包摂するインクルーシブなものがサステイナブルですよね．コードを増やして機能を増やすのは当たり前で，機能が増えるけどコードは同じ，というのが良いですよね．本当は，機能が増えてコードが減るのがサイコーです．

こちらに関しては、上記の図を見ると古いものも包摂するインクルーシブなものがサステイナブルとなっています。 後者の本当は，機能が増えてコードが減るのがサイコーです．という点に関しては残念ながらそうはなっていないです。

— Reply to this email directly, view it on GitHub https://github.com/jsk-ros-pkg/jsk_robot/issues/1562#issuecomment-1222136431, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYNXCJY6YRYBHRKS7K3TTV2NGLBANCNFSM563WJ4VQ . You are receiving this because you commented.Message ID: @.***>

knorth55 commented 2 years ago

メモ：

audible warningの内容を人間だけじゃなくて，ロボットが聞いて作業を助けてくれるとかなるといいなと思いました． PR2が作業を行い始めるが，目の前に物体があって動けないなどのaudible_warningがでて，それに合わせてほかのロボットが助けてくれるような．

tkmtnt7000 commented 2 years ago

メモ：

audible warningの内容を人間だけじゃなくて，ロボットが聞いて作業を助けてくれるとかなるといいなと思いました． PR2が作業を行い始めるが，目の前に物体があって動けないなどのaudible_warningがでて，それに合わせてほかのロボットが助けてくれるような．

僕の卒論の取り組みをうまく拡張するとできそう、という話になりそうだなと思いました