Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue-238 Fix tested on indigo & Fanuc #239

Closed

Conversation

PGlaubitzWork
Copy link
Contributor

@PGlaubitzWork PGlaubitzWork commented Aug 21, 2019

connectSocketHandle() will close and re-establish socket handle if the handle is from a previous connection, otherwise it will establish the socket handle.

For #238.

@haudren
Copy link

haudren commented Oct 12, 2020

@gavanderhoorn @Levi-Armstrong : Could you maybe have a look at this PR? it does indeed fix the issue we have with the FANUC robot when the connection drops. I am at your disposal if you need help reproducing the issue and so on.

@haudren
Copy link

haudren commented Nov 9, 2020

@gavanderhoorn @Levi-Armstrong : Who should I contact to get this pushed forward? I don't mind taking care of any necessary changes if needs be.

@JeremyZoss
Copy link
Member

@Levi-Armstrong I don't currently have a good system configured to run tests on this. But a read-through of the changes looks okay to me. A little caution is prudent, since this code is used by many drivers. But socket disconnect handling has long been a weak point of the drivers, so improvements are welcomed. Was independently verified in 2 different systems (both Fanuc) - see comments on #238. My vote is we merge this as-is and keep an eye out for new corner cases that may have been introduced.

@PGlaubitzWork Thanks for this submission!

@gavanderhoorn gavanderhoorn changed the base branch from kinetic-devel to melodic-devel June 22, 2021 16:08
@gavanderhoorn
Copy link
Member

Replacement: #263.

Thanks for the initial PR @PGlaubitzWork 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants