Inconsequent behavior of 'vector()' method for Quaternion/Position

[dymczykm]

The problem is that .vector() method returning Eigen vector works in a different way for Kindr Quaternion and Position objects. For Position, it returns a reference to the original data while for Quaternion a copy is returned.

The current situation is dangerous, because many people (including me) assume that same-name methods generally do the same. So, in my case, once I was happy with pointer my_position.vector().data(), I gladly wrote my_quaternion.vector().data() to realize that the object remains unchanged. If you're using some optimization library on top of Kindr, the last thing you suspect is that you're getting a copy and not a reference.

I understand it's not trivial to solve this misleading situation because Eigen is storing quaternions with underlying [xyzw] memory layout and Kindr 'promotes' Hamilton convention [wxyz] and you probably want to return a vector in this order.

I think the documentation should mention, that generally the safest way to get direct access to memory (e.g. for optimization purposes) is to use .toImplementation() method returning a reference to underlying Eigen object (and then using .data() to get a pointer).

I know this issue has been already discussed over mail (with @simonlynen), but @furgalep suggested that it may be worth to talk about it here and also keep track of it.

[gehrinch]

This is indeed problematic.

A possible solution would be to get rid of the underlying Eigen implementation. For now, this should be added to the documentation as suggested.

@dymczykm Can you please add it to the documentation?

[gehrinch]

We could improve the readability by using the keyword 'get' for value returns. For instance:

• Scalar EulerAnglesZyx::getPitch()
• Scalar& EulerAnglesZyx::pitch()

Any suggestions?

[HannesSommer]

I like the first one because it is pretty standard. The second one is much more a problem. In the programming guidelines we finally decided to use getPitchRef() - to be very explicit. See https://github.com/ethz-asl/programming_guidelines/issues/5#issuecomment-19169821

[HannesSommer]

For a const ref I would still use just getPitch()!