Conversation
[eye.hpp] restore eye display function
Fixed for PR
|
Thank you for your review. I apologize for the delay in addressing your feedback. Based on your suggestions, I have made the following changes. Please review them at your convenience. 1. Removed Unnecessary Functions
2. Split Functions to Simplify Responsibilities
3. Restructured File Organization
Please let me know if there are any further concerns or improvements needed. |
sktometometo
left a comment
There was a problem hiding this comment.
Basically, your updates look good to me except some points below.
| constexpr EyeAsset sad_eye = {"/normal_kxr_eyeball.jpg", "/normal_kxr_iris.jpg", "/normal_kxr_pupil.png", "/normal_kxr_reflex.png", "/normal_kxr_upperlid_rdown.jpg"}; | ||
| constexpr EyeAsset happy_eye = {"/normal_kxr_eyeball.jpg", "/white.jpg", "/white.png", "/white.png", "/normal_kxr_upperlid_happy.jpg"}; | ||
|
|
||
| // EDIT HERE: eye_status(ROSトピックのメッセージ)と それぞれのEyeAssetの対応付け |
There was a problem hiding this comment.
Basically, you should split definition and variable declaration because if you include the defintion, you will also declare the variable even when you don't want to use it.
There was a problem hiding this comment.
I recommend the structure like below.
eye_assets.hpp: definition of EyeAsseteye.hpp: defintion of EyeManager with constructor which has eye_assets_map and upperlid_position_map as arguments.emotion.hpp: definition of EmotionManagermain.cpp: declaration of eye_assets_map, upperlid_position_map and declaration of Eye instance with those variables.
|
金曜日にやっていた作業のひと段落まで行ったバージョンを https://github.com/k-okada/eye-display/commits/merge-all-robots-eyes-shinjo-merge/ に置きました。
@sawada10 コード見るタイミングが来たら、一緒に続きをしましょう。 |
この件ですが、 https://github.com/sktometometo/elevator_operation/blob/c7d86af1c81d319219b78c51fd465a85a55cb67a/sketchbooks/elevator_status_core2/src/main.cpp#L75-L79 みたいに loop で毎周期 nh.connected() チェックしてROSの初期化走らせ直して挙げると マイコンつけっぱなしでも rosserial_node.py を立ち上げたタイミングで getParamしてくれます。 void loop()
{
while (not nh.connected()) {
delay(100);
nh.spinOnce();
nh.getParam(...);
}
...
}みたいな感じです。 |
2cf7ec5 to
29189ea
Compare
|
3bdb2fb で、動いたがする
|
bc1a74d 一通りアニメーションができた気がする。
(疑問メモ:
|
|
0af478c 8b1cf88 normal_kxrの目(sample)に加えて、柏木さんの目に対応しました。
@a-ichikura @ayaha-n 0. kashiwagiをサポートできたレポジトリを持ってくる 1. 画像追加する ※pupilとかについては、fillCircle関数で描画していたと思うのですが、今のバージョンでは描画関数を使わないで、全て画像を用いることにしました。なので、fillCircleなどで書いていた図形も画像として置いていただきたいです。
2. launchファイルを書く 3. テストする で書き込んで、 で立ち上げて、 で目を変えて動作しているか確認お願いします。 ここまで試して良さそうでしたら、 多分、説明不足 & 何かしら起こるような気がするのでそのときは聞いてください |
|
とりあえず EyeStatus.msgのSHINE1が2つ定義されていたので修正しました。 しかし、 というエラーになって、 して EyeStatus.hの中身も確認しました。 エラーは変わらない状態です… |
|
上のEyeStatus.hは だったので で、参照先のEyeStatus.hに定義がなかったです。 というわけでとりあえず修正して |
|
↑ 名前を短く(kuromitsu → krmt)にしました。 そしてlaunchまではできましたが、挙動が変(normalの目が表示されたり、reflexが表示されていなかったり…でよくわからなくなってしまったので(もしかしてEyeStatus.hを編集してしまったから?)これ以上いじるのはやめておきます…すみません。 |
|
@sawada10 @a-ichikura 反応おそくなってすいません。 ドキュメント作っていなくて申し訳ないんですが、今のrosserial_version スケッチで使っている
|
|
あと、 一度マージしちゃってもいいかな |
|
@a-ichikura のくろみつの目の修正がなければ、 |
|
ごめんなさいおそくなりました |
50cf7f4
|
#29 変更がmasterに反映されたのでこちらも自動でcloseされてしまいました |
|
@sawada10 に確認したい内容
で,合っている? |
|
オフラインで先ほど話しましたが、一応コメントを残します。
この二つはどちらもyesです
これはNoで、8個?感情があると思います (@ayaha-n -> 市倉さんにやっていただいて、ちょっと丸投げで書いてもらうのは良くない気がしたので、今後時間あるタイミングで一緒に書きましょう) |
|
@sawada10 #32 で少し直しました.大きな違いは, またロボット毎の設定は これで, @ayaha-n @heissereal のロボットもyamlフィアルを追加すればよいはずです. @iory I2Cのコードだけど, @sktometometo main.cpp 読みやすくしたつもりですがどうでしょう.. |
|
@k-okada ぱっと見いいかんじだと思います、2点気になるところがあります。 今現在の仕様だと、必要な画像データは全てSPIFFSに書き込まれている前提になっていると思うのですが、SPIFFSのサイズは制限があるので種類が増えていったときに全部は入れられないのでプロジェクト的にまとめられた方がいいかなという思いがあります。依然作った あと、(結局simple_versionでも完全には消せていないのですが)プリプロセッサディレクティブをメインロジック部(特に あとは本質的ではないのですが細かいところで...
|
|
@k-okada 岡田先生 |
|
@sawada10 僕の環境で |
|
@sktometometo 返信遅くなりすみません
|
|
ありがとうございます、こうなるとそもそもSPIFFSに全部ファイル乗り切っているのかが気になりますね.... |
|
メモ:
|
|
RAMの使用量に差がある? |
|
今日、@ayaha-n さんと一緒にdendenの目を追加しようとしていました。
以下のようなエラーが出ます。 baud rateが原因かと思ったのですが、前回動いたときと今回のエラーが出たとき両方とも 次に、USBケーブルのせいかと思い、M5stack純正のUSBケーブルに変更しましたが、同様のエラーが出ました。 @sktometometo さんのお手元でも同様の状況でしょうか? |
see https://github.com/sktometometo/eye-display as original development source tree and sktometometo/eye-display#27 for discussion
see https://github.com/sktometometo/eye-display as original development source tree and sktometometo/eye-display#27 for discussion
see https://github.com/sktometometo/eye-display as original development source tree and sktometometo/eye-display#27 for discussion
|
@sawada10 これは信じられないほど申し訳ないんだけど, ということで,ついでなので,jsk_3rdparty に入れて, jsk_3rdparty は色々ファイルがあって見づらい場合もありますが,新し目のgitをつかうと, |
|
@sawada10 さんと電伝虫の目を表示できることを確認しました. ただ,目の表示方向に関する不思議な現象を観測したのでご報告します.
|
は,https://github.com/ayaha-n/jsk_3rdparty/pull/2 でなおったとおもいます.`pio run -e stamps3-ros
https://gist.github.com/k-okada/3e8da1f2d4805f4a4294328ccd9706ff#file-gistfile1-txt-L104-L109 みたいにsetup eye asset と表示して,direction も教えてくれるようにしました.ただ,時々 みたいに,subscriberはセットされているけど,rosparam は読んでい内容に見えることがあります. 次に発生したら,同じようにログを貼り付けてみて下さい. |
|
ありがとうございます.
こちらはdirection引数を付けずにlaunchしていましたが,kashiwagiでdefault=1なのに,一回目のlaunchでは0の方向になってしまっていました,という報告でした(今は改善しました). |
変更点
感情ごとに作成していた関数の統一
Eye)で行なっていた。ready_for_<name of emotion>_eye)」<name of emotion>)」を作成していた
EyeManager(目の画像準備、視線方向、まぶた位置の決定),感情表現を司るクラスEmotionManager(目の画像セットの切り替え、まぶたのアニメーション)set_emotion内で、eye_statusに基づいて、EyeManager内の機能を呼び出して、画像セットの変更やまぶたのアニメーション制御を行なうようにした。