search

ros-industrial / motoman

ROS-Industrial Motoman support (http://wiki.ros.org/motoman)
146 stars 193 forks source link

MotoROS v1.9.4 #402

Closed ted-miller closed 3 years ago

ted-miller commented 3 years ago

Fix lockup if network connection was broken without properly disconnecting. Update Visual Studio project file to be compatible with VS2019.

ted-miller commented 3 years ago

This update has been tested by @EricMarcil

gavanderhoorn commented 3 years ago

High-level comment: according to what I can find, TCP_KEEPCNT et al. should be supported on the VxWorks versions MotoROS targets. That should make it possible to reduce the "2 hours" idle timeout and configure it on a per fd level.

Using TCP idle detection/heartbeats could make the use of PING redundant and reduce the complexity of the implementation.

Was that considered?

Edit: additionally, could you add a short description of the approach taken to implement lost connections? SO_KEEPALIVE is enabled, but there also appear to be structural/control flow changes in addition to the use of the PING message. It's not entirely clear to me how those all interfact (if they do).

ted-miller commented 3 years ago

The mpSetSockOpt api does not support additional configuration options. Also, I dug through the vxworks header files and couldn't find the applicable options in there either. So, I'm curious what documentation you found.

The code to send a ping command was actually removed. I left the code to receive/handle a ping though. I've seen an instance where someone was using it and they got a bunch of unnecessary error messages.

Basically, all this new code does is allow a new connection to replace an old connection. If a new one comes in, then the old one is dropped. In the event that motion is in process, then I assume that the old connection is still in control and I reject the new connection.

gavanderhoorn commented 3 years ago

I won't have time to test this on real hw until Friday.

In the meantime, I'm going to accept the PR in preparation of merging it.

It could be that using TCP_KEEPCNT et al. is possible on VxWorks, but that could be added later.

gavanderhoorn commented 3 years ago

I've not been able to test this today as our YRC1000 has developed some problems with the external axis, blocking all use of the robot.

Based on @EricMarcil's tests we'll merge this.

Thanks @ted-miller for the PR and addressing the reconnection issue.

gavanderhoorn commented 3 years ago

Updated the wiki: diff.