-
Notifications
You must be signed in to change notification settings - Fork 7
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
hugo/feature/Internationalize ActivityKit and RC #1049
hugo/feature/Internationalize ActivityKit and RC #1049
Conversation
HPezz
commented
Sep 29, 2022
•
edited
Loading
edited
- Validated on robot
File comparision analysis report🔖 Info
Click to show memory sections
📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff list
|
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 @@
## develop #1049 +/- ##
========================================
Coverage 95.99% 96.00%
========================================
Files 133 133
Lines 3173 3178 +5
========================================
+ Hits 3046 3051 +5
Misses 127 127
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
aa0cf8e
to
8d25ed6
Compare
8d25ed6
to
73fdf3a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Validated on the robot
89d51ff
to
4097645
Compare
73fdf3a
to
b9e7ed9
Compare
Added the following comment Suggestion. Add void startAutonomousActivityMode() final
{
_lcd.turnOn();
auto card = _rfidkit.getLastMagicCardActivated();
_activitykit.displayMainMenu(card);
} |
b2ac5fc
to
75129f8
Compare
46e154b
to
1a77f51
Compare
75129f8
to
172a27b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still have two questions
void ActivityKit::displayMainMenu(const MagicCard &card) | ||
{ | ||
if (card.getLanguage() == Language::fr_FR) { | ||
_videokit.displayImage("/fs/home/img/system/robot-misc-choose_activity-fr_FR.jpg"); | ||
} else { | ||
_videokit.displayImage("/fs/home/img/system/robot-misc-choose_activity-en_US.jpg"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we check first if the card is indeed the dire_roll?
void ActivityKit::displayMainMenu(const MagicCard &card) | |
{ | |
if (card.getLanguage() == Language::fr_FR) { | |
_videokit.displayImage("/fs/home/img/system/robot-misc-choose_activity-fr_FR.jpg"); | |
} else { | |
_videokit.displayImage("/fs/home/img/system/robot-misc-choose_activity-en_US.jpg"); | |
} | |
} | |
void ActivityKit::displayMainMenu(const MagicCard &card) | |
{ | |
if (card != MagicCard::dire_roll) { | |
return; | |
} | |
if (card.getLanguage() == Language::fr_FR) { | |
_videokit.displayImage("/fs/home/img/system/robot-misc-choose_activity-fr_FR.jpg"); | |
} else { | |
_videokit.displayImage("/fs/home/img/system/robot-misc-choose_activity-en_US.jpg"); | |
} | |
} |
@@ -196,7 +196,9 @@ class RobotController : public interface::RobotController | |||
void startAutonomousActivityMode() final | |||
{ | |||
_lcd.turnOn(); | |||
_behaviorkit.displayAutonomousActivitiesPrompt(); | |||
|
|||
auto card = _rfidkit.getLastMagicCardActivated(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is smart but are you absolutely certain that it will work if:
- the user puts 2 cards (dice + something else)
- dice is read
- then the other one is read
- this function is called
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The execution speed between the dice_roll detection and the main menu display is so tiny that we couldn't put an other card immediately upon the dice roll detection, even willingly.
172a27b
to
6db1e51
Compare
172a27b
to
944f403
Compare
944f403
to
d4011bb
Compare
Kudos, SonarCloud Quality Gate passed! |