Python bindings for IDL-generated services

[PeterBowman]

I have been recently fiddling with Python bindings in the context of YARP+IDL. Since I see no related discussion nor code examples devoted for this matter, I wanted to post here my results and findings in case the quirks described along these lines are useful for someone, or there is a better way to achieve the same result.

So, my goal is to create a compiled library that encapsulates a Thrift service conveniently translated into C++ code via yarpidl_thrift through YARP's CMake utilities. See speech.thrift and the associated CMakeLists.txt. I'm creating a CMake SHARED target for reuse by other local targets, or to be exported and consumed in other CMake projects. As a result, this library comprehends the .h header I want to expose as a Python module via SWIG, and the compiled .so library against which the glue code will be linked.

Speaking of the bindings themselves, I pretty much replicated the same CMake code and structure present in the YARP repository: bindings tree. It is interesting to take a closer look into the .i file in which a roboticslab::TextToSpeechIDL service is defined:

%module "roboticslab_speech"

// enable polymorphism in Python-derived classes
%module(directors="1") "roboticslab_speech"
%feature("director") roboticslab::TextToSpeechIDL;

// correctly handle std::int16_t and std::vector<std::string> (in help() method)
%include "stdint.i"
%include "std_string.i"
%include "std_vector.i"

// avoid issues with some macros, e.g. YARP_os_API, YARP_DEPRECATED_MSG
%include "yarp/conf/api.h"
%include "yarp/os/api.h"
%include "yarp/os/Type.h" // avoid SWIG memory leaks due to a missing destructor

// we need this to preserve the inheritance chain in Python: TextToSpeechIDL < Wire < PortReader
%import(module="yarp") "yarp/os/PortReader.h"
%import(module="yarp") "yarp/os/Wire.h"

%{
#include "yarp/os/Type.h" // incomplete type due to a forward declaration in PortReader.h
#include "TextToSpeechIDL.h"
%}

%include "TextToSpeechIDL.h"

We see two mandatory standard %includes that my SpeechIDL.h interface requires, this will vary across projects and is mostly relevant on runtime. More interestingly, I noticed SWIG itself will fail without the remaining lines:

• The Thrift-generated interface is a C++ class named SpeechIDL which inherits from yarp::os::Wire, and this in turn is derived from yarp::os::PortReader. SWIG will complain that nothing is known about the base class (Wire) unless it is instructed to look into its definition, and similarly it needs to know about its parent PortReader as well. Note these two classes are abstract. The two following lines allow SWIG to correctly deduce the inheritance chain of SpeechIDL and relate it to the YARP Python module (there will be a import yarp in the resulting .py file, and SpeechIDL will be marked as a descendant of yarp.Wire):

  %import(module="yarp") "yarp/os/PortReader.h"
  %import(module="yarp") "yarp/os/Wire.h"

• A more subtle bug relates to the dynamic dispatch of polymorphic calls across the inheritance chain when a server is implemented through deriving the IDL-generated class. The read method defined in the C++ wrapper code, i.e. the one which forwards remote calls to the local implementation of this IDL service, is not aware of derived classes in Python code unless the following lines are added:

  %module(directors="1") "roboticslab_speech"
  %feature("director") roboticslab::TextToSpeechIDL;

