Clean CAN raw subdevice interfaces

[PeterBowman]

These devices are wrapped by CanBusControlboard (CBC):

• CuiAbsolute
• FakeJoint
• LacqueyFetch
• TechnosoftIpos
• TextilesHand

It just feels wrong to have CuiAbsolute implement several YARP motor interfaces:

yarp-devices/libraries/YarpPlugins/CuiAbsolute/CuiAbsolute.hpp

Lines 43 to 45 in d747fa6

	class CuiAbsolute : public yarp::dev::DeviceDriver, public yarp::dev::IControlLimits2Raw, public yarp::dev::IControlMode2Raw, public yarp::dev::IEncodersTimedRaw,
	public yarp::dev::IPositionControl2Raw, public yarp::dev::IPositionDirectRaw, public yarp::dev::IVelocityControl2Raw, public yarp::dev::ITorqueControlRaw,
	public ICanBusSharer, public ICuiAbsolute, public yarp::dev::IInteractionModeRaw

Ideas:

• CanBusControlboard should not crash if some of these subdevices don't inherit an unrelated interface.
• Introduce roles, e.g. motor devices (TechnosoftIpos and FakeJoint should derive from IPositionControl, IControlMode and so on), helpers (CuiAbsolute), end-effectors (LacqueyFetch, TextilesHand), boards (FakeControlboard)...
  • Mind the Dextra hand device: Develop serial Dextra device #176.
  • Perhaps a new omnibus interface could come in handy, e.g. make TechnosoftIpos derive from IMotorDevice, which in turn derives from standard YARP interfaces and ICanBusSharer, but doesn't introduce any new methods by itself.
  • Split CuiAbsolute configuration into a new group within .ini files? Useless interfaces aside, we also don't need to pass a default max vel/acc value nor several others.
• What is IInteractionMode and why do we need it? Drop unused interfaces.

[PeterBowman]

I might have discovered a massive pitfall in the way CanBusControlboard forwards calls to raw subdevices, see #214 (comment). It is imperative to be aware of which joints are actually controlled via YARP ports (look for controlledJoints at ControlBoardWrapper.cpp). The YARP wrapper clips our list of nodes (iPOS+encoders+grippers+...) to the number specified in the joints property, see launchManipulation.ini. Instead of iterating the full vector of nodes like in here, CanBusControlboard should pick the correct ids, no more and no less than required.

[PeterBowman]

Investigate:

• ImplementPositionControl::positionMove
• ControlBoardHelper.hpp
• ControlBoardHelper.cpp

[PeterBowman]

Current workaround (unstable branch): 0c64a1d, see motorIds class member.

[PeterBowman]

Current workaround (unstable branch): 0c64a1d, see motorIds class member.

Every time we setRemoteVariable, all seven iPOS nodes on that CAN bus are being talked to (6 on the arm + 1 on the head). Then, we set control modes on the 6 arm joints only.

[PeterBowman]

I might have discovered a massive pitfall in the way CanBusControlboard forwards calls to raw subdevices

Another scenario regarding IPositionControl::checkMotionDone was described at #214 (comment).

Also, keep in mind that the vector of nodes includes non-iPOS devices, e.g. encoders, grabbers, etc.

[PeterBowman]

I'd fancy a .ini file structure that doesn't enforce properties for nodes which are never going to use them, e.g. maxVels in CuiAbsolute. Also, at some point in the future we might develop a motor drive node (analogous to TechnosoftIpos) which expects a different set of properties. Therefore, a plausible solution is to segregate those properties per CAN node type and place them in separate groups within the .ini file, e.g. [TechnosoftIpos_2], [CuiAbsolute_102] etc.. The CanBusControlboard wrapper should not need to parse and pass them along to wrapped subdevices, just take care of loading the .ini group on a per-subdevice initialization basis.

[PeterBowman]

Following the .ini rework proposal, it might be wise to parse an additional per-device type group (e.g. all Cuis, all iPOS nodes, etc.) to gather common configuration parameters within (e.g. PT/PVT mode period).

Besides, I'd like to take advantage of the .ini file transclusion feature: let's put all iPOS/Cuis/grippers on their own separate .ini file and transclude them if needed from the parent .ini storing the configuration of the CanBusControlboard master device.

[PeterBowman]

