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

close #KLI-80 コマンド(CSV)で動作を指定し、動作できる #19

Merged
merged 13 commits into from
Jun 26, 2024

Conversation

keiya121
Copy link
Collaborator

@keiya121 keiya121 commented Jun 23, 2024

チェックリスト

  • clang-format している
  • コーディング規約に準じている
  • チケットの完了条件を満たしている

変更点

etrobocon2024/module/下に,コマンドで動作を指定し、実行するための以下のファイルを追加.

  • AreaMaster.cpp
  • AreaMaster.h
  • MotionParser.cpp
  • MotionParser.h

etrobocon2024/module/common/下に,コマンドパースに必要な以下のファイルを追加.

  • StringOperator.cpp
  • StringOperator.h

etrobocon2024/下に, 実行したいコマンド一覧を記載したcsvファイルを配置するためのディレクトリである、datafilesを作成.

テストのため, datafiles下に仮エリア「AreaMaster」を攻略するための以下のcsvファイルを追加.

  • AreaMasterLeft.csv
  • AreaMasterRight.csv

testディレクトリで実行する際と、etrobocon2024.cppでファイルパスを指定する際で、カレントディレクトリが異なるため、パスの相違を避けるために、etrobocon2024/test/gtest/gtest_build.shの一部コメントアウトをはずした.

動作テスト

https://www.notion.so/uom-katlab/CSV-62877770d0404b27a7c2b366845cdaad?pvs=4

実験結果

コマンドで指定した動作を実機上で実行することが出来た.

備考

昨年まででコマンドを作成していた動作のうち、現在mainブランチにマージされているものは、自分の確認では、回頭動作のみでした。
そのため、一旦回頭動作のみのコマンドを作成し、その他のコマンドの処理部分は、コメントアウトしてあります。
(コマンド自体の定義は、コメントアウトしていません。)
そのため、実機テスト、gtestも回頭動作のみ、コマンドを作成して実行できるか試しました。

Copy link

@keiya121 keiya121 self-assigned this Jun 23, 2024
Copy link
Collaborator

@YKhm20020 YKhm20020 left a comment

Choose a reason for hiding this comment

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

Logger 関連のところで少しだけ🙏

module/AreaMaster.cpp Outdated Show resolved Hide resolved
module/AreaMaster.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@aridome222 aridome222 left a comment

Choose a reason for hiding this comment

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

細かい箇所1点だけ。

areaMaster.run();
string output = testing::internal::GetCapturedStdout(); // キャプチャ終了

// find("str")はstrが見つからない場合string::nposを返すß
Copy link
Collaborator

Choose a reason for hiding this comment

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

ß ← エスツェット文字っていうのが入ってたから、一応気になったので修正お願いします。

Suggested change
// find("str")はstrが見つからない場合string::nposを返すß
// find("str")はstrが見つからない場合string::nposを返す

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ご確認いただきありがとうございます。修正しました。

Copy link
Collaborator

@bizyutyu bizyutyu left a comment

Choose a reason for hiding this comment

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

@YKhm20020 と似た指摘もいくつかあります。意見も聞きたい部分も。

module/AreaMaster.cpp Outdated Show resolved Hide resolved
module/MotionParser.cpp Outdated Show resolved Hide resolved
module/MotionParser.cpp Outdated Show resolved Hide resolved
module/MotionParser.cpp Outdated Show resolved Hide resolved
module/MotionParser.cpp Show resolved Hide resolved
Copy link
Collaborator

@bizyutyu bizyutyu left a comment

Choose a reason for hiding this comment

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

実機での動作確認をする必要があるみたいだけど、先にLGTM出しておきます。
大変なタスクお疲れ様!
LGTM

Copy link

@negiuniv negiuniv left a comment

Choose a reason for hiding this comment

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

遅くなってすまんが頼むー

module/AreaMaster.cpp Show resolved Hide resolved
FILE* fp = fopen(commandFilePath, "r");
// ファイル読み込み失敗
if(fp == NULL) {
snprintf(buf, LARGE_BUF_SIZE, "%s file not open!\n", commandFilePath);

Choose a reason for hiding this comment

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

ここも

module/AreaMaster.cpp Show resolved Hide resolved
module/MotionParser.cpp Show resolved Hide resolved
Copy link
Collaborator

@YKhm20020 YKhm20020 left a comment

Choose a reason for hiding this comment

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

ごめんなさい遅くなりました。LGTMです🙇
119322

Copy link

@negiuniv negiuniv left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@aridome222 aridome222 left a comment

Choose a reason for hiding this comment

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

OKです。LGTM!

@YKhm20020 YKhm20020 merged commit fd6c864 into main Jun 26, 2024
2 checks passed
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.

None yet

5 participants