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

Additional client information websocket #393

Merged

Conversation

Projects
None yet
3 participants
@achim-k
Copy link
Contributor

commented Mar 20, 2019

Latches a list of all connected clients, where each client is described by

  • the client's IP address
  • the time the client connected

This only works for the websocket server for now. Let me know if this is something useful. If so, then I will look into adding this functionality also to the other backends.

@jihoonl

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Looks good. Thanks.
I think open and on_close are not thread safe. Could you put a lock on add_client, and remove_client?

jihoonl and others added some commits Mar 28, 2019

@jihoonl
Copy link
Member

left a comment

Thanks. Just small nits

def remove_client(self, client_id, ip_address):
with self._lock:
self._clients.pop(client_id, None)
self.__publish()

This comment has been minimized.

Copy link
@jihoonl

jihoonl Mar 28, 2019

Member

Small NITS: wrong indentation.


def add_client(self, client_id, ip_address):
with self._lock:
client = ConnectedClient()

This comment has been minimized.

Copy link
@jihoonl

jihoonl Mar 28, 2019

Member

here too

Show resolved Hide resolved rosbridge_server/src/rosbridge_server/client_mananger.py Outdated
Show resolved Hide resolved rosbridge_server/src/rosbridge_server/client_mananger.py Outdated
Show resolved Hide resolved rosbridge_msgs/CMakeLists.txt Outdated
@achim-k

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

This PR has quite some commits now. I can rebase and squash my commits if that desired.

@jihoonl jihoonl merged commit 841c8ce into RobotWebTools:develop Mar 29, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jihoonl

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

Note that this PR introduced a new dependency on rosbridge_server. So, we need to release with minor patch bump for the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.