Merged
Conversation
takahashitom
requested changes
May 23, 2025
Collaborator
takahashitom
left a comment
takahashitom
left a comment
There was a problem hiding this comment.
とりあえずは、いくつかです。
実際に実機でビルドできるかは試せてないので、すぐ試そうと思います
takahashitom
approved these changes
May 23, 2025
Collaborator
takahashitom
left a comment
takahashitom
left a comment
There was a problem hiding this comment.
問題なさそうです。
フォースセンサなどの他の機能のAPIは、必要になった時に随時追加していく形で行けば、大丈夫だと思います。
おつかれさまでした。
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a Robot class to manage hardware resources, a Motion base class for defining robot behaviors, updates build scripts to include the new modules, and adds basic tests for the robot getters.
- Added
Robot.h/cppandMotion.h/cppundermodulesandmodules/motions - Updated
Makefile.incandCMakeLists.txtto include the newmotionsdirectory - Created
RobotTest.cppfor validating getter methods and adjustedcompose.yaml
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/RobotTest.cpp | Added tests for motor and camera getters; commented out ColorSensor tests |
| modules/motions/Motion.h | Defined Motion abstract base class |
| modules/motions/Motion.cpp | Implemented Motion constructor |
| modules/Robot.h | Declared Robot resource manager class |
| modules/Robot.cpp | Implemented Robot constructor and getters |
| modules/EtRobocon2025.h | Added Robot static member and updated author tag |
| modules/EtRobocon2025.cpp | Defined EtRobocon2025::robot and updated author |
| compose.yaml | Removed Docker Compose version declaration |
| Makefile.inc | Included modules/motions and spike API headers |
| CMakeLists.txt | Added motions sources and include directory |
Comments suppressed due to low confidence (2)
compose.yaml:1
- The
versionfield was removed; Docker Compose may require it for compatibility. Please re-addversion: '3.8'at the top of the file.
services:
tests/RobotTest.cpp:33
- ColorSensor tests are currently commented out. Remember to re-enable and implement these tests once ColorSensor support is added to maintain coverage.
// TEST(RobotTest, GetColorSensorInstanceReturnsReference)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
チェックリスト
変更点
modules\直下に以下のファイルを追加Robot.hRobot.cppmodules\motions\直下に以下のファイルを追加Motion.hMotion.cppMakefile.incにインクルードを追加motionsを作成したため，Makefile.incとCMakeLists.txtに追加メモ
RobotクラスのメンバにColorSensorインスタンスも持たせようと思ったのですが，
現在モックが無くテストができないため，コメントアウトしています．
ただ，今後必ず公式APIは使うため，インクルードリストには追加しました．
添付資料
Notion