Add unit testing to robot hardware#109
Conversation
Codecov Report
@@ Coverage Diff @@
## master #109 +/- ##
==========================================
+ Coverage 40.98% 44.10% +3.11%
==========================================
Files 24 31 +7
Lines 527 653 +126
Branches 273 321 +48
==========================================
+ Hits 216 288 +72
- Misses 82 83 +1
- Partials 229 282 +53
Continue to review full report at Codecov.
|
@bmagyar This is the one where it complains on the first usage of |
jordan-palacios
left a comment
jordan-palacios
left a comment
There was a problem hiding this comment.
Looks good. Minor changes suggested.
Karsten1987
left a comment
Karsten1987
left a comment
There was a problem hiding this comment.
can you please revert the style changes to make the diff smaller here? There's quite a bunch of unrelated and unnecessary whitespace changes in here.
| : name_(), | ||
| pos_(nullptr), | ||
| vel_(nullptr), | ||
| eff_(nullptr) | ||
| {} |
There was a problem hiding this comment.
why these changes? They seem unrelated
There was a problem hiding this comment.
I really don't know, as I put in the opening post,
There are some changes in this PR that should be credited to uncrustify...
Sure, I'll revert 'em
| : name_(), | ||
| cmd_(nullptr) | ||
| {} |
There was a problem hiding this comment.
why these changes? They seem unrelated
|
@Karsten1987 all unsolicited style changes have been reverted but I have to admit I kinda like them better that way, may submit a PR later with just that. One outstanding point from you regarding the de-referencing comment. @jordan-palacios thanks for the note on |
Karsten1987
left a comment
Karsten1987
left a comment
There was a problem hiding this comment.
a comment for the name of the validation function, but generally looks good to me.
| set_cmd(double cmd); | ||
|
|
||
| HARDWARE_INTERFACE_PUBLIC | ||
| bool valid_pointers() const; |
There was a problem hiding this comment.
that function name should be more precise. valid_pointers doesn't really say anything about what that function does, meaning which pointers are being checked.
There was a problem hiding this comment.
also, the function states pointers as in multiple pointers, the implementation does however only check a single cmd_.
There was a problem hiding this comment.
I have the same function implemented in all the other handles and I call this from the template function so they should all be called the same. TBH a proper solution would see inheriting such an interface to really ensure this function existing the same way for all
Co-authored-by: Karsten Knese <Karsten1987@users.noreply.github.com>
* joint_state_controller using resource manager * Touch up CI version for ros-action Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
4 participants
As per mentioned during the last meeting that this package has no proper unit testing, I've added some for
RobotHardware. The initial plan was to move the one fromtest_robot_hardwarebut it is very tied to what exactly is happening with that implementation and I wanted to have a more general set of tests.There are some changes in this PR that should be credited to uncrustify...
On the other hand, @Karsten1987 if you could assist with the cppcheck failure I'd be glad. I am getting the same locally and don't know what to do with it really...