epfl-lasa / control-libraries

A collection of library modules to facilitate the creation of full control loop algorithms, including state representation, motion planning, kinematics, dynamics and control.
https://epfl-lasa.github.io/control-libraries
GNU General Public License v3.0
27 stars 2 forks source link

Use pyquaternion in bindings #283

Closed domire8 closed 2 years ago

domire8 commented 2 years ago

This is something that we had on our mind since we started, but wasn't important up until now (and maybe still isn't but I had time for that small feature and we have breaking changes already).

By installing pyquaternion we can do get_orientation and set_orientation with a pyquaternion.quaternion.Quaternion object, which is quite nice because you can then do a lot of quaternion operations directly with that object.

For simplicity and actually also consistency, I renamed the overloaded functions in cpp set_orientation with arguments Eigen vector and std vector to set_orientation_coefficients, as it was already the case for the getter.

As a result, set/get_orientation only works with Eigen quaternion or pyquaternion, and set/get_orientation_coefficients with vectors and lists.

What I don't know is how we could in pybind typehint the set/get_orientation function such that the user knows that he/she has to work with pyquaternion because pyquaternion is not known in cpp and I have to use py::object and then just throw an exception if its of uncorrect type

domire8 commented 2 years ago

@eeberhard See my last commit for what you proposed. I have to say I don't like it that much. In my opinion its neither more readable, nor faster but its error prone. Even a dict has a getitem method and do we want to just raise any exception when you try to set the orientation with a dict? In my opinion we should only allow the types we know how to handle safely - that's also how we do it for the rest of the bindings.