-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementation of ICub device #25
Conversation
5353300
to
0733eab
Compare
bool firstRun = true; | ||
mutable std::mutex mtx; | ||
|
||
using yarp::os::BufferedPort<yarp::os::Bottle>::onRead; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you try to switch to using yarp::os::BufferedPort<yarp::os::Vector>::onRead;
? If it works, reading the message will be more robust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @Yeshasvitvs @lrapetti
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I try to switch here to using yarp::os::BufferedPort<yarp::sig::Vector>::onRead;
, I am getting the following:
[ 84%] Building CXX object devices/ICub/CMakeFiles/ICub.dir/src/ICub.cpp.o
/Users/lrapetti/robotology-playground/wearables/devices/ICub/src/ICub.cpp:47:11: error:
using declaration refers into
'yarp::os::BufferedPort<yarp::sig::Vector>::', which is not a base class
of 'WrenchPort'
using yarp::os::BufferedPort<yarp::sig::Vector>::onRead;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [devices/ICub/CMakeFiles/ICub.dir/src/ICub.cpp.o] Error 1
make[1]: *** [devices/ICub/CMakeFiles/ICub.dir/all] Error 2
make: *** [all] Error 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost done in my opinion beyond previous unresolved comments. Remember to pass this file through clang-format before merging.
ee3990a
to
4d1d418
Compare
If I understood well, the |
Done |
Done |
Following the discussion with @diegoferigo we can change to using |
I did a mess with these comments, I forgot that this PR was open and instead of doing a review I commented the file of the commit 😅 Anyway:
You avoid using the pair control board index + joint index in the control board. You just use the joint name. Simple clear.
Before the merge, possibly ;) |
4f55e8c
to
1857545
Compare
Update position variable name in VirtualJointKinSensor Update interface pointer handling in joint sensor constructor Update spherical joint variables name in XsensSuit device
c53a830
to
01d9fe6
Compare
All the checks are passed.
This is the only change request still pending. @Yeshasvitvs @diegoferigo, if you agree I would proceed with merging so that we can unlock other issues. |
I think @diegoferigo is not responsive on GitHub and he already expressed in person his ok with merging the PR, so if you need a quick feedback you may need to contact him via chat. |
I'm off this week and I doubt I can do a last pass on this PR before my return. As said last time, if the remote control board remapper is postponed, this PR is good to go. Please open an issue to keep track of this task. If there are any questions please be specific and point me to the code so thatI can have a look with the phone. |
@lrapetti ok for me |
issue opened at #49 |
Cleaned PR starting from #16.
For the moment I am leaving the original commits, in case we may prefere to squash them.