robotology / gz-sim-yarp-plugins

YARP plugins for Modern Gazebo (gz-sim).
BSD 3-Clause "New" or "Revised" License
9 stars 2 forks source link

Avoid clock plugin deadlock when YARP_CLOCK is already set #192

Closed xela-95 closed 2 months ago

xela-95 commented 3 months ago

Fix https://github.com/robotology/gz-sim-yarp-plugins/issues/182.

xela-95 commented 3 months ago

Up to now I've checked that by commenting out line https://github.com/robotology/gz-sim-yarp-plugins/pull/192/files#diff-e544b1b964ccc8b1b9805630527cfbd807c46fa4333c455e5e60d185304578e3R57

    yarp::conf::environment::set_string("YARP_CLOCK", "/clock");

in the new regression test, the test passes.

Now I have to find how to correctly handle this.

xela-95 commented 3 months ago

Up to now the test is generating an exception:

[Dbg] [Sensors.cc:262] Waiting for init
[Dbg] [SystemManager.cc:74] Loaded system [gz::sim::systems::SceneBroadcaster] for entity [1]
[INFO] |yarp.os.Port|/fv-az693-479//home/runner/work/gz-sim-yarp-plugins/gz-sim-yarp-plugins/build/bin/ClockTest/12999/clock:i| Port /fv-az693-479//home/runner/work/gz-sim-yarp-plugins/gz-sim-yarp-plugins/build/bin/ClockTest/12999/clock:i active at tcp://127.0.0.1:10002/
Error:  |yarp.os.Network| cannot find port /root
Failure: name server did not accept connection to topic.
Error:  |yarp.os.NetworkClock| Cannot find time port "/clock" or a time topic "/clock@"
[FATAL] |yarp.os.Time| failed creating NetworkClock client, cannot open input port
traversaro commented 3 months ago

Perhaps this the reason why I used an external yarpserver in the gazebo-classic test?

xela-95 commented 3 months ago

Perhaps this the reason why I used an external yarpserver in the gazebo-classic test?

Yep it could be 😅

xela-95 commented 2 months ago

In https://github.com/robotology/gz-sim-yarp-plugins/pull/192/commits/21dba7bb132447ad771f3f755a5a020323f7ffa5 I tried to launch yarp server from a separate process using tiny-process-library, repeating what has been done in gazebo-yarp-plugins. Unfortunately this does not fix the exception I was getting before:

13: [INFO] |yarp.os.Port|/IITICUBLAP218//home/acroci/repos/gz-sim-yarp-plugins/build/bin/ClockTest/586888/clock:i| Port /IITICUBLAP218//home/acroci/repos/gz-sim-yarp-plugins/build/bin/ClockTest/586888/clock:i active at tcp://127.0.0.1:10002/
13: [ERROR] |yarp.os.Network| cannot find port /root
13: Failure: name server did not accept connection to topic.
13: [ERROR] |yarp.os.NetworkClock| Cannot find time port "/clock" or a time topic "/clock@"
13: [FATAL] |yarp.os.Time| failed creating NetworkClock client, cannot open input port
xela-95 commented 2 months ago

I'm not fully convinced of the return value of yarp::os::NetworkBase::isNetworkInitialized(), since it is always returning true, even if I do not start a yarp server.

traversaro commented 2 months ago

I'm not fully convinced of the return value of yarp::os::NetworkBase::isNetworkInitialized(), since it is always returning true, even if I do not start a yarp server.

The whole name of yarp::os::Network is a bit confusing. yarp::os::Network is basically just a class that contains methods that are related to the yarp ports network, for example connect, but its yarp::os::NetworkBase::isNetworkInitialized only checks if yarp::os::Network constructor or yarp::os::Network::init has been called, it does not check if there is a valid yarpserver to which to connect. If you want to do that, the correct method is the checkNetwork: https://www.yarp.it/latest/classyarp_1_1os_1_1NetworkBase.html#a6f723e9789da7a725ab73a61d5fbf53c .

xela-95 commented 2 months ago

Thanks to @traversaro suggestions I've been able to adapt the clock plugin to work without deadlocks also in case of the YARP_CLOCK variable already set. Ready for review.

xela-95 commented 2 months ago

Merging 🚀