moveit / moveit_task_constructor

A hierarchical multi-stage manipulation planner
https://moveit.github.io/moveit_task_constructor
BSD 3-Clause "New" or "Revised" License
176 stars 150 forks source link

rviz interaction #66

Open rhaschke opened 5 years ago

rhaschke commented 5 years ago

I'm struggling with some design decisions.

  1. Relationship of rviz Panel and Display As discussed at ROSCon, having multiple displays and a unique panel poses some challenges:

    1. Do we want several displays to visualize their (received) solution in parallel? I think, yes. But it might be confusing.
    2. If they do so, which (yellow) feedback should be displayed in the panel to mark the currently visualized part? From all of them in parallel? This probably will be confusing.
    3. When the user interacts with the panel and navigates solutions, I suggest to hide/block all other displays to avoid confusion. Hence, in this case there is a unique active solution. The interaction "ends", when there is no solution selected anymore. Agreed?
    4. If the interaction ends, the solution will not be shown anymore (also see #65).
    5. If there is no interaction "blocking" other displays, they will show their solution. If this replay is finished, the visualization automatically vanishes. Agreed?
    6. What about the solution slider? I suggest to have only a single slider, embedded into the panel, which is used to navigate the panel's active solution in time. However, this requires the panel to be open in order to have the slider.
  2. Visualization / Editing of stage properties

    1. There are/will be two modes of operation for the panel: a) Monitoring a remote planning process via the topics */description, */solution, */statistics and b) Editing/Executing/Monitoring an interactively configured task, running locally in rviz. Do we want editing for both of them? IMHO, for monitoring a remote process, a "back channel" to edit it's stage properties is not needed and eventually will interfere with an already running planning process. In local mode, editing, of course, is mandatory.
    2. In local mode, I think of custom rviz::Properties to e.g. provide an interactive marker to adjust a PoseStamped MTC property. To this end, we will need an rviz::PropertyFactory, which creates rviz properties from MTC properties.
    3. For remote mode, https://github.com/ubi-agni/moveit_task_constructor/tree/property-edit provides a basic, read-only display of properties received from the remote process via strings in */description topic. Obviously the purely string-based display is not suitable for complex hierarchical properties like geometry_msgs/PoseStamped or moveit_msgs/Constraints. Currently this string serialization simply uses the stream operator, which is provided for most basic types as well as for all ROS msg types. For ROS msgs the output format is YAML. For example, Constraints look like this:

      name: joint_constraints[] position_constraints[] orientation_constraints[] visibility_constraints[]

      Hence, we could device a generic YAML -> rviz::Property converter to display all elements hierarchically. I have considered two alternatives to this:

      • Working on python integration anyway, I have a solution which uses YAML serialization of Python in C++ as well (calling the Python interpreter from C++). However, I consider this as nasty, introducing the boost::python dependency to core.
      • Using ROS msg serialization we could encode and transmit the ROS msg in binary format. However, this will render the */description topic as not human readable. On the rviz side, however we could use the same rviz::PropertyFactory used for local properties, to also display remote ones. Which solution is preferred?
  3. Implicit Properties Although properties need to be declared in stages, currently it's possible to set non-declared properties as well: Stage::setProperty(name, value) or Stage::properties().set(name, value), which implicitly declares the new property. If a property with that name already exists, a warning is issued. Question: Should we forbid this? In the past, we discussed to explicitly declare properties (and their propagation from parent -> child, or via interfaces).

v4hn commented 5 years ago

Do we want several displays to visualize their (received) solution in parallel?

Yes.

If they do so, which (yellow) feedback should be displayed in the panel to mark the currently visualized part? From all of them in parallel? This probably will be confusing.

I would still propose to show it for all. I think it would be more confusing/feel buggy to show it only for one.

When the user interacts with the panel and navigates solutions, I suggest to hide/block all other displays to avoid confusion. Hence, in this case there is a unique active solution. The interaction "ends", when there is no solution selected anymore.

Yes.

If the interaction ends, the solution will not be shown anymore (also see #65).

Yes. Interaction should only end when the user disables "interaction"/"introspection" mode explicitly, e.g., as proposed in #65 . Until then, the solution should still be displayed, either last state or in a loop, probably together with the slider.

If there is no interaction "blocking" other displays, they will show their solution. If this replay is finished, the visualization automatically vanishes. Agreed?

This probably makes sense, although it can feel very weird to have the whole robot (in one of my setups together with some big walls around) suddenly disappear at the end of the trajectory.

What about the solution slider? I suggest to have only a single slider, embedded into the panel

I agree with one unique slider for introspection through the panel. I'm undecided about embedding it into the panel. Yes, there is tight coupling between these panels as described here. But with more complex panels, you forbid the user to decide on their layout. Especially for the slider it can make sense to set it wider than the MTC panel for better precision with the mouse.

There is no real need to slide through automatically displayed solutions when you can still inspect them when needed (as long as they are highlighted in the panel when visualized automatically).

Implicit Properties Should we forbid this?

Imho yes. This is actually a TODO I added for myself during WMD to robustify against user errors. The question remains how to handle Containers here.

Visualization / Editing of stage properties There are/will be two modes of operation for the panel

I still believe RViz should visualize only and not do planning itself. I am aware that you setup a lot of the code in the RViz plugins to support this though. My reasoning is that all projects (no matter how you build/design/create them) should still work the very same way without RViz running and we currently have no way to do this when tasks are created in RViz.

Personally, I would prefer an external unit (e.g., a move_group capability) and a topic/service-based protocol to modify tasks. I also had user requests for this. Let's talk about this more in person.

Please call me once you read through this. :)

v4hn commented 5 years ago

Visualization / Editing of stage properties [Message serialization/parsing]

I'm not sure whether you're aware of http://wiki.ros.org/ros_type_introspection I believe this might provide an easy way out for us, but I did not investigate further yet.

rhaschke commented 5 years ago

Implicit Properties: Should we forbid them?

Imho yes. The question remains how to handle Containers here.

It's not only containers, but also interfaces, which pose a problem ;-) My idea was to have a boolean flag in PropertyMap which states whether implicit properties are allowed or not. Stages would disable them, containers and interfaces would enable them.

For containers, you originally voted for explicit declarations as well, didn't you? IMHO this still becomes too verbose.

rhaschke commented 5 years ago

I'm not sure whether you're aware of http://wiki.ros.org/ros_type_introspection

No, I wasn't. Might be a solution. However, this would mean that data will be transmitted binary, i.e. not human-readable, while the rest of description is human-readable.

rhaschke commented 5 years ago

I looked into http://wiki.ros.org/ros_type_introspection. It's only one-way. There seems no support to serialize a message and thus update an mtc::Property after editing in rviz. Also, the library doesn't lookup ROS msg definitions (e.g. via roslib::getPath()). Rather one, needs to provide the full-fledged C++ msg definition, which contains all sub message definitions. I think this is also required for consistency. Hence, additionally to the serialized messages, we would need to publish all message definitions for ROS msg properties.

rhaschke commented 5 years ago

Alternatively, YAML allows two-way operation. Serialization (available in C++ and Python) allows to easily publish / export properties from C++ code. Via YAML deserialization (available only in Python), we can convert YAML back to the ROS msg C++ type and feed it into a property. However, as pointed out above, this introduces a dependency to boost::python for the rviz visualization.

v4hn commented 3 years ago

I believe this concept issue can be closed these days. Things are more concrete by now. Let's create new issues for open points as needed.

rhaschke commented 3 years ago

Please keep this. I didn't find the time to work on the rviz display yet and this issue has some important notes for me.