Skip to content

Conversation

@ShotaAk
Copy link
Contributor

@ShotaAk ShotaAk commented Mar 2, 2025

consai_game パッケージを追加します。

現時点ではPlayファイルの読み込みと、play_node、wolrd_model_provider_nodeの起動のみに対応しています。

既存のconsaiの動作には影響しません。

使用方法

$ cd /path/to/consai_game
$ ros2 run consai_game main.py --playbook config/playbook

Loading playbook from config/playbook/halt_blank.yaml
Loading playbook from config/playbook/stop_blank.yml
Main update, process ID: 327819, CPU: 10
[INFO] [1740903633.467007250] [play_node]: Play update, process ID: 327819, CPU: 10
[INFO] [1740903633.467724011] [world_model_provider_node]: WorldModelProvider update, process ID: 327819, CPU: 10
[INFO] [1740903633.468261754] [play_node]: Selected play: halt_blank
[INFO] [1740903633.546697317] [play_node]: Play update, process ID: 327819, CPU: 10
[INFO] [1740903633.549544920] [world_model_provider_node]: WorldModelProvider update, process ID: 327819, CPU: 10

@github-actions
Copy link

github-actions bot commented Mar 2, 2025

test_scenario_kickoff.py failed. Failure logs: https://github.com/SSL-Roots/consai_ros2/actions/runs/13613590408/artifacts/2677011095

@ShotaAk ShotaAk requested a review from tilt-silvie March 2, 2025 08:23
@ShotaAk ShotaAk marked this pull request as ready for review March 2, 2025 08:23
@github-actions
Copy link

github-actions bot commented Mar 2, 2025

test_scenario_obstacle_avoidance.py failed. Failure logs: https://github.com/SSL-Roots/consai_ros2/actions/runs/13613621937/artifacts/2677017554

@tilt-silvie
Copy link
Contributor

@ShotaAk そもそも的なところになってしまうんだけど、playはなんでyamlで書こうという話になったんだっけ?

@ShotaAk
Copy link
Contributor Author

ShotaAk commented Mar 2, 2025

@tilt-silvie 比較的使い慣れている形式だからですね。真面目に比較検討はしてないです

@tilt-silvie
Copy link
Contributor

@ShotaAk PythonコードではなくYAMLなりJSONを使う理由はあるかな?
静的解析でPython側との関連が読めなかったり、そもそも関連付けを辞書で管理する必要があったりして結構面倒事が増えそうな印象

Copy link
Contributor

@tilt-silvie tilt-silvie left a comment

Choose a reason for hiding this comment

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

良い

  • 全体的に型がつけてあって素晴らしい
  • dataclassいいね

気になる

  • playbookは複数作れるようにしたい

    • 対戦相手ごとに戦略を切り替えられるようにしたいので
    • ただし、play自体は使い回せるようにしたい
    • そのために、playbookディレクトリの下にplayをいれるのではなく、playとplaybookを独立して管理できるようにしたほうが良さそう
    • playbookとplayが多対多の参照関係になるイメージ
  • playの処理をROSノードとして実装してるけど、playからのtacticsの呼び出しはserviceかactionとして実装するイメージ?

@ShotaAk
Copy link
Contributor Author

ShotaAk commented Mar 3, 2025

@ShotaAk PythonコードではなくYAMLなりJSONを使う理由はあるかな?

静的解析でPython側との関連が読めなかったり、そもそも関連付けを辞書で管理する必要があったりして結構面倒事が増えそうな印象

現段階でyamlにこだわる理由はないですね、pythonに統一しちゃいます 👍

@ShotaAk
Copy link
Contributor Author

ShotaAk commented Mar 3, 2025

@tilt-silvie playbookを複数にすること了解です!
playbook_for_〇〇.pyみたいなのをplaybooksディレクトリに複数用意する感じですね。

tacticついては実装悩み中です

@ShotaAk ShotaAk requested a review from tilt-silvie March 4, 2025 14:39
@ShotaAk
Copy link
Contributor Author

ShotaAk commented Mar 4, 2025

