Backend features

[beverlylytle]

What type of change is this?

• Bug fix in a backwards-compatible manner.
• New feature in a backwards-compatible manner.
• Breaking change: bug fix or new feature that involve incompatible API changes.
• Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• I added a line to the CHANGELOG.rst file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
• I ran all tests on my computer and it's all green (i.e. invoke test).
• I ran lint on my computer and there are no errors (i.e. invoke lint) .
• I added new functions/classes and made them available on a second-level import, e.g. compas_fab.robots.Configuration.
• I have added tests that prove my fix is effective or that my feature works.
• I have added necessary documentation (if appropriate)

[beverlylytle]

@gonzalocasas here is a draft of the backend refactor (clearly not finished yet). Which bits and pieces do you like? And which are headed in the wrong direction?

[beverlylytle]

To give some context to the ClientManager, it could be used something like

with ClientManger(['ros', 'vrep']) as manager:
    robot = manager.ros.load_robot()
    manager.vrep.is_connected

[romanarust]

To give some context to the ClientManager, it could be used something like

with ClientManger(['ros', 'vrep']) as manager:
    robot = manager.ros.load_robot()
    manager.vrep.is_connected

Maybe we should also consider the other way around. E.g. we could load a compas_fab.Robot from a client, but in some cases, it could also be beneficial to create a client from a compas_fab.Robot. (Especially if we work with multiple clients at the same time, or if we need to retrieve information from the robot)

with ClientManager(['pybullet']).from_robot(robot) as manager:
    manager.pybullet.create_collision_map()

[gonzalocasas]

with ClientManager(['pybullet']).from_robot(robot) as manager:
    manager.pybullet.create_collision_map()

While the ClientManager is a nice construct, I am not entirely sure I understand its application.
From a user's side, what is exactly the benefit of it considering these two snippets?

with ClientManger(['ros', 'vrep']) as manager:
    robot = manager.ros.load_robot()
    print(manager.vrep.is_connected)

vs

with RosClient() as ros:
    with VrepClient() as vrep:
        robot = ros.load_robot()
        print(vrep.is_connected)

[beverlylytle]

The point of the ClientManager is to gracefully allocated and deallocate resources when you have multiple context managers. The issue that could arise in your second examples is that the __enter__ method could fail for VrepClient and then the __exit__ method of RosClient wouldn't necessarily be called, as far as I understand. I also thought it would be a nicer user experience to not have to write so many nested levels.

[gonzalocasas]

The point of the ClientManager is to gracefully allocated and deallocate resources when you have multiple context managers. The issue that could arise in your second examples is that the __enter__ method could fail for VrepClient and then the __exit__ method of RosClient wouldn't necessarily be called, as far as I understand. I also thought it would be a nicer user experience to not have to write so many nested levels.

Definitely agree on the nesting! On the other point, I believe __exit__ will also be called in that case.

[beverlylytle]

I guess you are right about the call to __exit__: https://docs.python.org/3/reference/compound_stmts.html#the-with-statement
And while one could write:
with RosClient() as ros, VrepClient() as vrep:
I still feel that the ClientManager could be useful. Say, if one wanted to add a collision mesh, there could be some sort of global method which calls the appropriate method for each client.

[gonzalocasas]

I still feel that the ClientManager could be useful. Say, if one wanted to add a collision mesh, there could be some sort of global method which calls the appropriate method for each client.

That is a good argument! I believe this is/will be relevant for @yijiangh 's Pychoreo integration, since there was the need to simultaneously maintain the planning scene in both ROS and Pybullet.

[gonzalocasas]

This is a pretty big PR, and in general terms, is looking really good! 👏 Great stuff!

I added a bunch of comments, some are important to address, in particular I feel like adding docs about this is fundamental. The other important issue to resolve is the whole ClientManager/contextlib2 story.

Since I added comments at different times, I'm not entirely sure they all read well linearly, so, let me know if stuff is unclear.

[gonzalocasas]

As discussed, the travis fail is totally unrelated to this pull request, so, from my side I think this is good to go!