Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update REP-I0001 with implementation-related changes/clarifications #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update REP-I0001 with implementation-related changes/clarifications #13

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
378783f
Update REP-I0001 with implementation-related changes/clarifications
JeremyZoss Jun 9, 2015
af8a3ca
Clarification updates, per #13 review comments
JeremyZoss Jun 12, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 35 additions & 35 deletions rep-I0001.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ The following definitions are used throughout:
Motivation
==========

The first version of the motion/status interface (loosely documented in code and wikis) worked well for single arm manipulators. Multi-arm (asynchronous or synchronous) was not supported in a generic manor that could be used across all platforms [#discuss]_. Some attempts were made to support multiple arms using the existing interface, but these often required multi-arm joint states which could exceed the max number of joints and did not allow for mixed multi-arm/single-arm motion. The intent of this REP is to define a new set of simple messages[#simp_msg]_ for multi-arm control. These messages will replace existing motion control message (they will be deprecated). The approach taken in the REP is one that creates a new set of clients for robot motion. The underlying simple message libraries will remain unchanged (except for the addition of new messages).
The first version of the motion/status interface (loosely documented in code and wikis) worked well for single arm manipulators. Multi-arm (asynchronous or synchronous) was not supported in a generic manner that could be used across all platforms [#discuss]_. Some attempts were made to support multiple arms using the existing interface, but these often required multi-arm joint states which could exceed the max number of joints and did not allow for mixed multi-arm/single-arm motion. The intent of this REP is to define a new set of simple messages[#simp_msg]_ for multi-arm control. These messages will replace existing motion control message (they will be deprecated). The approach taken in the REP is one that creates a new set of clients for robot motion. The underlying simple message libraries will remain unchanged (except for the addition of new messages).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space between messages and [#simp_msg]_ (in "new set of simple messages[#simp_msg]_ for multi-arm control").


Requirements
Expand All @@ -62,8 +62,7 @@ The industrial robot controller motion interface shall meet the following requir

Design Assumptions
=========
The following assumptions are inherent in the client/server design:
* All joints (regardless of group) shall be uniquely named.
None at this time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify why you removed that assumption? I can see why it would make sense to remove the regardless of group bit, but now it allows for multiple joints in the same group being identically named. Is that intentional?



Simple Message Structures
Expand Down Expand Up @@ -109,9 +108,11 @@ The dynamic joint state is meant to mimic both the ROS JointState and FollowJoin
accelerations[]
effort[]
position_desired[]
position_errors[]
position_error[]
velocity_desired[]
velocity_errors[]
velocity_error[]
accel_desired[]
accel_error[]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why revert to singular only here? To get this more in-line with the JointState msg, I'd make all field names singular (ie: position, velocity, acceleration, etc).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message maps to both the JointState and FollowJointTrajectoryFeedback, which eventually encapsulates data using a JointTrajectoryPoint. These messages use different conventions, so there is no "best choice". In the end, I updated error to match FollowJointTrajectoryFeedback, but left the pos/vel/accel naming as it was in the original REP. I'm happy to use something more consistent, if it makes sense.

For what it's worth, the DynamicJointPoint maps only to JointTrajectoryPoint, so using "mostly plural" (effort is singular) to match the ROS message makes sense there. I could buy an argument that we should match everything to JointTrajectoryPoint's naming, for consistency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I think I'd vote for consistency. Let's see whether there are any more opinions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional note for the accel_* fields: why abbreviate here? All other field names are not, so it seems inconsistent to do it for acceleration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For brevity in typing. {shrug}. In reality, these field names don't officially appear anywhere. There are some related constants, comments, and methods in the ROS/Karel code, but they don't adhere strictly to these names either.

effort_desired[]
effort_error[]

Expand All @@ -125,12 +126,13 @@ The dynamic group status is meant to mimic both the ROS-I RobotStatus message.
num_groups: # of motion groups included in this message
group[]: # length of this array must match num_groups
id: control-group ID for use on-controller
mode:
e_stopped:
drives_powered:
motion_possible:
in_motion:
e_stopped:
error_code:
in_error:
in_motion:
mode:
motion_possible:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just asking: do we want to order this like RobotStatus.msg, or keep the order of the simple_message/robot_status.h header? This PR orders it like RobotStatus.msg. The old older was that of the robot_status.h header in simple_message.

Obviously we don't have to keep this in the same order, as we are not bound by backward compatibility issues.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. The linked header and the actual order used in the RobotStatus::load(..) and RobotStatus::unload(..) implementations would seem not to match with each other.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks to me as though everything is consistent, after this update to the REP. Your comments link to rev 12a74a1, but it looks like commit 4405b0d updated the header comments to match the field ordering in load/unload.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yes, you're right. I even fixed that order myself (ros-industrial/industrial_core@4405b0dc). Sorry for the noise.



Communications Method
Expand All @@ -156,27 +158,40 @@ The motion interface can be expressed as four variations:

Node Configuration
---------
In order to support the various methods of control, the motion node must be somewhat dynamic/statically reconfigurable[see current parameters]. The node must be able to support subscriptions to multiple topics (all of the same type) as well as conversion from ROS group organizations to controller organization. This mapping would look similar to the MoveIt controller manager[#ctrl_mgr]_.
The yaml file will contain a list of structures that defines the joint trajectory topics as well as the mapping to the controller.::
In order to support the various methods of control, the motion node must be somewhat dynamic/statically reconfigurable[see current parameters]. The node must be able to support subscriptions to multiple topics (all of the same type) as well as conversion from ROS group organizations to controller organization. This mapping would look similar to the MoveIt controller manager [#ctrl_mgr]_.

As in previous interfaces, the node will be configured using a ROS parameter specifying ROS-to-controller mapping. This parameter will contain a list of structures that define both the topic namespaces as well as the mapping to the controller.::

topic_list:
- name: <topic name>
controller_joint_map:
- group: <controller group#>
ns: <topic namespace>
group: <controller group#>
joints:
- <joint_1>
- <joint_2>
- <joint_N>
- name: <topic name>
- group: <controller group #>
ns: ...

This structure allows users to configure the robot interface to receive motion commands in several different ways:

#. Single Topic
all groups use same namespace (e.g. `ns: ""`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In reStructuredText you need double back-ticks for verbatim text.

#. Separate Topics
use group-specific namespaces
#. Mixed Mode
joint map may contain multiple entries for the same group, in different namespaces. This enables separate multi-arm topics for: left, right, both.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to avoid referring to left and right as groups, as that is only relevant when dealing with dual-arm manipulators, while multi-group motion is used in many more setups. Perhaps just adding a "for example" to the sentence.


The interface node will always listen for motion commands on the same topic as the current interface (`joint_path_command`), inside the specified namespace for each group.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see earlier comment about verbatim text.

And all other occurrences later.


State Interface
=========
The robot state interface encapsulates all the data coming FROM the robot controller, including joint position, velocity (if available), effort(if available), position error and general robot status information [#rbt_stat]_. The implementation of the state interface is simpler than the motion interface because it can be generalized to the multi-arm case, where a single arm is just a specific example.
The robot state interface encapsulates all the data coming FROM the robot controller, including joint position, velocity (if available), effort(if available), position error and general robot status information [#rbt_stat]_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing space between effort and (if available).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: don't use capitalisation for emphasis, use *from* (in "coming FROM the robot controller").


The state interface is split into a joint state and robot status interface (although they will utilize the same socket connection, see `Communications Method`_). The split allows joint state feedback to be sent at a higher rate than status information (which should change slowly).
* Joint State - A single controller message is split into N JointState messages.
* Robot Status - A single controller message that contains status information for each arm.

As in the motion interface, users can specify which namespace to use for each robot group. Unlike the motion interface, multiple groups that share a namespace are *always* combined into a single message (regardless of sync vs. async motion). Separate namespaces can be used to receive separate messages for each motion group.

For `JointState` and `JointTrajectoryFeedback` messages, aggregation of multiple robot groups is easily supported through the explicit joint-name list in the message. For `RobotStatus` messages, the status of multiple groups is aggregated into a single message using simple logic and the `TriState` message type: If all groups share the same status value, use that value, otherwise use `TriState::Unknown`. Some fields (such as `in_motion`) may use modified logic (if "any" group in motion...).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps reword paragraph a bit: to which group does "the status of multiple groups is aggregated" refer here? Motion groups on the controller, those defined in the configuration yaml or a mix-and-match of those two?

Ie: what is the 'status of a group' defined in the config yaml that spans joints from two motion groups on the controller (if that is possible)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also: not too sure about the "Some fields may use modified logic". Can 'a group' be said to be in motion if one or more are not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm struggling a bit with the rewording. Perhaps I can explain my intent, and you can suggest more specific areas that could be clarified?

The intent is that "group" always refers to a Robot Motion Group, as defined on the robot controller, and always is directly/uniquely associated with a fixed set of physical motors. The ROS client can use the "mapping rules" defined in the configuration parameter/YAML to publish the joint states on one or more topics ("namespaces").

If each Robot Motion Group is mapped to a separate namespace, the ROS interface looks much like the current single-group case: each topic has messages that contain only joint-state data for a single robot motion group. For a dual-arm case, you might get a /left/joint_states topic with [left_joint_1, ..., left_joint_6] joints and a /right/joint_states topic with [right_joint_1, ..., right_joint_6] joints.

However, if multiple Robot Motion Groups are mapped to the same namespace, the multi-group core is more visible. Rather than publish separate messages for each motion-group on the same topic, the intent is to combine joints from all groups into a single topic. For a dual-arm robot, you might see: /joint_states with [left_joint_1, ..., right_joint_6].

I was trying to highlight that the RobotStatus message is more complex, since it does not allow "lossless" aggregation of multiple RobotStatus messages into a single message. The DynamicGroupState simple message defined above states that the robot will send a separate RobotStatus data-structure for each Motion Group. My suggestion is that we can probably come up with some "common sense" logic rules for aggregation that should apply to the current RobotStatus fields.

Here's an example, for a dual-arm robot, with both Robot Motion Groups mapped to a single namespace:

  • from robot:
    • left: drives_powered: Y, e_stopped: N, in_motion: Y, mode: AUTO
    • right: drives_powered: N, e_stopped: N, in_motion: N, mode: AUTO
  • published to ROS:
    • status: drives_powered: UNKNOWN, e_stopped: N, in_motion: UNKNOWN, mode: AUTO
      Alternatively, we could write the logic for some fields (e.g. in_motion) to return TRUE if any participating Robot Motion Groups were TRUE (instead of UNKNOWN in the mixed case).


.. image:: rep-I0001/state_interface.png

Expand All @@ -185,26 +200,11 @@ Node Configuration
---------
Similar to the motion interface, the state interface will require configuration. The state interface will have to parse messages coming from the robot and convert the date into the desired ROS topics. The level of configuration available on the robot controller will vary, so the messages coming from the controller may be more or less dynamic. The state node, based on configuration, will identify the pertinent information from the robot controller and convert to ROS topics. Additional information will be ignored.

The yaml file will contain a list of structures that defines the joint trajectory/status topics as well as the mapping to the controller. Note, this configuration is very similar to the motion node, with the exception that the state node performs a one-to-one mapping from controller groups to topics. The motion node, in addition to this, can perform a one(topic) to many (groups) mapping.::

topic_list:
- state
group: <controller group#>
- joint
- name: <topic name>
ns: <topic namespace>
joints:
- <joint_1>
- <joint_2>
- <joint_N>
- status
- name: <topic name>
- ns: <topic namespace>
...
The state interface uses the same ROS configuration parameter as the motion node. Robot groups may be mapped to the same namespace (aggregate status messages) or different namespaces (per-group messages), or both. As in the motion interface, the same topics are used within each namespace as are used in the current state interface.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add some example configurations for each of the cases? The "may be mapped to X, Y or both" can be confusing.


Backwards Compatibility
=========
This REP creates a new industrial robot client package that will not be backwards compaptible with the previous version. This means that all servers will have to be rewritten to support this new client. Because this REP does not change the ROS API (except for multi-arm considerations), the previous client/server versions can continue to be used as is. Transition to the client/servers described by this REP can be a gradual process as the capabilities enabled by this new design are required.
This REP creates a new industrial robot client package that may not be backwards compaptible with the previous version. The new Dynamic simple_message types will be used for all data communications with the robot, even in single-arm configurations. This means that all robot servers will have to be rewritten to support the new dynamic message types. This REP attempts to minimize the impact to ROS-side API, so most ROS clients will be unaffected. The new industrial robot client package should continue to support the existing `controller_joint_names` definition, for backwards compatibility. Transition to the client/servers described by this REP can be a gradual process as the capabilities enabled by this new design are required.

If incompatible client/server combinations are ever used, there is little risk of undesirable behavior. Because the simple message base protocol is not changed by this REP, the client/server should recognize the new message types as undefined and return an error reply code.

Expand Down