@tilt-silvie yamlファイル使うのやめました。
playbookを複数作れるようにしました。

確認お願いします 🙏

@github-actions
Copy link

github-actions bot commented Mar 4, 2025

test_scenario_ball_placement.py failed. Failure logs: https://github.com/SSL-Roots/consai_ros2/actions/runs/13656154764/artifacts/2689337645

Copy link
Contributor

@tilt-silvie tilt-silvie left a comment

Choose a reason for hiding this comment

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

@ShotaAk 修正あざす!いい形になってきたと思います!
PlayをFactoryで生成するのいいね。 自分はPlayのインタフェースをABCクラスとして作って、具体的なPlayはそれを継承して…みたいな感じで実装するのを想像してたけど、FactoryでPlayのインスタンスを作ってしまう方がシンプルで良い気がします。

あとより良くするために、骨組み(フレームワークといったほうが良いかも)部分と肉部分を明確にディレクトリ分けると良いと思った。
今の consai_game/consai_game の中にある骨組み部分が consai_game/consai_game/core に入って、肉部分が consai_game/consai_game/play とかに入るイメージかな
明確にディレクトリを分けておくことで、骨実装マンと肉実装マンの責任分界点が明確になって作業しやすくなると思う。
あとはCONSAIを第三者に使ってもらうときも、肉だけ実装してくればいいよーという方向に持っていきやすくなるし

@ShotaAk
Copy link
Contributor Author

ShotaAk commented Mar 5, 2025

@tilt-silvie レビューありがとう!
肉と骨を分けること了解👌
coreにはtacticとかworldmodelなどの骨も入る、という方針でディレクトリ整えてみます

@ShotaAk ShotaAk requested a review from tilt-silvie March 5, 2025 22:34
@ShotaAk
Copy link
Contributor Author

ShotaAk commented Mar 5, 2025

@tilt-silvie ディレクトリ分けました〜

world_modelに関しては中身を実装してから配置します。

@ShotaAk
Copy link
Contributor Author

ShotaAk commented Mar 5, 2025

PlayNode内にlibrarianを追加してます。
playbookの登録をしてから、arg_parserに引数を追加する都合上、クラス変数&メソッドにしてます。。。
(良い実装方法が思いつかず)

if __name__ == '__main__':

    PlayNode.register_playbook('default', playbook_default.plays)
    PlayNode.register_playbook('hoge', playbook_default.plays)
    PlayNode.register_playbook('fuga', playbook_default.plays)

    arg_parser = argparse.ArgumentParser()
    PlayNode.add_arguments(arg_parser)

    args, other_args = arg_parser.parse_known_args()
    rclpy.init(args=other_args)

    play_node = PlayNode(update_hz=10, book_name=args.playbook)

@github-actions
Copy link

github-actions bot commented Mar 5, 2025

test_scenario_kickoff.py failed. Failure logs: https://github.com/SSL-Roots/consai_ros2/actions/runs/13686986198/artifacts/2699833030

@tilt-silvie
Copy link
Contributor

@ShotaAk 分割ナイス!依存関係が整理されてかなり良くなったと思います

librarianについては、そもそもlibrarianを消すってのはどう?
ノードの引数としてplaybookファイルのパスを渡すようにして、それを importlib 使って動的にロードするのはどうかな
そうしとくと、そもそもplaybookを辞書として持つ必要がなくなるはず

https://zenn.dev/giba/articles/7ed4c5a8f07728

@ShotaAk
Copy link
Contributor Author

ShotaAk commented Mar 7, 2025

@tilt-silvie 書き換えました〜これでどうでしょうか?

@github-actions
Copy link

github-actions bot commented Mar 7, 2025

test_scenario_stop.py failed. Failure logs: https://github.com/SSL-Roots/consai_ros2/actions/runs/13720803534/artifacts/2710749317

@tilt-silvie
Copy link
Contributor

@ShotaAk 神!マージ〜〜

@tilt-silvie tilt-silvie merged commit d480def into main Mar 7, 2025
12 of 13 checks passed
@tilt-silvie tilt-silvie deleted the feature/add_consai_game branch March 7, 2025 12:55
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