Bugfix in Yarp iRobotControl implementation #118
Conversation
GiulioRomualdi
requested review from
traversaro,
prashanthr05 and
S-Dafarra
… in YarpRobotControl class
…e when the YarpRobotControl interface is initalized
GiulioRomualdi
force-pushed
the
fix/IRobotControl
branch
from
a96a357 to
8c17556
Compare
| constexpr unsigned maxIter = 100; | ||
| constexpr unsigned timeout = 500; |
There was a problem hiding this comment.
Is it worth putting them as part of the initialization parameters?
There was a problem hiding this comment.
We can have default values and the user can change them. Do you agree?
…eter in the YarpRobotControl class
|
Waiting for @S-Dafarra approval before merging |
| ok = ok && ptr->getParameter("positioning_tolerance", m_pimpl->positioningTolerance); | ||
| // optional parameters | ||
| int temp = 0; | ||
| if (ptr->getParameter("reading_timeout", temp)) |
There was a problem hiding this comment.
The problem with this is that if the reading_timeout field is missing, it will always throw an error. We may need to add a "quiet" mode for optional parameters. Anyways, this goes outside the scope of this PR.
There was a problem hiding this comment.
The problem with this is that if the reading_timeout field is missing, it will always throw an error. We may need to add a "quiet" mode for optional parameters. Anyways, this goes outside the scope of this PR.
Yes, this is really annoying. I opened an issue for that: #120
This PR fixes two bugs in the
yarpimplementation of theiRobotControlclass.