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

Unify socket open #174

Merged
merged 3 commits into from
Sep 4, 2023
Merged

Conversation

fmauch
Copy link
Collaborator

@fmauch fmauch commented Sep 4, 2023

While implementing #172 I realized there are a couple of things that IMHO would benefit from a refactoring regarding our socket abstraction. This PR tries to do that.

Before, it as a virtual method that in its base implementation returned `false`
only.

It had to be (and was) implemented in several classes inheriting from it
and all implementations were the same. This commit defines the method inside
the base class directly.
@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed Coverage
include/ur_client_library/comm/stream.h ø
include/ur_client_library/ur/dashboard_client.h ø
include/ur_client_library/comm/tcp_socket.h 100.00%
src/comm/tcp_socket.cpp 100.00%

📢 Thoughts on this report? Let us know!.

@fmauch fmauch marked this pull request as ready for review September 4, 2023 09:20
@fmauch fmauch requested a review from urmahp September 4, 2023 09:20
Copy link
Collaborator

@urmahp urmahp left a comment

Choose a reason for hiding this comment

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

LGTM :)

@fmauch fmauch merged commit 7626fe5 into UniversalRobots:master Sep 4, 2023
18 of 19 checks passed
@fmauch fmauch deleted the unify_socket_open branch September 4, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants