Skip to content
This repository has been archived by the owner on Sep 29, 2020. It is now read-only.

[Interface] Remove wbm_setWorldLink ? #43

Closed
traversaro opened this issue Oct 21, 2015 · 8 comments
Closed

[Interface] Remove wbm_setWorldLink ? #43

traversaro opened this issue Oct 21, 2015 · 8 comments
Assignees
Milestone

Comments

@traversaro
Copy link
Contributor

A lot of users have been confused by the semantics of the setWorldLink function #26 and now it is possible to use all the functions by directly passing the complete state #27 .

If no code it is supposed to use this function, perhaps it is a better idea to just remove it from the interface, to avoid confusion/misuse by the users?

After that, for symmetry with all existing calls, it could make sense to have an updateState in which you pass all the state, with also the base position (basically a combination of setWorldFrame + updateState ).

In this way we would be more consistent with all the existing interfaces.

@traversaro traversaro added this to the Cleanup milestone Oct 21, 2015
@traversaro traversaro changed the title [Interface] Remove wbm_setWorldLink and wbm_setWorldFrame ? [Interface] Remove wbm_setWorldLink ? Oct 21, 2015
@traversaro
Copy link
Contributor Author

cc @naveenoid @gabrielenava opinions?

@traversaro
Copy link
Contributor Author

This would also have the side benefits of removing the hardcoded "l_sole" in the code ( https://github.com/robotology/mex-wholebodymodel/blob/6603ab444e828f05bcb71c0317e4421e97736ac8/src/modelstate.cpp#L41 ).

@traversaro
Copy link
Contributor Author

Another example of confusion regarding wbm_setWorldLink semantics : #47 (comment) .

@DanielePucci
Copy link
Contributor

I completely agree with you: passing the whole state is the right way to go. How do we pass this state for SE(3)? Do we pass quaternions or what? @naveenoid opinions?

@traversaro
Copy link
Contributor Author

We are currently passing rotation matrices.

@DanielePucci
Copy link
Contributor

Now we are talking [1]

[1] http://www.youtube.com/watch?v=k17KH6F63aE&t=1m24s

@naveenoid
Copy link
Contributor

I will most likely remove this once PR #49 is merged

@naveenoid
Copy link
Contributor

Tackled in #52

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

No branches or pull requests

3 participants