Spike autocharge #1336
Open
Spike autocharge #1336
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
YannLocatelli
force-pushed
the
yann/feature/charge/use-imukit-and-motors-to-go-on-charge
branch
2 times, most recently
from
f7136e8 to
44d0e19
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1336 +/- ##
========================================
Coverage 98.74% 98.74%
========================================
Files 146 146
Lines 3759 3759
========================================
Hits 3712 3712
Misses 47 47 ☔ View full report in Codecov by Sentry. |
YannLocatelli
force-pushed
the
yann/feature/charge/use-imukit-and-motors-to-go-on-charge
branch
2 times, most recently
from
e5da452 to
b23b856
Compare
YannLocatelli
commented
Dec 12, 2023
YannLocatelli
force-pushed
the
yann/feature/charge/use-imukit-and-motors-to-go-on-charge
branch
from
b23b856 to
17cb998
Compare
🔖 Version comparison
|
📈 Changes Impact Analysis Report📌 Info
🤖 Firmware impact analysis
Click to show memory sections🔬 Detailed impact analysisClick to show detailed analysis for all targets
🗺️ Map files diff output
|
📈 Changes Impact Analysis Report📌 Info
🤖 Firmware impact analysis
Click to show memory sections🔬 Detailed impact analysisClick to show detailed analysis for all targets
🗺️ Map files diff output
|
YannLocatelli
force-pushed
the
yann/feature/charge/use-imukit-and-motors-to-go-on-charge
branch
3 times, most recently
from
06ce90e to
dc99b43
Compare
YannLocatelli
changed the title
5 tasks
YannLocatelli
force-pushed
the
yann/feature/charge/use-imukit-and-motors-to-go-on-charge
branch
from
dc99b43 to
a049b4a
Compare
HPezz
reviewed
Dec 13, 2023
Comment on lines
56
to
68
| if (is_right_tilted) { | ||
| if (should_move_forward) { | ||
| spinRight(kMinPwmOutput, kMinPwmOutput + speed_offset); | ||
| } else { | ||
| spinLeft(kMinPwmOutput, kMinPwmOutput + speed_offset); | ||
| } | ||
| } else { | ||
| if (should_move_forward) { | ||
| spinLeft(kMinPwmOutput + speed_offset, kMinPwmOutput); | ||
| } else { | ||
| spinRight(kMinPwmOutput + speed_offset, kMinPwmOutput); | ||
| } | ||
| } |
There was a problem hiding this comment.
It's been a while since I've coded in C++ !! I kind of remember of us not using ternary operator for a reason but can you put this part in a independant function ?
void SealStrategy::performSpin(bool is_right_tilted, bool should_move_forward, float speed_offset) {
if (is_right_tilted) {
should_move_forward ? spinRight(kMinPwmOutput, kMinPwmOutput + speed_offset)
: spinLeft(kMinPwmOutput, kMinPwmOutput + speed_offset);
} else {
should_move_forward ? spinLeft(kMinPwmOutput + speed_offset, kMinPwmOutput)
: spinRight(kMinPwmOutput + speed_offset, kMinPwmOutput);
}
}
This would give a nice :
if (abs_float(angles.roll) > kRollTolerance) {
auto speed_offset = convertToPwmFrom(angles.roll > 0 ? angles.roll : -angles.roll);
performSpin(is_right_tilted, should_move_forward, speed_offset);
} else if (abs_float(angles.pitch) > kPitchTolerance) {
auto speed = convertToPwmFrom(angles.pitch > 0 ? angles.pitch : -angles.pitch);
should_move_forward ? moveForward(speed) : moveBackward(speed);
} else {
stopMotors();
}
There was a problem hiding this comment.
Thank you!
Ternary was mostly not used since it might be hard to read and understand. However I never used them to call a function, only to set variable so I don't know if you can use it as is.
I will let them in if/else statement since it is easier to read and understand what happened to me
I will add your suggestion to move in function these whole if/else statement
|
This was referenced Dec 15, 2023
YannLocatelli
force-pushed
the
yann/feature/charge/use-imukit-and-motors-to-go-on-charge
branch
from
29b5d0f to
1799df5
Compare
seal strategy
An original suggestion from @HPezz Co-Authored-By: Hugo Pezziardi <[email protected]>
An original suggestion from @HPezz Co-Authored-By: Hugo Pezziardi <[email protected]>
YannLocatelli
force-pushed
the
yann/feature/charge/use-imukit-and-motors-to-go-on-charge
branch
from
1799df5 to
c1cc2f0
Compare
|
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.
No description provided.