Skip to content

#KL25-19 USBカメラから画像を取得する#4

Merged
takuchi17 merged 8 commits intomainfrom
ticket-KL25-19
May 22, 2025
Merged

#KL25-19 USBカメラから画像を取得する#4
takuchi17 merged 8 commits intomainfrom
ticket-KL25-19

Conversation

@Hara1274
Copy link
Copy Markdown
Contributor

@Hara1274 Hara1274 commented May 18, 2025

チェックリスト

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

変更点

・USBカメラを制御するCameraCaptureクラスを追加。
・CameraCaptureクラス用のディレクトリをmodulesディレクトリの直下に追加
・opencv4ライブラリとCameraCaptureクラスを使用できるように、Makefile.incとCMakeLists.txtを修正

添付資料

https://www.notion.so/uom-katlab/USB-1f4dd5b1cc18806a9c69e7a015ee8391?pvs=4

@notion-workspace
Copy link
Copy Markdown

@Hara1274 Hara1274 changed the title Ticket kl25 19 KL25-19 USBカメラから画像を取得する May 18, 2025
@Hara1274 Hara1274 requested a review from a team May 18, 2025 11:54
Copy link
Copy Markdown
Contributor

@molpui0726 molpui0726 left a comment

Choose a reason for hiding this comment

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

細かいところで色々あるけど、動作してるのを見たからさほど問題に感じてない。

ただ、ところどころ boolean で◯◯を実行する系の関数が書かれてて、関数の中でエラー処理した上で false 返してるところがあるけど、これって必要なの?
呼び出し元でもう一回確認したいとか?

Copy link
Copy Markdown
Collaborator

@takuchi17 takuchi17 left a comment

Choose a reason for hiding this comment

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

同時にレビュー始めちゃったから木村さんと被ってるかもしれない.
ロジックがおかしいところは一か所あってそれ以外はリファクタリング的な感じです.

@takuchi17 takuchi17 changed the title KL25-19 USBカメラから画像を取得する #KL25-19 USBカメラから画像を取得する May 18, 2025
@Hara1274
Copy link
Copy Markdown
Contributor Author

了解です。修正します🙇

Copy link
Copy Markdown
Collaborator

@takuchi17 takuchi17 left a comment

Choose a reason for hiding this comment

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

いくつか対応してもらってないところがあります。対応しなくてもいいんじゃないかなというところがあれば、気軽に意見を書いてもらえると助かる🙏

@Hara1274
Copy link
Copy Markdown
Contributor Author

見落としてた部分を修正しました🙇bool型の関数はとりあえずそのままにしてます。

Copy link
Copy Markdown
Contributor

@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.

見た感じ大丈夫そう!よく実装できてる!
細かいところだけ、修正お願いします。

Copy link
Copy Markdown
Collaborator

@takahashitom takahashitom left a comment

Choose a reason for hiding this comment

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

コメントだけです。おおきな修正はないかな。

テストに関しては、画像取得系をモックで行うのはさすがにコストが高いと思うので、実機で確認できたで問題ないと思います。

エラー出力を使っているのは、すごくいいと思うのだけど、こういったRTOS上で動くプロジェクトにおいては出力系の処理って割と重くて、動作不良の原因になりがちらしいから多用には注意してください。
for 文とかで出力を高頻度で行うとかしなければ問題ないと思いますが、一応頭の片隅にとどめておいてください。

あと、タスクの完了条件に動画の取得・保存があったと思うのだけど、いつの間にかやらない方針になった感じ?
ログ確認とかで必要になると思うし、保存はしなくても動画取得自体は絶対に使うだろうし、やって欲しかったんだけど、getFrames()が動画みたいに使えるのかな。
でも、やっぱり動画形式のファイルで出力できた方がいい希ガス。
そこだけ確認したいかな。

全体的に問題なさそうだったんで問題ないと思います()。
大きな修正はないので、先にアプルーブ出しておきます。

Copy link
Copy Markdown
Collaborator

@takuchi17 takuchi17 left a comment

Choose a reason for hiding this comment

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

今ある分の対応してくれればOKだと思うので、approve出します。
テストが少ないのは仕方がない。

あと、録画がない理由なんだけど、プラレールにしろフィグにしろ、現状競技において録画をする必要がないと判断したからクラスのメンバ関数としては実装していないです。走行体目線の動画が必要だと分かったら随時実装します。自分の考えでは、画像一枚一枚の方が動画よりも画像処理とか機械学習に繋げるのが楽なのと、ログもgetFramesでの画像で十分ではないかと思っています。昨年は走行体目線の動画をログに使いたい場合もなかったですし。もちろん走行中ずっと保存するのは容量食うとは思いますが、大会以外は一区画ずつの走行が多くなると思うので問題ないとは思っています。あとから必要な部分だけ1フレームずつ保存しておけますし。

ただ、今分かったんだけど、opencvでリアルタイムでフレームをGUI表示できる関数があるらしいので、走行中の画像処理がうまくできているかの可視化はあったら便利かもしれないとは思ってる。今回のタスクでやる必要はないと思うんだけど。

Copy link
Copy Markdown
Contributor

@molpui0726 molpui0726 left a comment

Choose a reason for hiding this comment

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

いったん僕の方からは LGTM 出しとく
いいね!👍️
カメラ関連は実際に動かしてみないとわからないところあったり、そもそも使うのが先だったりするから動かなくてもしゃーないね。
いったんはなまるではある。
117606

Copy link
Copy Markdown
Contributor

@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.

修正お疲れ様!
LGTM

Copy link
Copy Markdown
Collaborator

@takuchi17 takuchi17 left a comment

Choose a reason for hiding this comment

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

追加で一個.これはした方がいいかも.かもというのは、一回しかインスタンス化しない予定だけど、次の走行に影響しないとも言いきれないから。アプルーブは外さないけど,対応お願いします.

@takuchi17 takuchi17 merged commit bc69083 into main May 22, 2025
6 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.

5 participants