#KL25-18 カラーセンサから値を取得する by HaruArima08 · Pull Request #5 · KatLab-MiyazakiUniv/etrobocon2025

HaruArima08 · 2025-05-19T13:02:23Z

チェックリスト

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

変更点

カラーセンサを制御するColorSensorクラスの作成
ColorSensorクラスのダミーファイル作成
ColorSensorクラスの値取得をする関数のテストを行う、ColorMeasureTest.cppの追加

notion-workspace · 2025-05-19T13:02:27Z

カラーセンサから値を取得する

takuchi17

ちょっと時間的な理由で表面上しか見れてません。後日ちゃんと見ます😣

takuchi17 · 2025-05-19T14:44:07Z

CMakeLists.txt

+  ${PROJECT_TEST_DIR}/dummy
  ${PROJECT_TEST_DIR}/helpers
-)
+  )


この括弧は一個下げた方が気持ちいい

modules/API/ColorSensor.h

takuchi17 · 2025-05-19T14:48:35Z

modules/API/ColorSensor.h

+  /**
+   * カラーセンサのRGB値を取得する
+   * @param  値を設定するRGB構造体、各色10ビット
+   * @return -
+   */
+  void getRGB(RGB& rgb) const


細かいけど説明に引数名がないかな。

Suggested change

/**

* カラーセンサのRGB値を取得する

* @param 値を設定するRGB構造体、各色10ビット

* @return -

*/

void getRGB(RGB& rgb) const

/**

* カラーセンサのRGB値を取得する

* @param rgb 値を設定するRGB構造体、各色10ビット

* @return -

*/

void getRGB(RGB& rgb) const

takuchi17 · 2025-05-19T14:49:54Z

modules/API/ColorSensor.h

+  /**
+   * カラーセンサで色を測定する
+   * @param surface trueならば表面の色から、falseならば他の光源の色を検出する
+   * @return 色（hsvによる表現）
+   */
+  void getColor(HSV& hsv, bool surface = true) const


引数の説明にhsvがない

Suggested change

/**

* カラーセンサで色を測定する

* @param surface trueならば表面の色から、falseならば他の光源の色を検出する

* @return 色（hsvによる表現）

*/

void getColor(HSV& hsv, bool surface = true) const

/**

* カラーセンサで色を測定する

* @param surface trueならば表面の色から、falseならば他の光源の色を検出する

* @param hsv なんちゃら

* @return 色（hsvによる表現）

*/

void getColor(HSV& hsv, bool surface = true) const

takuchi17 · 2025-05-19T14:52:13Z

modules/API/ColorSensor.h

+  /**
+   * カラーセンサで色を測定する（近似なし）
+   * @param surface trueならば表面の色から、falseならば他の光源の色を検出する
+   * @return 色（hsvによる表現）
+   */


ここも引数hsvの説明がないな

takuchi17 · 2025-05-19T14:55:57Z

modules/API/Port.h

+#ifndef PORT_H_
+#define PORT_H_


Hの後に_って他のクラスつけてたっけ？他のインクルードガードをPORT_Hにすることは多分ないから大丈夫だと思うけど、一応確認。

ダミーの方は PORT_H になってるからミス？
多分ビルド対象外になってたはずだから、PORT_H でも大丈夫なはず

同じ名前が重複するとインクルードガード効かずビルドが通らなかったので、クラスの方とダミーのほうで分けて書いてました。
分かりやすくクラスのほうは _ 外して、ダミーは先頭に DUMMY_ を足してみました。

takuchi17 · 2025-05-19T14:56:58Z

tests/ColorMeasureTest.cpp

