roboticslab-uc3m / yarp-devices

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

Migrate FakeControlboard's JMC to FakeJoint #192

Closed PeterBowman closed 4 years ago

PeterBowman commented 6 years ago

The FakeJoint device lacks several interface implementations (and, probably, does not interpolate trajectories). On the other hand, FakeControlboard can do that, but only on a controlboard-level manner, i.e. joint control is embedded. The point here is to extract each joint controller from that and make FakeControlboard just load dynamically the number of joints/devices requested by the user.

In the long run, I wonder whether we'll need FakeControlboard at all, since CanBusControlboard may load FakeJoints on request and even make them work together with other node devices, either on a real CAN network or with a simulated one (CanBusFake).

jgvictores commented 6 years ago

Totally agree with dropping FakeControlboard in the future.

PeterBowman commented 5 years ago

Idea: leave FakeControlboard as an empty device-launcher shell, move all logic to FakeJoint, and let it be instantiated by CanBusControlboard (it already is) and FakeControlboard. The point being that CanBusControlboard should focus on CAN communications, therefore there would always be a CAN bus device attached to it. Either we make it optional and rename the controlboard device, or keep these two things separate: a universal device launcher that supports CAN devices with some optional fake joints, and a "fake-CAN" controlboard only composed by fake joints.

jgvictores commented 5 years ago

Whichever solution, I guess we are looking for a balance between:

  1. Having fake devices that are useful for debugging
  2. Not having to maintain excessive code

That said, if we really have to hack the CanBusFake a lot in order to emulate the CAN behavior for things to work, and see it is not really worth the effort, we should forget about it, just work with FakeControlboard. However, I believe this is not the case. Instead, CanBusFake can be very minimal, so we could actually just forget about FakeControlboard and work with CanBusControlboard+CanBusFake. I'd see no need for renaming CanBusControlboard as it only switches between CanBus options. Exception: think about including devices that are not integrated via CAN.

PeterBowman commented 5 years ago

Idea: leave FakeControlboard just for its use as a multiplexer (https://github.com/roboticslab-uc3m/yarp-devices/issues/171) of FakeJoints, i.e. assign it the board role (https://github.com/roboticslab-uc3m/yarp-devices/issues/211).

PeterBowman commented 4 years ago

Note that testBasicCartesianControl.cpp opens a FakeControlboard device.

PeterBowman commented 4 years ago

It is a misconception that FakeControlboard is a control board of FakeJoints, and I probably got it wrong from the very beginning. Indeed, the FakeControlboard is a fully fledged simulation of a joint controller that, at the time of writing, accepts position and velocity commands. The YARP counterpart device named fakeMotionControl follows a similar principle but, unless I'm wrong, it does not interpolate intermediate targets as our device does (through time stepping), i.e. all movements are instantaneous.

On the other hand, the FakeJoint does literally nothing. It is a stub implementation of several motor interfaces with the only aim to return a boolean true so that callers perceive a positive result in usual completion checks (if (!p->positionMove(q)) return false;). In contrast, FakeControlboard has been successfully exploited as a lightweight simulator in testBasicCartesianControl, among other valid usage.

I'll have to reconsider my original point. FakeJoint should not die and its purpose must prevail. When a physical driver/motor fails, we replace its device in the CAN bus configuration with this kind of fake "controller". It must return true and, probably, I have to review its treatment of output values (e.g. how should q be treated in getEncoders(q)?). That's as far as it goes regarding internal state.

Then, FakeControlboard or anything similar to it should be kept available, too. I wonder if YarpOpenraveControlboard is a better fit for this case, although it is certainly sure that an additional, heavy dependency (OpenRAVE) makes this device less attractive for trivial usage (cc @jgvictores).

Perhaps current status-quo is worth to be preserved, code cleanup aside. I'm going to implement all raw motor interfaces in FakeJoint as described earlier (mostly bare function body, return true).

jgvictores commented 4 years ago

By original design goals:

Regarding YarpOpenraveControlboard replacing FakeControlboard: IMHO, I'd say there are strong enough drawbacks. You'd need to compile openrave-yarp-plugins (most dependencies served: YARP, YCM, CMake, except for OpenRAVE). Regarding OpenRAVE here (just to test a kinematic solver) :

As I write this, I'll describe how my initial conception of this issue was a terrible idea. My idea was in fact CanBusControlboard replacing FakeControlboard, via: 1) We move the light-weight simulation logic to the FakeJoints, and 2) We use a CanBusFake. Drawbacks: 1) This is undesired, we do not want a dead/dummy driver to emulate it as moved (so we'd have to indicate it to act as "blocked" anyway), and 2) All the CanBusFake calls would also populate the screen too much to actually be useful to test kinematic solvers.

As such, yes, it seems reasonable to maintain the FakeControlboard/FakeJoint coexistence. In any case, if I were forced to eliminate FakeControlboard, the YarpOpenraveControlboard via seems so bloated I would rather promote CanBusControlboard+CanBusFake for this task (which may be due to my ignorance on the current state of CanBusControlboard, which you may consider also over-bloated for the use case).

PeterBowman commented 4 years ago

All the CanBusFake calls would also populate the screen too much to actually be useful to test kinematic solvers.

Those calls are meaningless, therefore I think we can disable all console output in CanBusFake and thus avoid this trouble.

As such, yes, it seems reasonable to maintain the FakeControlboard/FakeJoint coexistence. In any case, if I were forced to eliminate FakeControlboard, the YarpOpenraveControlboard via seems so bloated I would rather promote CanBusControlboard+CanBusFake for this task (which may be due to my ignorance on the current state of CanBusControlboard, which you may consider also over-bloated for the use case).

I'm not sure I'm getting this right. Do you want to repurpose the CanBusFake device? Shouldn't that read CanBusControlboard + FakeJoint instead?

Question: should we rename FakeControlboard/FakeJoint so that noone gets confused about their main goal? I'm prone to pick VirtualControlboard for the former so that "fake" always means "no-op" (as in CanBusFake).

For the sake of completeness, I found the fakebot device in YARP's codebase. Somewhat close to our intent, but we won't use it anyway.

PeterBowman commented 4 years ago

Question: should we rename FakeControlboard/FakeJoint so that noone gets confused about their main goal? I'm prone to pick VirtualControlboard for the former so that "fake" always means "no-op" (as in CanBusFake).

ASWJ let's rename FakeControlboard to EmulatedControlboard.

PeterBowman commented 4 years ago

Closing as WONTFIX per the above reasons. Two tasks derive from this:

PeterBowman commented 4 years ago

rename FakeControlboard and post an announcement

Done at https://github.com/roboticslab-uc3m/yarp-devices/commit/daf0bc55704f16e57ad1538b87d62bb9d13ca525, cc @roboticslab-uc3m.