roboticslab-uc3m / yarp-devices

A place for YARP devices
https://robots.uc3m.es/yarp-devices/
9 stars 7 forks source link

Clean CAN raw subdevice interfaces #211

Closed PeterBowman closed 4 years ago

PeterBowman commented 5 years ago

These devices are wrapped by CanBusControlboard (CBC):

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

https://github.com/roboticslab-uc3m/yarp-devices/blob/d747fa641e798d3a606e6aecf25f04bcbdff96d4/libraries/YarpPlugins/CuiAbsolute/CuiAbsolute.hpp#L43-L45

Ideas:

PeterBowman commented 5 years ago

I might have discovered a massive pitfall in the way CanBusControlboard forwards calls to raw subdevices, see https://github.com/roboticslab-uc3m/yarp-devices/issues/214#issuecomment-494901096. 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 commented 5 years ago

Investigate:

PeterBowman commented 5 years ago

Current workaround (unstable branch): https://github.com/roboticslab-uc3m/yarp-devices/commit/0c64a1ddb03dd5d1a53be478211bbabb7e631f6d, see motorIds class member.

PeterBowman commented 5 years ago

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 commented 5 years ago

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

Another scenario regarding IPositionControl::checkMotionDone was described at https://github.com/roboticslab-uc3m/yarp-devices/issues/214#issuecomment-494901096.

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

PeterBowman commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

Perhaps the absolute encoders could be enabled by the adequate YARP interface? Issue https://github.com/roboticslab-uc3m/yarp-devices/issues/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 commented 4 years ago

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

PeterBowman commented 4 years ago

Cui encoders ready at https://github.com/roboticslab-uc3m/yarp-devices/commit/a123bc981bbb7153b79874621065603bb6fbd3e5. Currently no data is requested from the relative encoders, this will be done at https://github.com/roboticslab-uc3m/yarp-devices/issues/232 via TPDO configuration.

PeterBowman commented 4 years ago

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 commented 4 years ago

Essentially https://github.com/roboticslab-uc3m/yarp-devices/issues/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 commented 4 years ago

Good catch! Yes, https://github.com/roboticslab-uc3m/yarp-devices/issues/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 commented 4 years ago

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

PeterBowman commented 4 years ago

Summary & roadmap:

Currently working on bolded entries.

PeterBowman commented 4 years ago

Device-side ready at https://github.com/roboticslab-uc3m/yarp-devices/commit/cfd8d88e8555b8fd399e4ab80db95df9082e138f, the Lacquey firmware update is targeted at https://github.com/roboticslab-uc3m/yarp-devices/issues/235.