ros-industrial / industrial_core

ROS-Industrial core communication packages (http://wiki.ros.org/industrial_core)
153 stars 180 forks source link

Adds receive overloads with timeouts #246

Closed ghost closed 3 years ago

ghost commented 4 years ago

See also #34.

Existing receive functions are unaffected, and behave exactly as before. The only big difference is that all classes inheriting from SmplMsgConnection, i.e., SimpleSocket must now override a receiveBytes member function with a timeout parameter. (So if any other classes inherit from SmplMsgConnection, they will need to be updated.)

Also, a few random whitespace fixes I couldn't resist.

ghost commented 4 years ago

Ping!

ghost commented 4 years ago

If anyone is wondering why this is necessary, consider the following code:

if (conn->isReadyReceive(ms_timeout)) {
  conn->receiveMsg(res);
}

If you were hoping this code would always result in a non-blocking read, well that's not guaranteed.

The problem is that isReadyReceive can only check if any data is ready for reading. On the other hand, receiveMsg will block until all data has been read.

So if you receive the first few bytes of a message, but then the connection hangs:

  1. isReadyReceive will call rawPoll and return true.
  2. receiveMsg will call receiveBytes, which will enter its internal while loop.
  3. receiveBytes will successfully receive a few bytes.
  4. (Connection hangs here.)
  5. receiveBytes will no longer getting any bytes.
  6. Stuck forever in while (remainBytes > 0) {.

The problem is that there is no way to escape the internal while loop, and doing isReadyReceive doesn't actually prevent receiveMsg from getting stuck. The problem is worse for large messages.

ghost commented 4 years ago

@realanton I would if I could but this is actually the ROS Industrial repo! :laughing:

gavanderhoorn commented 3 years ago

Newer version in #267.