hugo/feature/Add ShapeRecognition activity #1015
Merged
Conversation
This file contains 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
Contributor
HPezz
commented
Sep 9, 2022
•
HPezz
commented
- Validated on robot
HPezz
changed the base branch from
develop
to
hugo/feature/Add-SuperSimon-activity
File comparision analysis report🔖 Info
Click to show memory sections📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff list
|
Codecov Report
@@ Coverage Diff @@
## hugo/feature/Add-SuperSimon-activity #1015 +/- ##
=======================================================================
Coverage ? 96.03%
=======================================================================
Files ? 133
Lines ? 3106
Branches ? 0
=======================================================================
Hits ? 2983
Misses ? 123
Partials ? 0 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
File comparision analysis report🔖 Info
Click to show memory sections📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff list
|
HPezz
force-pushed
the
hugo/feature/Add-SuperSimon-activity
branch
from
3633449
to
dda1778
Compare
HPezz
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
from
f627356
to
1cc8ac6
Compare
HPezz
added
01 - type: story
HPezz
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
from
1cc8ac6
to
7328a07
Compare
HPezz
force-pushed
the
hugo/feature/Add-SuperSimon-activity
branch
from
dda1778
to
831e956
Compare
HPezz
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
from
7328a07
to
94fdd65
Compare
ladislas
requested changes
Sep 18, 2022
| ++_score; | ||
|
|
||
| if (_score == kScoreToWin) { | ||
| _backup_callback(MagicCard::dice_roll); |
There was a problem hiding this comment.
this is strange -- you're creating an invisible but very strong dependency on something and Activity can' possibly know: what the backup callback would do.
We need to find a better way of exiting an app than that.
If we change the main callback in the Robot Controller, it might break all the activities which should not be the case as they are not at all related
| @@ -56,6 +56,7 @@ | |||
| #include "RobotController.h" | |||
| #include "SDBlockDevice.h" | |||
| #include "SerialNumberKit.h" | |||
| #include "ShapeRecognition.h" | |||
There was a problem hiding this comment.
Could we have a header for all autonomous activities in order to avoid to overcrowd the included files here?
Suggested change
| #include "ShapeRecognition.h" | |
| #include "AutonomousActivities.h" |
Suggested change
| #include "ShapeRecognition.h" | |
| #include "Activities.h" |
Comment on lines
29
to
42
| RFIDKit &_rfidkit; | ||
| interface::VideoKit &_videokit; | ||
| ReinforcerKit &_reinforcerkit; | ||
|
|
||
| static constexpr uint8_t kRoundsNumber = 5; | ||
| static constexpr uint8_t kNumberOfShapes = 4 * 2; | ||
|
|
||
| uint8_t _score = 0; | ||
| std::optional<Shape> _current_shape {}; | ||
| std::function<void(const MagicCard &)> _backup_callback {}; | ||
| std::array<Shape, kNumberOfShapes> _shapes = {Shape::square, Shape::square, Shape::circle, Shape::circle, | ||
| Shape::triangle, Shape::triangle, Shape::star, Shape::star}; | ||
|
|
||
| void launchNextRound(); |
There was a problem hiding this comment.
Guidestyle point
Private methods come first before private members
Comment on lines
21
to
35
| auto on_tag_detected_callback = [this](const MagicCard &card) { | ||
| if (card == _current_shape->card) { | ||
| _reinforcerkit.playDefault(); | ||
| ++_score; | ||
|
|
||
| if (_score == kRoundsNumber) { | ||
| _backup_callback(MagicCard::dice_roll); | ||
| return; | ||
| } | ||
|
|
||
| launchNextRound(); | ||
| } else { | ||
| _backup_callback(card); | ||
| } | ||
| }; |
There was a problem hiding this comment.
I have a question, why this lambda isn't a private method?
There was a problem hiding this comment.
About lambda vs std::bind [Link]
HPezz
force-pushed
the
hugo/feature/Add-SuperSimon-activity
branch
from
831e956
to
685ff14
Compare
HPezz
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
from
79eeecd
to
30bd601
Compare
HPezz
force-pushed
the
hugo/feature/Add-SuperSimon-activity
branch
from
685ff14
to
7b788a4
Compare
HPezz
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
from
30bd601
to
06d5650
Compare
HPezz
force-pushed
the
hugo/feature/Add-SuperSimon-activity
branch
from
7b788a4
to
b88df8a
Compare
HPezz
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
from
06d5650
to
7b61938
Compare
HPezz
force-pushed
the
hugo/feature/Add-SuperSimon-activity
branch
from
b88df8a
to
ccca540
Compare
HPezz
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
3 times, most recently
from
c52bf66
to
2dcea08
Compare
HPezz
force-pushed
the
hugo/feature/Add-SuperSimon-activity
branch
from
ccca540
to
bd777e8
Compare
HPezz
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
from
2dcea08
to
9fd66a4
Compare
ladislas
force-pushed
the
hugo/feature/Add-SuperSimon-activity
branch
from
bd777e8
to
23f3098
Compare
Base automatically changed from
hugo/feature/Add-SuperSimon-activity
to
develop
September 22, 2022 10:15
ladislas
force-pushed
the
hugo/feature/Add-ShapeRecognition-activity
branch
from
9fd66a4
to
99298e6
Compare
|
SonarCloud Quality Gate failed. |
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.