roboticslab-uc3m / yarp-devices

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

[LaunchManipulation] --homePoss is not working #175

Closed rsantos88 closed 6 years ago

rsantos88 commented 6 years ago

Since this commit (https://github.com/roboticslab-uc3m/yarp-devices/commit/3fcf1b5aeda71a234c2a61b2b7905213ca8d5f85), --homePoss is not working. The robot doesn't move to the home position and the shell shows this:

[debug] IControlMode2Impl.cpp:93 setControlMode(): (0, 7565168)
[debug] IControlMode2RawImpl.cpp:289 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (1, 7565168)
[debug] IControlMode2RawImpl.cpp:289 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (2, 7565168)
[debug] IControlMode2RawImpl.cpp:289 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (3, 7565168)
[debug] IControlMode2RawImpl.cpp:289 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (4, 7565168)
[debug] IControlMode2RawImpl.cpp:289 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (5, 7565168)
[debug] IControlMode2RawImpl.cpp:289 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (6, 7565168)
[debug] IControlMode2RawImpl.cpp:289 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (7, 7565168)
[debug] IControlMode2RawImpl.cpp:111 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (8, 7565168)
[debug] IControlMode2RawImpl.cpp:111 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (9, 7565168)
[debug] IControlMode2RawImpl.cpp:111 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (10, 7565168)
[debug] IControlMode2RawImpl.cpp:111 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (11, 7565168)
[debug] IControlMode2RawImpl.cpp:111 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (12, 7565168)
[debug] IControlMode2RawImpl.cpp:111 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:93 setControlMode(): (13, 7565168)
[debug] IControlMode2RawImpl.cpp:111 setControlModeRaw(): (0, 7565168)
[debug] IControlMode2Impl.cpp:69 getControlModes(): 
[debug] DeviceDriverImpl.cpp:298 open(): Moving motors to zero.
[debug] DeviceDriverImpl.cpp:331 open(): Moved motors to zero.
[INFO]created device <CanBusControlboard>. See C++ class roboticslab::CanBusControlboard for documentation.

but no movement... Doing git checkout 72d4c8e5b615fc14d7ab4a087e54d70b6c052d1d (reference: https://github.com/roboticslab-uc3m/yarp-devices/commit/72d4c8e5b615fc14d7ab4a087e54d70b6c052d1d) it works again without any problems. @PeterBowman could you fix it? Thanks!

jgvictores commented 6 years ago

From what I'm seeing, it shouldn't be due to 3fcf1b5aeda71a234c2a61b2b7905213ca8d5f85.

Could you do the following to see if this is repeatable?

  1. 3 tests with 3fcf1b5aeda71a234c2a61b2b7905213ca8d5f85
  2. 3 tests with 72d4c8e5b615fc14d7ab4a087e54d70b6c052d1d
  3. 3 tests with 3fcf1b5aeda71a234c2a61b2b7905213ca8d5f85
  4. 3 tests with 72d4c8e5b615fc14d7ab4a087e54d70b6c052d1d

My guess is it may be related with #170

rsantos88 commented 6 years ago

With 08f6a4162ef0af342ff9489c21901d062fc88e8f:

[debug] DeviceDriverImpl.cpp:298 open(): Moving motors to zero.
[debug] DeviceDriverImpl.cpp:331 open(): Moved motors to zero.
rsantos88 commented 6 years ago

With 72d4c8e5b615fc14d7ab4a087e54d70b6c052d1d:

[debug] DeviceDriverImpl.cpp:294 open(): Moving motors to zero.
[debug] DeviceDriverImpl.cpp:302 open(): Value of relative encoder ->0.000000
[debug] DeviceDriverImpl.cpp:309 open(): It's already in zero position
PeterBowman commented 6 years ago

My fault, but PolyDriver's docs and implementation are a bit weird wrt. PolyDriver::getValue:

https://github.com/roboticslab-uc3m/yarp-devices/blob/08f6a4162ef0af342ff9489c21901d062fc88e8f/libraries/YarpPlugins/CanBusControlboard/DeviceDriverImpl.cpp#L301

Looks like getValue performs a lookup on the monitor instance (not on options passed through open), ref.

PeterBowman commented 6 years ago

From PolyDriver.cpp:

Value PolyDriver::getValue(const char *option) {
    if (system_resource==nullptr) {
        return Value::getNullValue();
    }
    return HELPER(system_resource).getValue(option);
}

The system_resource member is a local instance of YarpDevMonitor (which extends SearchMonitor). However, we don't use it at all if the config parameter to PolyDriver::open already has a monitor object attached to itself:

bool PolyDriver::open(yarp::os::Searchable& config) {
    if (isValid()) {
        // already open - should close first
        return false;
    }
    if (system_resource==nullptr) {
        system_resource = new YarpDevMonitor;
    }
    yAssert(system_resource!=nullptr);
    bool removeMonitorAfterwards = false;
    if (config.getMonitor()==nullptr) {
        config.setMonitor(&HELPER(system_resource));
        removeMonitorAfterwards = true;
    }

    //dd = Drivers::factory().open(config);
    coreOpen(config);
    HELPER(system_resource).info.fromString(config.toString());
    if (removeMonitorAfterwards) {
        config.setMonitor(nullptr);
    }
    return isValid();
}

The monitor linked to config registers a device key in coreOpen. If this is not the local monitor instantiated in open, getValue won't return what we actually expect.

As pointed out by @rsantos88, this happens since 3fcf1b5 (https://github.com/roboticslab-uc3m/questions-and-answers/issues/49).

PeterBowman commented 6 years ago

Partially reverted in 6467003, should be fine now. I'll be looking for some means to make this work again...

rsantos88 commented 6 years ago

Partially reverted in 6467003, should work now. I'll be looking for some means to make this work again...

Thanks @PeterBowman. Yes, it works now.

PeterBowman commented 6 years ago

The high-priority bug is solved, but the rationale behind the now-reverted commit affects more repos. Let's get back to https://github.com/roboticslab-uc3m/questions-and-answers/issues/49 and track it properly at that place.

PeterBowman commented 5 years ago

The high-priority bug is solved, but the rationale behind the now-reverted commit affects more repos. Let's get back to roboticslab-uc3m/questions-and-answers#49 and track it properly at that place.

Locally split back into https://github.com/roboticslab-uc3m/yarp-devices/issues/207.