hugo/feature/Mock LedKit #1206
Merged
hugo/feature/Mock LedKit #1206
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
HPezz
force-pushed
the
hugo/feature/Mock-LedKit
branch
from
5811314
to
9d5d337
Compare
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 #1206 +/- ##
========================================
Coverage 96.12% 96.12%
========================================
Files 146 146
Lines 3565 3565
========================================
Hits 3427 3427
Misses 138 138
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
HPezz
force-pushed
the
hugo/feature/Mock-LedKit
branch
from
cf44073
to
dd5eaed
Compare
ladislas
requested changes
Dec 14, 2022
| #include "interface/libs/VideoKit.h" | ||
|
|
||
| namespace leka { | ||
|
|
||
| class BehaviorKit | ||
| { | ||
| public: | ||
| explicit BehaviorKit(interface::VideoKit &videokit, LedKit &ledkit, interface::Motor &motor_left, | ||
| explicit BehaviorKit(interface::VideoKit &videokit, interface::LedKit &ledkit, interface::Motor &motor_left, |
There was a problem hiding this comment.
not related to the PR, but could behaviors depend on motionkit instead of motors? how are motors used here?
| CommandKit &cmdkit, RFIDKit &rfidkit, ActivityKit &activitykit) | ||
| interface::Motor &motor_right, interface::LED &ears, interface::LED &belt, | ||
| interface::LedKit &ledkit, interface::LCD &lcd, interface::VideoKit &videokit, | ||
| BehaviorKit &behaviorkit, CommandKit &cmdkit, RFIDKit &rfidkit, ActivityKit &activitykit) |
There was a problem hiding this comment.
not related directly but we could also have an interface for behaviorkit, CommandKit, RFIDKit and ActivityKit to make testing easier
Comment on lines
53
to
62
| EXPECT_CALL(mock_videokit, playVideoOnce); | ||
| EXPECT_CALL(mock_motor_left, stop).Times(1); | ||
| EXPECT_CALL(mock_motor_right, stop).Times(1); | ||
| EXPECT_CALL(mock_motor_left, spin).Times(1); | ||
| EXPECT_CALL(mock_motor_right, spin).Times(1); |
There was a problem hiding this comment.
not directly related but:
this should be tested with mock::MotionKit now that Reinforcer use MotionKit
this will make everything cleaner -- no need to test the motors are spinning individually when you have the MotionKit::rotate function
Comment on lines
133
to
135
| EXPECT_CALL(mock_motor_left, stop()).Times(AtLeast(2)); | ||
| EXPECT_CALL(mock_motor_right, stop()).Times(AtLeast(2)); |
There was a problem hiding this comment.
unrelated question: if MotionKit is running, will this really stop the actuators?
RobotController should also stop MotionKit in this case, no?
HPezz
force-pushed
the
hugo/feature/Mock-LedKit
branch
from
dd5eaed
to
9f47dfe
Compare
YannLocatelli
requested changes
Dec 14, 2022
There was a problem hiding this comment.
Set in same commit
- BehaviorKit modifications and BehaviorKit_tests modifications
- ReinforcerKit modifications and ReinforcerKit_tests modifications
--
There is a lot of new Uninteresting mock function call - returning directly.
--
Does it works on the robot?
--
Review done before ac510ed was pushed
HPezz
force-pushed
the
hugo/feature/Mock-LedKit
branch
3 times, most recently
from
d641c2a
to
d928e5e
Compare
HPezz
force-pushed
the
hugo/feature/Mock-LedKit
branch
from
5d61435
to
59d6656
Compare
HPezz
force-pushed
the
hugo/feature/Mock-LedKit
branch
from
59d6656
to
776aa06
Compare
HPezz
force-pushed
the
hugo/feature/Mock-LedKit
branch
from
62cf717
to
10430ec
Compare
|
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.
Mock LedKit to simplify tests using it like BehaviorKit, Reinforcer or more consequently RobotController