search

roboticslab-uc3m / kinematics-dynamics

Kinematics and dynamics solvers and controllers.
https://robots.uc3m.es/kinematics-dynamics/
GNU Lesser General Public License v2.1
19 stars 12 forks source link

Reinstate controller vocabs in C++ language bindings #180

Closed PeterBowman closed 3 years ago

PeterBowman commented 5 years ago

Once upon a time, all these vocabs were accessible from SWIG-generated Python bindings. They are not anymore since https://github.com/roboticslab-uc3m/kinematics-dynamics/issues/150, see also linked YARP issues. It's not clear if a recent SWIG version overrides this faulty behavior, but YARP devs moved on and preprocessor definitions have been replaced by constexpr global variables, which seem correctly exported to Python. Also, we know that those vocabs, no matter how we define them, placed within an enumeration within a class, work as intended, too (but global enums fail).

Note that we define ROBOTICSLAB_VOCAB in two places: ICartesianControl.h and ICartesianSolver.h, the latter #included by the latter. Macro definitions may appear twice or more as long as they look exactly the same. Global functions (including the constexpr ones) must obey the One Definition Rule, thus having two definitions will result in compilation errors.

ASBJ (as suggested by @jgvictores), we could follow the constexpr way and define the function within ICartesianSolver.h, which is always included. I'm not sure if that makes sense given that most vocabs are defined in ICartesianControl.h. Alternatively, move the funcion to a separate header file, or depend on YARP's vocab system.

PeterBowman commented 3 years ago

We use these interfaces in the context of YARP devices anyway. @jgvictores OK for turning these macros into constexpr YARP vocabs?

jgvictores commented 3 years ago

Sure, would be great!

PeterBowman commented 3 years ago

Not done yet, I didn't notice these vocabs (unless wrapped inside an enum such as ICartesianControl.BASE_FRAME) do not translate directly into integers, as YARP's vocabs do, but into an intermediate SWIG-C wrapped type:

In [16]: yarp.VOCAB_AMP_CURRENT
Out[16]: 7693153

In [17]: kd.VOCAB_CC_ACT
Out[17]: <Swig Object of type 'yarp::conf::vocab32_t *' at 0x7ff2fcf03630>

My suspicion is that SWIG is provided with the full definition of yarp::os::createVocab, hence it knows what to do (or perhaps it's the yarp::conf::vocab32_t?).

PeterBowman commented 3 years ago

My suspicion is that SWIG is provided with the full definition of yarp::os::createVocab, hence it knows what to do (or perhaps it's the yarp::conf::vocab32_t?).

Looks like it's the typedef as switching from yarp::conf::vocab32_t to plain (and arch-dependent...) int did the trick: https://github.com/roboticslab-uc3m/kinematics-dynamics/commit/9f85cff79681d2ee3952ed718717c765e3c67776. I hope that someone comes up with something better in the future.