Fixed auto reconnection to the RTDE server. #384
Conversation
When reconnecting to RTDE it requires you to set the input and output recipes again, this is now done properly.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #384 +/- ##
==========================================
- Coverage 78.22% 76.51% -1.72%
==========================================
Files 92 92
Lines 4212 4283 +71
Branches 467 482 +15
==========================================
- Hits 3295 3277 -18
- Misses 680 761 +81
- Partials 237 245 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Also fixed that we can destroy the rtdeClient while reconnecting
| if (stream_.getStreamType() == URStreamType::RTDE) | ||
| { | ||
| if (on_rtde_reconnect_cb_) | ||
| { | ||
| URCL_LOG_WARN("Failed to read from RTDE stream, invoking on reconnect callback and stopping the producer"); | ||
| on_rtde_reconnect_cb_(); | ||
| } | ||
| else | ||
| { | ||
| URCL_LOG_ERROR("Failed to read from RTDE stream without a reconnect handler stopping the producer"); | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
I didn't really like the special handling of an RTDE producer here. Couldn't we generalize this by checking whether there is a reconnection callback registered? If none then we simply do not reconnect and if yes, trigger reconnect independent of the stream type. Sorry that I just come up with this now, but that seems cleaner to me.
There was a problem hiding this comment.
Reconnecting to the stream, will just not have any affect if it is a RTDE stream, but I will change the check.
Applied suggestions from code review
src/rtde/rtde_client.cpp
Outdated
| } | ||
|
|
||
| bool RTDEClient::negotiateProtocolVersion(const uint16_t protocol_version) | ||
| std::pair<bool, uint16_t> RTDEClient::setProtocolVersion() |
There was a problem hiding this comment.
It seems like we are only interested in the boolean return value, so should we reduce the signature to that one?
Co-authored-by: Felix Exner <feex@universal-robots.com>
Co-authored-by: Felix Exner <feex@universal-robots.com>
2 participants
When reconnecting to RTDE it requires you to set the input and output recipes again, this is now done properly.