• Moreover, SWIG has some trouble parsing the PortReader base class because of certain macros. I noticed that YARP_os_API (in PortReader.h) and YARP_DEPRECATED_MSG (in ConnectionReader.h) lead to cryptic errors, easily hacked around with these additional includes (along with a memory leak we'd get warned about on runtime):

  %include "yarp/conf/api.h"
  %include "yarp/os/api.h"
  %include "yarp/os/Type.h" // avoid SWIG memory leaks due to a missing destructor

• Lastly, there is a forward declaration of yarp::os::Type in PortReader.h. It is used as a return type in getReadType(), and the compiler complains about it while building the generated wrapper code. We need to include its header explicitly.

And that's all. The remote YARP/Thrift service can be accessed now from Python code as in this example client app. It is also possible to write a server-side implementation.

[traversaro]

fyi @S-Dafarra @GiulioRomualdi @RiccardoGrieco @lrapetti @kouroshD

[PeterBowman]

While it's rather easy to write such a client in Python, I can't see a way to make this work for server responders. I think it's caused by polymorphism not spanning through the C++/Python glue code barrier. Even if I define a read method for my IDL class, no attach method (either via RpcServer or Wirelink) seems to make incoming requests be forwarded as expected: all messages go to the auto-generated C++ implementation (suitable for clients, but not for servers).

[PeterBowman]

Fixed! I had to tell SWIG I want to enable polymorphism via %module(directors="1") and %feature("director") (docs). I have updated the above .i code and description accordingly, also adding an example of server-side implementation through inheritance in Python.

[drdanz]

Hi @PeterBowman,
I've been working a bit on this as well, some time ago, you might want to have a look at https://github.com/drdanz/test_thrift_lua and https://github.com/drdanz/test_thrift_py, perhaps you could find something useful there.

One of my goal for YARP 3.6 would be to have the .i file autogenerated by the thrift compiler, would you like to help me defining how this file should be generated?

I would also like to generate a CMakeLists.txt file or a CMake macro to generate a library and the bindings.

[PeterBowman]

Hi, @drdanz. The Lua repo is probably private, I can't access it. From the Python one, I noticed you import all os/dev/sig headers. I found it error-prone and resorted to including only the headers I really need (as in my above example .i code), albeit in a sort of hackish way (because of SWIG complaining on very specific macros, see also my inline comments).

Do we really need the Thrift compiler to autogenerate the .i? Perhaps it is simple enough for CMake code to take care of this, as you mostly need the name of the service and a fair amount of boilerplate.

[drdanz]

The Lua repo is probably private, I can't access it.

Sorry, it is now public in case you want to have a look.

Do we really need the Thrift compiler to autogenerate the .i? Perhaps it is simple enough for CMake code to take care of this, as you mostly need the name of the service and a fair amount of boilerplate.

Yes, this is probably a better idea if we don't need any knowledge of the internals of the thrift structures.

From the Python one, I noticed you import all os/dev/sig headers. I found it error-prone and resorted to including only the headers I really need (as in my above example .i code), albeit in a sort of hackish way (because of SWIG complaining on very specific macros, see also my inline comments).

I think I imported everything, because this was a quick test to get the bindings working... The main problem could be that, if you want to make this generic, you cannot know in advance what is used by the module.

%include "yarp/os/Type.h" // avoid SWIG memory leaks due to a missing destructor

%{
#include "yarp/os/Type.h" // incomplete type due to a forward declaration in PortReader.h
#include "TextToSpeechIDL.h"
%}

This looks like a bug in the generated file... Would it make sense to include the <yarp/os/Type.h> file in the files generated by thrift for services?

[PeterBowman]

The main problem could be that, if you want to make this generic, you cannot know in advance what is used by the module.

Agreed, but at least for (Python) services only you should just need to include yarp/os/Wire.h and yarp/os/PortReader.h (because of the involved inheritance and abstraction layers) in this autogenerated .i from the SWIG's perspective. Stressing on that last point:

Would it make sense to include the <yarp/os/Type.h> file in the files generated by thrift for services?

Please note my bindings generation occurs at a different stage. Prior to that, I have autogenerated the .h/.cpp files via Thrift and then consolidated them into a shared library. SWIG only needs the .h to perform the translation, and the SWIG-generated wrapper is later linked against my Thrift-generated lib. In other words, the missing <yarp/os/Type.h> is a compiler issue that occurs while building the SWIG wrapper, not the Thrift-generated library. Unless you mean to add <yarp/os/Type.h> into the autogenerated .h, but that's against include-what-you-use.