@jgvictores suggested we could start a TechnosoftIposWithCui device that inherits from TechnosoftIpos and adds the encoder functionality. I'm quite positive about that, although composition makes a little more sense to me (instantiate a CuiAbsolute within TechnosoftIpos). This will render the role separation mostly pointless if we consider that grippers are less-sophisticated actuators (which needs not to be necessarily true).

[PeterBowman]

Perhaps the absolute encoders could be enabled by the adequate YARP interface? Issue #157 suggests yarp::dev::IMotorEncoders could fit the relative encoders (uses encoder counts as input/output parameters). So, should yarp::dev::IEncoders be aimed for use with absolute encoders (and fall back to relative encs)?

[PeterBowman]

Had a brief and pleasant face-to-face with @PeterBowman, some new ideas have been laid out:

• YARP controlboard wrappers should not need to know a thing about internal CAN-to-motor-id associations, this mapping shall be handled by the CanBusControlboard internally and only said motor ids will be exposed. That is, additional CAN nodes which do not perform any movement, for instance the relative/absolute encoders, do not participate in the motor id assignment on the YARP-exposed side. This is a strong argument in favor of embedding everything encoder-related within the motor device, be it the TechnosoftIpos or some kind of gripper we do not have yet. To sum up, CAN node assignment intricacies are orquestated by the CanBusControlboard, whereas internal implementation, including the aforementioned class composition strategy, is meant to be cared for by the raw subdevices.

• There should be nothing special about allowing more than one motor id par CAN node id (Map controlled joints to motor ids in raw subdevices #171). Some Technosoft drives might already work this way (ref). In consequence, motor devices and gripper-like devices would be treated equally in this regard.

• Motor-id-to-YARP-id association regarding gripper devices might benefit from the usual controlboardwrapper mechanisms. We "split" all four TEO's CAN buses in a way that a separate YARP port is opened for the head and trunk body parts. However, the same port is shared by motor drives and grippers (LacqueyFetch, TextilesHand, and the WIP Dextra device (Develop CAN Dextra device #227)). I would like to instantiate separate YARP wrappers for the gripper nodes; then, apart from the usual motor interfaces, additional configuration could be shared on the YARP network with the remote variables interface: Expose actuator commands in gripper-like subdevices #228.

[PeterBowman]

Cui encoders ready at a123bc9. Currently no data is requested from the relative encoders, this will be done at #232 via TPDO configuration.

[PeterBowman]

Next (and probably last) big step here: implement some sort of LUT within CanBusControlboard to map exposed joint indexes (e.g. like the j in positionMove(j, ref)) to motor ids controlled by raw subdevices. For instance, if we open three raw devices A, B and C, where A controls two joints, B three, and C one joint respectively, then CanBusControlboard::positionMove(4) should map to B::positionMoveRaw(2). For comparison, check the subdevice mechanism at ControlBoardWrapper.

[jgvictores]

Essentially #171 originally called "Maybe raw index 0 wasn't a great idea"?
Lots has changed, by the main idea of the issue is that the first argument of "(e.g. like the j in positionMoveRaw(j, ref))" (note the Raw) can be exploited to pass useful info.

[PeterBowman]

Good catch! Yes, #171 is definitely the right place for speaking about device mapping. I'll update the status in that ticket.

So, the last thing left to do here: similarly to the process undergone at the CuiAbsolute device, let's remove unused interfaces in LacqueryFetch, TextilesHand and so on. The following is still valid:

CanBusControlboard should not crash if some of these subdevices don't inherit an unrelated interface.

What is IInteractionMode and why do we need it? Drop unused interfaces.

[PeterBowman]

BTW teo-developer-manual/cui-absolute-values.md should die.

[PeterBowman]

Summary & roadmap:

• CuiAbsolute: done at a123bc9.

• DextraCanControlboard: inherits from DextraRawControlboardLib, which takes care of properly implementing motor interfaces.

• FakeJoint: I'm going to rewrite this, see Migrate FakeControlboard's JMC to FakeJoint #192.

• LacqueyFetch: most interfaces will be replaced by yarp::dev::IPWMControl.

• TechnosoftIpos: just going to remove the IInteractionMode implementation.

• TextilesHand: will undergo some major changes at Promote TextilesHand to a control board device #234, not touching this right now.

Currently working on bolded entries.

[PeterBowman]

Device-side ready at cfd8d88, the Lacquey firmware update is targeted at #235.