search

FirefoxMetzger / scikit-bot

Robotics in Python
Apache License 2.0
13 stars 2 forks source link

Speak the Ignition discovery protocol #5

Open FirefoxMetzger opened 3 years ago

FirefoxMetzger commented 3 years ago

Ignition uses ign-transport to communicate between different parts of the library, and we can introspect this communication using ropy.Subscriber.

Under the hood, this uses a mix of zmq and the ignition CLI (via subprocess) to make itself known to ignition and to start receiving messages. The main reason why this isn't implemented cleaner is that I didn't find documentation on how topic discovery works in ignition. However, there appears to be some documentation (living here) that describes how ignition passes messages around to discover topics.

A great addition would be to implement this discovery protocol here, so that we can cleanly interface with Ignition.

Possible related issue: https://github.com/ignitionrobotics/ign-transport/issues/225

mjcarroll commented 3 years ago

Great work on ropy so far!

One thought is that we could either expose a C API for doing a CPython-style wrapper. Alternatively, ROS2 has been moving forward with using pybind11 as a wrapper.

Adding a python api natively to transport could help reduce some wheel reinvention.

FirefoxMetzger commented 3 years ago

Great work on ropy so far!

Thanks! Glad to hear you like it 😄

Adding a python api natively to transport could help reduce some wheel reinvention.

I agree that it would be very nice to not have to reinvent the wheel unnecessarily. What I am less clear about is how to accomplish that. I seem to just see obstacles everywhere. Here are some thoughts:

  1. The current transport API only offers callback-based subscriptions, which voids any chance of determinism. This is fine for most cases, but if your aim is determinism and reproducible simulations then callbacks are a no-go. You need something like an await-able call to keep the simulation in sync and account for network/transport delays after each simulation step. ropy.ignition.Subscriber.recv(blocking=True) does just that. Here is a related comment and the comments above are partly on this topic. Unfortunately, this isn't possible via the current ign-transport API, but I guess it could be added? However, if you combine gym-ignition (to control the main loop) and ropy (for message passing) you can do just that.

On the topic of generated bindings (swig and to some extent pybind11) there are two drawbacks:

  1. If you use a wrapper you are missing out on all the language-specific features. For example, ropy.ignition.Subscriber implements a context manager, so you can use with ign.Subscriber(...) as topic: and have guaranteed cleanup. The same QoL applies to the interface in general. The generated API will always have a C++ feel with signatures that may be unintuitive in a language that isn't C++, cryptic error messages, and/or bad exception handling. gym-ignition (a really awesome library) is a good example here (uses swig bindings around a C++ API). You end up having to do something like assert model.to_gazebo().reset_base_world_linear_velocity(scenario_core.Array3d(velocity.tolist())) for something that should really be model.base_velocity = velocity (and fails with an Exception instead of True/False) in python.
  2. Another big point is documentation. If you come from python (in particular scipy/pydata), you are quite pampered on this point. Documentation is simply amazing. Lack of documentation aside (this could be fixed by simply writing it), documentation for C++ doesn't always translate well to the language of the wrapper. OpenCV is a nice example. The docs are primarily written for C++, and you have to infer the python syntax from it (though this improved in the recent past). Not exactly beginner-friendly.

Another option is to write a python wrapper by hand with meaningful signatures and docstrings, which then calls the C++ API under the hood. Many big python libraries do this (pytorch/torch, tensorflow, pyzmq, numpy, scipy) and this effectively deals with the two drawbacks of an automatically wrapped library. In general, this is a very nice approach and if Ignition goes down that road I'll probably end up helping out in some form. For ign-transport specifically, I think it may be easier to speak just speak the same protocol on the wire and use I/O as the natural boundary between languages. It just seems like less work given that we have pyzmq and betterproto which make communicating with Ignition quite pythonic already. For this to work, what is missing is node discovery, but that is mainly due to a lack of understanding on my part.