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

Closes RobotWebTools/rosbridge_suite#721 #722

Merged
merged 2 commits into from
Mar 15, 2022

Conversation

curiosus42
Copy link

@curiosus42 curiosus42 commented Mar 8, 2022

Public API Changes
None

Description

When the TCP connection of the client is closed, the Python Socket.recv(...) function returns an empty byte array. However, since the while loop checks for an empty string, the thread hangs in an infinite loop and causes the connection not to be disconnected.

Also added minor description changes for more clarity.

Fixes #721

self.protocol.incoming(data.decode().strip(''))
else:
pass
if self.bson_only_mode: # is False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the is False comment referring to?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was just a little memory aid that I forgot to remove.
Thanks for the hint and the fast response. I removed in the newest commit.

@amacneil
Copy link
Member

Thanks! Added one comment, other than that looks good.

@jtbandes jtbandes added the ros1 ros1 label Mar 10, 2022
@amacneil amacneil merged commit 441df7d into RobotWebTools:ros1 Mar 15, 2022
@amacneil
Copy link
Member

🙏 ty

@ribes4
Copy link

ribes4 commented Jun 10, 2022

Hi @amacneil,

As I commented in the issue: ros1 RosbridgeTcpSocket won't close and spins in an endless loop #721.

Is there any plan to release these changes and update the release candidate package for ros 1 to fix this bug?

Thanks!

@amacneil
Copy link
Member

@jtbandes do you know how to do a release?

@jtbandes
Copy link
Member

Made releases for melodic & noetic

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

Successfully merging this pull request may close these issues.

None yet

4 participants