Problems with HandUnits
While implementing the ShapeHand-skill, we noticed that the interface/structure/implementation of the HandUnits is rather messy and inconsistent. The code appears to have grown over a long time, while it hasn't been properly maintained. This issue should document known problems and open questions for a potential rework (probably with a new HandUnitInterface in this repository):
HandUnitInterface
-
Used naming scheme for hand joints not specified -> inconsistent: There are two naming schemes for the hand joints: The fully qualified names (e.g. "LeftHandIndex") and the names with removed hand-name-prefix (e.g. "Index"). The interface doesn't specify which naming scheme to use (e.g. for
setJointAngels
orgetShapeJointValues
). Therefore, different implementations of the interface work with different naming schemes. The interface should specify a naming scheme (we prefer the shortened names), which should then be used by all its implementations.- ARMAR-7's HandUnit works solely with shortened joint names. ArmarXSimulation's HandUnitDynamicSimulation implementation (used in the ARMAR-7 simulation) works solely with fully qualified names. This makes it impossible to write clean code that works on the robot and in simulation.
-
No proper interface for not position-based "shapes": The interface provides methods (
setShape
,getShapeJointValues
,getShapeNames
) to shape the hand according to predefined shapes. This interface is designed for position-based shapes, as defined by the preshapes (sets of target angles for the hand joints) in the robot.xml. The interface doesn't properly support not position-based, more complex "shapes" (-> "operations"). Therefore, such operations are currently made available as normal shapes by the implementations, which results in inconsistencies due to the differently thought interface. We should discuss if/how the interface should support such operations.- ARMAR-7's HandUnit implements a special-cased "Relax"-shape to stop controlling the joints. This isn't a normal shape defined by some target values, but more of an "operation". Therefore,
getShapeJointValues("Relax")
is not applicable here and is hence not implemented. Thus, this implementation is (due to the lack of support for such "operations" by the interface) inconsistent regarding this shape.
- ARMAR-7's HandUnit implements a special-cased "Relax"-shape to stop controlling the joints. This isn't a normal shape defined by some target values, but more of an "operation". Therefore,
-
Exception handling not specified -> inconsistent: The interface doesn't specify how exceptions should be handled at all. Thus, each implementation handles exceptions differently, sometimes very strangely. The interface should specify how to handle exceptions. (Is there a reason why no Ice-exceptions are used?)
-
setJointForces
: I think @dreher mentioned that at least the naming is bad...? Does the method provide the interface we want/need/can provide (why is this method not implemented by ARMAR-7's HandUnit)? -
No method to get available joints and value ranges: The interface doesn't provide any methods to obtain the controlled joints and their value ranges. This might be needed to write generic code interacting with a HandUnit.
-
Dead code / wrong abstraction-level: There seem to be some dead methods (e.g.
setShapeWithObjectInstance
,sendJointCommands
). Some methods (e.g.describeHandState
) seem to be on a much higher abstraction-level and therefore shouldn't be part of the interface...? We should clarify what the scope of a HandUnit should be.- E.g.
setShapeWithObjectInstance
andsendJointCommands
aren't implemented by ARMAR-6's HandUnit or ARMAR-7's HandUnit.
- E.g.
-
(HandUnitListener/HandUnitObserverInterface: What do we want to do with these?)
ARMAR-7 HandUnit implementation
-
Hacky implementation of "Relax", "Open", and "Close" shapes: The implementation of the special-cased "Relax", "Open" and "Close" through the inherited methods from ethercat/hand/common/AbstractHandUnitControllerWrapper is not very clean. See for example here and above (No proper interface for not position-based "shapes").
-
Robot-specific code in EtherCAT: While the implementation of the HandUnit itself is in ARMAR-7/Realtime, most of the relevant code, including ARMAR-7 specific code, is in EtherCAT. Where should such stuff be implemented in the long term?
-
No warning for invalid joints / joint values: No exceptions are thrown / nothing gets logged when invalid joint names are passed in 'setJointAngles'. Neither is an exception throw / something logged if target positions are outside the allowed range; instead, the targets are just set to the allowed limits.
ArmarXSimulation's HandUnitDynamicSimulation implementation (used in ARMAR-7 simulation)
-
Movement not simulated: This HandUnit (as the KinematicUnit) doesn't simulate joint movement; instead, target positions are reached instantly. Do we want to / can simulate the movement and is it in the scope of the HandUnit (this is probably more of problem for the KinematicUnit which lacks proper simulation)?.
-
"Relax" shape not available: The "Relax" shape is not available because it's only special-cased in the real ARMAR-7 HandUnit and not defined in the robot.xml (see above).
-
Inconsistency in expected joint identifiers:
setJointAngles
only accepts fully qualified joint names like "LeftHandFingers" because the angles are passed directly to the KinematicUnit. However, each time a warning "No corresponding finger joint found for LeftHandIndex" is thrown because a few lines later it expects the short identifiers like "Fingers":setJointAngles
->HandUnitDynamicSimulation::handUnitJointsToRobotJoints
-> warning -
getCurrentJointValues
doesn't work:getCurrentJointValues
returns an empty map (at least in the ARMAR-7 simulation). -
setShape
exception handling: ThesetShape(string shapeName)
-implementation sets the shape whose name partially matches theshapeName
if no such shape is defined (why??? -> wrong abstraction-level?).
General
-
Wrong abstraction-levels: In general, the code is not well-structured, and many things are implemented in weird places. In many cases, abstraction-levels are messed up, and high-level abstractions are implemented (many times very similarly) in low-level code.
- E.g. EtherCAT reimplements preshape loading, which should be the same for all HandUnits and thus be implemented somewhere more general (rather should the base implementation be used/improved).
-
Many overlapping/useless HandUnit implementations: There are different HandUnit implementations all over the place in many different repositories. Many of which have much code duplication or are probably just dead code. In many cases, these are not appropriately structured through inheritance or similar, but are reimplemented everywhere differently.
- RobotAPI/components/units/HandUnit
- RobotAPI/components/units/HandUnitSimulation
- RobotAPI/components/KITHandUnit
- RobotAPI/components/KITProstheticHandUnit
- ArmarXSimulation/components/HandUnitDynamicSimulation
- ARMAR-7/Realtime/units/armar7_unit/units/KITHandUnit
- ARMAR-6/Realtime/units/Armar6Unit/Units/KITHandUnit
- ...