{"action":"created","issue":{"url":"https://api.github.com/repos/roboticslab-uc3m/kinematics-dynamics/issues/113","repository_url":"https://api.github.com/repos/roboticslab-uc3m/kinematics-dynamics","labels_url":"https://api.github.com/repos/roboticslab-uc3m/kinematics-dynamics/issues/113/labels{/name}","comments_url":"https://api.github.com/repos/roboticslab-uc3m/kinematics-dynamics/issues/113/comments","events_url":"https://api.github.com/repos/roboticslab-uc3m/kinematics-dynamics/issues/113/events","html_url":"https://github.com/roboticslab-uc3m/kinematics-dynamics/issues/113","id":250725390,"number":113,"title":"Cartesian-related interfaces should be representation-agnostic","user":{"login":"PeterBowman","id":9977198,"avatar_url":"https://avatars1.githubusercontent.com/u/9977198?v=4","gravatar_id":"","url":"https://api.github.com/users/PeterBowman","html_url":"https://github.com/PeterBowman","followers_url":"https://api.github.com/users/PeterBowman/followers","following_url":"https://api.github.com/users/PeterBowman/following{/other_user}","gists_url":"https://api.github.com/users/PeterBowman/gists{/gist_id}","starred_url":"https://api.github.com/users/PeterBowman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PeterBowman/subscriptions","organizations_url":"https://api.github.com/users/PeterBowman/orgs","repos_url":"https://api.github.com/users/PeterBowman/repos","events_url":"https://api.github.com/users/PeterBowman/events{/privacy}","received_events_url":"https://api.github.com/users/PeterBowman/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"PeterBowman","id":9977198,"avatar_url":"https://avatars1.githubusercontent.com/u/9977198?v=4","gravatar_id":"","url":"https://api.github.com/users/PeterBowman","html_url":"https://github.com/PeterBowman","followers_url":"https://api.github.com/users/PeterBowman/followers","following_url":"https://api.github.com/users/PeterBowman/following{/other_user}","gists_url":"https://api.github.com/users/PeterBowman/gists{/gist_id}","starred_url":"https://api.github.com/users/PeterBowman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PeterBowman/subscriptions","organizations_url":"https://api.github.com/users/PeterBowman/orgs","repos_url":"https://api.github.com/users/PeterBowman/repos","events_url":"https://api.github.com/users/PeterBowman/events{/privacy}","received_events_url":"https://api.github.com/users/PeterBowman/received_events","type":"User","site_admin":false},"assignees":[{"login":"PeterBowman","id":9977198,"avatar_url":"https://avatars1.githubusercontent.com/u/9977198?v=4","gravatar_id":"","url":"https://api.github.com/users/PeterBowman","html_url":"https://github.com/PeterBowman","followers_url":"https://api.github.com/users/PeterBowman/followers","following_url":"https://api.github.com/users/PeterBowman/following{/other_user}","gists_url":"https://api.github.com/users/PeterBowman/gists{/gist_id}","starred_url":"https://api.github.com/users/PeterBowman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PeterBowman/subscriptions","organizations_url":"https://api.github.com/users/PeterBowman/orgs","repos_url":"https://api.github.com/users/PeterBowman/repos","events_url":"https://api.github.com/users/PeterBowman/events{/privacy}","received_events_url":"https://api.github.com/users/PeterBowman/received_events","type":"User","site_admin":false}],"milestone":null,"comments":0,"created_at":"2017-08-16T18:54:38Z","updated_at":"2017-08-16T19:17:29Z","closed_at":null,"body":"Currently, the class relationship tree for cartesian control-related interfaces may force its intermediate actors to make assumptions about angle representation up to four times, that is, from the perspective of:\r\n\r\n1. the client (application) code, say a module that needs to interface with our cartesian controller (e.g. the `CartesianControlClient`), or pipe directly to a solver device (`KdlSolver`, `AsibotSolver`)\r\n\r\n2. the cartesian controller ([`ICartesianControl`](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/TeoYarp/ICartesianControl.h))\r\n\r\n3. the cartesian solver, called by 2. or as a stand-alone device as in 1. ([`ICartesianSolver`](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/TeoYarp/ICartesianSolver.h))\r\n\r\n4. the trajectory generator, so far only used by 2. ([`Trajectory`](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/TrajectoryLib/Trajectory.hpp))\r\n\r\nAll of these make use of generic containers in their public interface, e.g.:\r\n\r\n```c++\r\n// ICartesianControl\r\nbool movj(const std::vector<double> &xd);\r\n\r\n// ICartesianSolver\r\nbool appendLink(const std::vector<double>& x);\r\n\r\n// Trajectory\r\nbool getX(const double movementTime, std::vector<double>& x);\r\n```\r\n\r\nSince the `std::vector<double>` parameters carry no information about the angle representation, clients may not *encode* said vectors at their convenience; instead, they must conform to the `angleRepr` .ini parameter used to consistently instantiate the whole chain (usually passed to an implementor of `ICartesianControl`, like `BasicCartesianControl`). Note that it didn't work that way prior to #101 as *axisAngle* was virtually hardcoded. This is equally true for intermediate classes, e.g. `BasicCartesianControl::inv` *talks* to a `ICartesianSolver::invKin` and the vector passed between them ([ref](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/TeoYarp/BasicCartesianControl/ICartesianControlImpl.cpp#L37)) must be kept coherent with the chosen representation.\r\n\r\nAnother point of grief is the lack of uniformity across different methods within each of the aforementioned interfaces. Compare:\r\n\r\n* `KdlSolver::diffInvKin` - one may not notice this at first, but the KDL implementation forces callers to pass a `std::vector` encoded as a velocity twist (i.e. scaled axis-angle notation, [ref](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/TeoYarp/KdlSolver/ICartesianSolverImpl.cpp#L135)). Similarly, the return parameter of `KdlSolver::fwdKinError` is another twist-encoded container ([ref](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/TeoYarp/KdlSolver/ICartesianSolverImpl.cpp#L56)); however, its first input parameter is a \"partially\" constrained vector, in a manner such that callers are not free to decide on which representation to work with, yet it's not fully hardcoded - here, it depends on the device initialization via CLI/.ini file.\r\n\r\n* `LineTrajectory::getX` vs `LineTrajectory::getXdot` - the latter forces clients to adhere to velocity twists since this is the angle representation used in the CMC thread of `BasicCartesianControl` ([ref](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/TeoYarp/BasicCartesianControl/RateThreadImpl.cpp#L10)). Now, we may develop a full picture of this mess:\r\n  ```c++\r\n  trajectory.getX(movementTime, desiredX); // returns 'desiredX', angle-repr as specified in .ini\r\n  trajectory.getXdot(movementTime, desiredXdot); // returns 'desiredXdot' - this is always a velocity twist\r\n  // ...\r\n  iCartesianSolver->fwdKinError(desiredX, currentQ, commandXdot); // pass .ini-controlled 'desiredX', returns (infinitiesimal) displacement twist in 'commandXdot'\r\n  // ... finishes performing numerical differentiation on 'commandXdot'\r\n  if (! iCartesianSolver->diffInvKin(currentQ, commandXdot, commandQdot) ) {} // accepts velocity twist\r\n  ```\r\n\r\nMethod calls may seem quite clean by now in [`BasicCartesianControl/ICartesianControlImpl.cpp`](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/TeoYarp/BasicCartesianControl/ICartesianControlImpl.cpp#L37), but clashes between representations already arised while working on #105 and #86, and also motivated #107.\r\n\r\nAt this point, we may want to distinguish what kind of representations are considered here:\r\n\r\n* Linear vs angular:\r\n  * linear: ATM only cartesian, at some point we may want to explore cylindrical and spherical coordinates\r\n  * angular: currently the ones handled by [`KdlVectorConverter`](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/libraries/KdlVectorConverterLib/KdlVectorConverter.cpp) with more to come (see *velocity* below)\r\n\r\n* Position vs velocity (vs acceleration?):\r\n  * position: as in `KdlVectorConverter`, used to describe a pose\r\n  * velocity: integration factors used to express rotational velocity in terms of an angle set and vice-versa\r\n\r\n---\r\n\r\nProposed solutions:\r\n\r\n1. Force all interfaces to accept the same representation, ideally *axisAngle* (scaled) as it is the one used by KDL internals.\r\n2. Improve `KdlVectorConverter` to handle all cases of linear/angular position/velocity(/acceleration) representations, make it store positions/angles and pass it as a parameter type to all interface methods listed above (instead of `std::vector`). Callers and callees will act as encoders and decoders of those representations, respectively (e.g. store our data as *eulerZYZ*, pass it through an interface, unpack it as *axisAngle* on the callee's side). Probably, this class would have to implement `yarp::os::Portable` in order to make itself readable and writable on the YARP network."},"comment":{"url":"https://api.github.com/repos/roboticslab-uc3m/kinematics-dynamics/issues/comments/322871972","html_url":"https://github.com/roboticslab-uc3m/kinematics-dynamics/issues/113#issuecomment-322871972","issue_url":"https://api.github.com/repos/roboticslab-uc3m/kinematics-dynamics/issues/113","id":322871972,"user":{"login":"PeterBowman","id":9977198,"avatar_url":"https://avatars1.githubusercontent.com/u/9977198?v=4","gravatar_id":"","url":"https://api.github.com/users/PeterBowman","html_url":"https://github.com/PeterBowman","followers_url":"https://api.github.com/users/PeterBowman/followers","following_url":"https://api.github.com/users/PeterBowman/following{/other_user}","gists_url":"https://api.github.com/users/PeterBowman/gists{/gist_id}","starred_url":"https://api.github.com/users/PeterBowman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PeterBowman/subscriptions","organizations_url":"https://api.github.com/users/PeterBowman/orgs","repos_url":"https://api.github.com/users/PeterBowman/repos","events_url":"https://api.github.com/users/PeterBowman/events{/privacy}","received_events_url":"https://api.github.com/users/PeterBowman/received_events","type":"User","site_admin":false},"created_at":"2017-08-16T19:17:29Z","updated_at":"2017-08-16T19:17:29Z","body":"I'll give the second option a try. @jgvictores I'd greatly appreciate any idea or objection you could come up with, I'm a bit in a hurry to have this closed and move on to the next point. Perhaps you already run into any of this; for instance, I noticed that `transCoordsUsingJoints` is ignoring orientation values at [PremultPorts.cpp#L54](https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/8d798ca1b44df125fd404261ca04996641738a83/programs/transCoordsUsingJoints/PremultPorts.cpp#L54)."}}
{"actor":{"display_login":"PeterBowman"}}