yann hugo/feature/sm/add emergency stop state #853
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
Jun 8, 2022
•
HPezz
commented
- Validé sur le robot
- ✨ (sm): Add Shutoff State + Emergency Stop Event
- ⚡ (rc): Add on_entry Shutoff State, RC turnOffActuators
- 🚸 (os): Link RFID event to SM event (Emergency Stop)
- 🚸 (sm): Add transition from Shutoff to Working
- 🚸 (sm): Add transition from Shutoff to Charging
- 🚸 (sm): Add transition from Charging to ShutOff
- 🚸 (sm): Add transition from Idle to ShutOff
- 🚸 (sm): Add transition from Sleeping to ShutOff
- 🚚 Rename ShutOff in EmergencyStopped
|
Close #850 in favour of this one |
2 tasks
Codecov Report
@@ Coverage Diff @@
## develop #853 +/- ##
===========================================
+ Coverage 95.36% 95.39% +0.02%
===========================================
Files 115 115
Lines 2544 2560 +16
===========================================
+ Hits 2426 2442 +16
Misses 118 118
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
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
|
YannLocatelli
approved these changes
Jun 8, 2022
ladislas
approved these changes
Jun 8, 2022
There was a problem hiding this comment.
LGTM as well 👍
quelques petits changements:
- renommer les commits pour avoir
emergency_stop/pedet passhut_off - dans la transition table, je mettrai toutes les
state + emergency_stopensemble, ça simplifie la lecture et ça sera plus facile à ajouter dans le futur -- ça montre aussi que c'est homogène pour toutes les states et on peut vérifier qu'on en a pas oublié
HPezz
force-pushed
the
yann-hugo/feature/sm/add-emergency-stop-state
branch
from
a336b85
to
9e25588
Compare
First transition from Working to Shutoff with Emergency Stop event
HPezz
force-pushed
the
yann-hugo/feature/sm/add-emergency-stop-state
branch
from
9e25588
to
b0421b2
Compare
Event command_received
Event charge_did_start
YannLocatelli
force-pushed
the
yann-hugo/feature/sm/add-emergency-stop-state
branch
from
b0421b2
to
f9e5163
Compare
ladislas
approved these changes
Jun 9, 2022
Comment on lines
206
to
211
| void turnOffActuators() final | ||
| { | ||
| _lcd.turnOff(); | ||
| stopActuators(); | ||
| } | ||
|
|
There was a problem hiding this comment.
est-ce qu'on a besoin d'une nouvelle fonction ici? on ne peut pas mettre le lcd dans stopActuators?
si on peut pas, il faut un nom qui montre la différence et les mettre l'une à côté de l'autre dans le code (celle-ci en dessous)
Suggested change
| void turnOffActuators() final | |
| { | |
| _lcd.turnOff(); | |
| stopActuators(); | |
| } | |
| void stopActuatorsAndLcd() final | |
| { | |
| _lcd.turnOff(); | |
| stopActuators(); | |
| } | |
libs/RobotKit/include/StateMachine.h
Outdated
Comment on lines
142
to
144
| struct turn_off_actuators { | ||
| auto operator()(irc &rc) const { rc.turnOffActuators(); } | ||
| }; |
There was a problem hiding this comment.
ici aussi il faut faire le rename proposer plus haut pour garder homogène.
parce qu'au fond "stop" et "turn off" c'est globalement la même chose et on voit pas que dans un tu as le lcd et pas dans l'autre.
|
Kudos, SonarCloud Quality Gate passed! |
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.