@@ -0,0 +1,126 @@
+/**
+ * @file ColorMeasureTest.cpp


ColorSensor.cppじゃないのは何で？理由があるならいいけど、ないならソースファイルと統一してもらいたい🙏

Suggested change

* @file ColorMeasureTest.cpp

* @file ColorSensorTest.cpp

takuchi17 · 2025-05-19T14:58:29Z

tests/dummy/spike/pup/colorsensor.h

@@ -0,0 +1,76 @@
+/**
+ * @file colorSensor.h


Suggested change

* @file colorSensor.h

* @file colorsensor.h

tests/dummy/spike/pup/colorsensor.h

aridome222

少し気になったところの回答と、細かい修正お願いします。

modules/API/ColorSensor.h

tests/ColorMeasureTest.cpp

takuchi17

追加で一つだけかな．setDetectableColors関数のところだけ確認お願いします．

modules/API/ColorSensor.h

tests/ColorMeasureTest.cpp

refactor:ファイル名をソースファイルと統一するためにColorSensorTest.cppに変更した

molpui0726

いっこだけ？
他プルリクと実装ファイルが被ってるところがあるから、マージの順番とかは考えないといけない。

modules/API/ColorSensor.h

molpui0726 · 2025-05-20T15:28:54Z

modules/API/Port.h

+#ifndef PORT_H_
+#define PORT_H_


ダミーの方は PORT_H になってるからミス？
多分ビルド対象外になってたはずだから、PORT_H でも大丈夫なはず

takuchi17

良さそうです。テストも充実してると思います。中身も理解してるのが伝わります。自分は実際に見てないからわからないけど、次のタスクで多分ColorJudgeとかやる（誰かが）と思うから、その時に適宜調整すればいいと思う。
お疲れありがとう。

takuchi17 · 2025-05-20T15:52:29Z

CMakeLists.txt

+  ${PROJECT_TEST_DIR}/dummy
  ${PROJECT_TEST_DIR}/helpers
-)
+    )


すまん、このスペースだけ気になった。どうせコンフリクト起きるからその時してもいいんだけどね。

takuchi17 · 2025-05-20T16:17:32Z

tests/ColorSensorTest.cpp

+namespace etrobocon2025_test {
+
+  // RGB値取得テスト
+  TEST(RgbColorTest, getRGB)


気が回らなかったけど、テスト名はアッパーキャメルでお願いします。一応コーディング規約そうなってます🙏他のも。信じてapproveはそのままにしときます。

Suggested change

TEST(RgbColorTest, getRGB)

TEST(RgbColorTest, GetRGB)

衝撃の事実、これは、、、去年の人たちが悪いね！

大変申し訳ございませんでした。

実は僕がやりました。公式やらどこ見てもアッパキャメルだったので、ここを区切りにしたかったです。個人的にはアッパーの方が大文字でぱっと見で読みやすいのもある。ちなみにこのリポジトリのwikiを作った時にすでに変えてました。あまり個人の意見を出したくはないのだけど、今年度は今年度なのでね。

aridome222

修正してくれたのでapprove出します！お疲れ様！

takahashitom

いい感じだと思います。
めんどくさかったら直さなくてもいいです。
どうせコンフリクトするところなので。

実際には動作してるのかしてないのかよくわからない関数もあったけど、このタスクでは、RGB, HSV, 反射光の値が取得できていれば十分だと思うので問題ないと思います。
動作不明関数たちも、もし使えたら割と有用そうなのもあるので、引き続き調査していきましょう。
あとは、HSV取得の片方が動作不良っぽい話とかRGBの取得が 10bit だとかそういう調査の結果わかったことは、Notion の方にまとめておいてもらえると助かります。

お疲れさまでした。

takahashitom · 2025-05-21T02:16:07Z

Makefile.inc

+
+
+


どうせコンフリクトするところだから直さなくてもいい気がするけど、
ここはいらないかな

Suggested change

molpui0726

僕が出した分がなおってたのでLGTMだします！
おつかれさまんさー

HaruArima08 added 3 commits May 19, 2025 06:40

add:カラーセンサクラスの実装と値取得のダミーとテストファイル作成

1aa551d

refactor:細かなコメントと値の修正

9940a39

refactor:CMakeList.txt、インクルードガード、テスト名をアッパーキャメルに修正

3560b27

takuchi17 requested changes May 19, 2025

View reviewed changes

aridome222 assigned HaruArima08 and aridome222 May 19, 2025

aridome222 requested changes May 19, 2025

View reviewed changes

takuchi17 requested changes May 20, 2025

View reviewed changes

modules/API/ColorSensor.h Show resolved Hide resolved

modules/API/ColorSensor.h Show resolved Hide resolved

tests/ColorMeasureTest.cpp Show resolved Hide resolved

HaruArima08 added 2 commits May 20, 2025 23:49

Rename ColorMeasureTest.cpp to ColorSensorTest.cpp

ac4f364

refactor:ファイル名をソースファイルと統一するためにColorSensorTest.cppに変更した

update:ColorMeasureTest.cppをソースファイルと統一するためにColorSensorTest.cppに変更した

d9fdb39

molpui0726 requested changes May 20, 2025

View reviewed changes

takuchi17 approved these changes May 20, 2025

View reviewed changes

takuchi17 reviewed May 20, 2025

View reviewed changes

aridome222 approved these changes May 20, 2025

View reviewed changes

takahashitom approved these changes May 21, 2025

View reviewed changes

molpui0726 approved these changes May 21, 2025

View reviewed changes

takahashitom closed this May 22, 2025

		#ifndef PORT_H_
		#define PORT_H_

		#ifndef PORT_H_
		#define PORT_H_

Conversation

HaruArima08 commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

チェックリスト

変更点

Uh oh!

notion-workspace bot commented May 19, 2025

Uh oh!

takuchi17 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aridome222 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

takuchi17 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

molpui0726 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

takuchi17 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

takuchi17 May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aridome222 left a comment

Choose a reason for hiding this comment

Uh oh!

takahashitom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

molpui0726 left a comment

Choose a reason for hiding this comment

HaruArima08 commented May 19, 2025 •

edited

Loading

takuchi17 left a comment •

edited

Loading

takuchi17 May 20, 2025 •

edited

Loading