Add overall spin timeout to rosserial read. #334
Conversation
| { | ||
| // Reset back to first mode. | ||
| mode_ = MODE_FIRST_FF; | ||
| return -2; |
There was a problem hiding this comment.
I supposed with a timeout of 0 (the default), the behaviour is unchanged, but it would be good to use a constant here instead, and also clarify that the expectation for users of setSpinTimeout is that they check for this return code on spinOnce, and if they see it, flush the buffer.
There was a problem hiding this comment.
-2 was used for the other timeout below, I could make it something else also.
There was a problem hiding this comment.
I guess this actually works for a user even if they don't explicitly flush the queue— all it does is ensure that spinOnce only takes up to a certain amount of time, so if there's a burst of messages which arrive all at once, those will get processed over a series of spin invocations, so long as there's the buffer space to accommodate them.
In any case, yes, please define SPIN_OK and SPIN_TIMEOUT consts and return them from this function in the appropriate spots.
mikeodr
force-pushed
the
spin-once-timeout
branch
3 times, most recently
from
8905561 to
bd9cf18
Compare
| if ((hardware_.time() - c_time) > spin_timeout_) | ||
| { | ||
| // Reset back to first mode. | ||
| mode_ = MODE_FIRST_FF; |
There was a problem hiding this comment.
@mikepurvis, if we intend to allow for just a continuation of processing, setting this would be detrimental. This kind of forces a buffer flush.
There was a problem hiding this comment.
Might be better to avoid doing so then. All the deserializing state is maintained between spins— if there's outside logic that drops portions of the buffer, the next spin will realise this and resynchronize anyway.
mikeodr
force-pushed
the
spin-once-timeout
branch
from
bd9cf18 to
45666a6
Compare
Reading may block for to long, while there is a check for a timeout, it is not at the global scope of the while(true) statement.
mikeodr
force-pushed
the
spin-once-timeout
branch
from
45666a6 to
887fa24
Compare
| @@ -64,6 +64,10 @@ class NodeHandleBase_ | |||
| namespace ros | |||
| { | |||
|
|
|||
| const int SPIN_OK = 0; | |||
| const int SPIN_ERR = -1; | |||
| const int SPIN_TIMEOUT = -2; | |||
There was a problem hiding this comment.
There's a case to be made that these should be static const int members of NodeHandle, but it's probably fine for them just to be here.
|
Looks like a great change, thanks for the contribution! |
…am to jade-devel * commit 'a707373c7e9f9ad40ad9440550840e29960c9290': Changed hardcoded pin 13 to LED_BUILTIN (ros-drivers#328) Re-attempting rosserial for VEX Cortex (ros-drivers#380) Added service to force an Arduino hard reset in serial_node.py (ros-drivers#349) Fix compiling on boost > 1.66 (ros-drivers#362) Use the ! prefix introduced in gcc4mbed for mbed examples (ros-drivers#304) Add support for boolean parameters (ros-drivers#355) Change Travis to use clang-5.0.0 (ros-drivers#363) [python] fix an unboundlocalerror (ros-drivers#346) Added ESP32 support (ros-drivers#345) Retry opening the serial port every 3 seconds (ros-drivers#342) In rosserial_arduino, changed embedded type size for ROS uint64 to 8 bytes from 4. (ros-drivers#312) 0.7.7 Changes Copy src/examples to install dir so make_libraries.py doesn't fail (ros-drivers#336) Add overall spin timeout to rosserial read. (ros-drivers#334) Fixing formatting on files. (ros-drivers#333) Fix catkin lint errors (ros-drivers#296)
Reading may block for to long, while there is a check for a timeout, it is not at the global scope of the while(true) statement.