Skip to content
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

Migration from controlboard to controlboardwrapper2 #36

Closed
EnricoMingo opened this issue Nov 13, 2013 · 19 comments
Closed

Migration from controlboard to controlboardwrapper2 #36

EnricoMingo opened this issue Nov 13, 2013 · 19 comments

Comments

@EnricoMingo
Copy link
Member

In the last updated version of YARP from Alberto we have a new devicedriver that is the controlboardwrapper2 (that is the same one present in the real robots COMAN and iCub). We have to update the simulator too. We have tried to make it works but we have some problems, I think a new branch will be needed. "Controlboard" still works, we have new ports for reading torques and velocity so in the future the extra theard to publish these informations in the simulator will be deleted. Alberto will be the one that will work on these migration so pls do not change the structure of the simulator from now on.

@traversaro
Copy link
Member

Couldn't alberto work from the refactored code in https://github.com/robotology/gazebo_yarp_plugins/tree/renaming to avoid redoing the work?

@EnricoMingo
Copy link
Member Author

I don't know what is the best way to do this migration, I was thinking to do not go further implementing other things in the functionality to let Alberto work with no other surprise.

@traversaro
Copy link
Member

It is just refactoring/changing names to classes, not adding new functionality (I expressly did the work in the italian night so It could be integrated this morning without impacting any kind of new work). If necessary I can do the renaming work also after Alberto adds the new functionality, but I guess now the renaming branch is functionally identical to the master branch (I avoid directly commiting it only because I wanted someone else to test/review it before commiting).

@EnricoMingo
Copy link
Member Author

From which we saw this morning is not so simple. There are different changes to do. Maybe Alberto can better clarify the types of changing are needed.

@traversaro
Copy link
Member

No, I was referring to the changes in the renaming branch #31 (comment) that I did this night, they are simply refactoring/renaming classes as already discussed in #31.
My proposal would be that we merge this changes related to renaming (if they are not ok we can still discuss in #31) and then add new functionality.

@EnricoMingo
Copy link
Member Author

ok, I think there are no problems.

@barbalberto
Copy link
Collaborator

Ok, I'll start from the renaming branch.
I'll do some quick fix in order to migrate to the new wrapper early and then the new feature will be added step by step.

I just have to understand a little how the coman device was implemented.

@EnricoMingo
Copy link
Member Author

Ok perfect!

@ghost ghost assigned barbalberto Nov 13, 2013
@traversaro
Copy link
Member

I have made some modification (only in filenames and directory structure) to renaming https://github.com/robotology/gazebo_yarp_plugins/tree/renaming branch as discussed in the hangout.

@traversaro
Copy link
Member

@barbalberto @EnricoMingo Can I delete the Alberto branch? It is inconsistent and make that test fail.

@EnricoMingo
Copy link
Member Author

Yes
Il giorno 20/nov/2013 08:16, "Silvio Traversaro" notifications@github.com
ha scritto:

@barbalberto https://github.com/barbalberto @EnricoMingohttps://github.com/EnricoMingoCan I delete the Alberto branch? It is inconsistent and make that test
fail.


Reply to this email directly or view it on GitHubhttps://github.com//issues/36#issuecomment-28868066
.

@traversaro
Copy link
Member

Done.

@barbalberto
Copy link
Collaborator

Ah, ok.

I saw the error didn t understand the source.
I'll fix it.

I need the branch 'cause I doing other changes.

Alberto

On 20/11/2013 08:16, Silvio Traversaro wrote:

@barbalberto https://github.com/barbalberto @EnricoMingo
https://github.com/EnricoMingo Can I delete the Alberto branch? It
is inconsistent and make that test fail.


Reply to this email directly or view it on GitHub
#36 (comment).

@traversaro
Copy link
Member

@barbalberto My fault, I wrongly understood by @EnricoMingo reply that you were ok in deleting the branch, I already deleted it.
However it was just a pointer to the commit aaf9fb9 so I guess you can simply fork again this commit (or directly fork the master).

@EnricoMingo
Copy link
Member Author

Sorry too, since the branch was merged I thought that the work for that
branch was finished. My fault.
Il giorno 20/nov/2013 09:50, "Silvio Traversaro" notifications@github.com
ha scritto:

@barbalberto https://github.com/barbalberto My fault, I wrongly
understood by @EnricoMingo https://github.com/EnricoMingo reply that
you were ok in deleting the branch, I already deleted it.
However it was just a pointer to the commit aaf9fb9aaf9fb9cc539324cd13433676402db93f1824175so I guess you can simply fork again this commit (or directly fork the
master).


Reply to this email directly or view it on GitHubhttps://github.com//issues/36#issuecomment-28871690
.

@barbalberto
Copy link
Collaborator

Well, no big deal.
I'll create a new one with a better name :)

Alby

On 20/11/2013 09:52, EnricoMingo wrote:

Sorry too, since the branch was merged I thought that the work for that
branch was finished. My fault.
Il giorno 20/nov/2013 09:50, "Silvio Traversaro"
notifications@github.com
ha scritto:

@barbalberto https://github.com/barbalberto My fault, I wrongly
understood by @EnricoMingo https://github.com/EnricoMingo reply that
you were ok in deleting the branch, I already deleted it.
However it was just a pointer to the commit
aaf9fb9aaf9fb9cc539324cd13433676402db93f1824175so
I guess you can simply fork again this commit (or directly fork the
master).


Reply to this email directly or view it on
GitHubhttps://github.com//issues/36#issuecomment-28871690
.


Reply to this email directly or view it on GitHub
#36 (comment).

@traversaro
Copy link
Member

Just before closing this issue, a question for @barbalberto : it makes sense that you get a

yarpdev: ***ERROR*** could not find device <controlboardwrapper2>

If you have iCub 1.1.13 installed from binary package in Ubuntu (and Yarp not compiled with YARP_COMPILE_EXPERIMENTAL_WRAPPERS )? Apparently I wrongly assumed that the device was available from the last iCub binary.

@barbalberto
Copy link
Collaborator

Yes, it is correct.
Controlboardwrapper is still available in icub source, but gazebo plugin has to be indepent from icub (that's exactly the reason why we moved the wrappers from icub to yarp) therefore plugins doesn't look into icub code.

Anyway wrappers will soon be removed from icub repo and the only ones available will be the yarp ones (and the flag will also go away).

Silvio Traversaro notifications@github.com Ha scritto:

Just before closing this issue, a question for @barbalbertohttps://github.com/barbalberto : it makes sense that you get a

yarpdev: _ERROR_ could not find device

If you have iCub 1.1.13 installed from binary package in Ubuntu (and Yarp not compiled with YARP_COMPILE_EXPERIMENTAL_WRAPPERS )? Apparently I wrongly assumed that the device was available from the last iCub binary.


Reply to this email directly or view it on GitHubhttps://github.com//issues/36#issuecomment-30047004.

@traversaro
Copy link
Member

I guess the migrations is completed, and we can close this issue (bugs in the implementation of the controlboardwrapper2 can be discussed in specific issues).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants