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

Less restrictive CommsClient Bind interface #32

Closed
osrf-migration opened this issue Dec 3, 2018 · 6 comments
Closed

Less restrictive CommsClient Bind interface #32

osrf-migration opened this issue Dec 3, 2018 · 6 comments

Comments

@osrf-migration
Copy link

Original report (archived issue) by Jon Fink (Bitbucket: jonfink-arl).


Rather than only allowing the CommsClient to bind to a member function of a class, accept a std::function that can accept a wider range of C++ functionals. E.g.,

bool Bind(std::function<void(const std::string &_srcAddress, 
                             const std::string &_dstAddress, 
                             const uint32_t _dstPort,
                             const std::string &_data)> cb,
                    const std::string &_address = "",
                    const int _port = kDefaultPort)
@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


@caguero , what do you think of this proposal?

@osrf-migration
Copy link
Author

Original comment by Jon Fink (Bitbucket: jonfink-arl).


@nkoenig @caguero I actually have a branch where I've implemented this API (while keeping old method).

Should I be able to push to the repository? (I tried at one point and it did not work, but I'm using a git interface to mercurial so that may be causing issues.

@osrf-migration
Copy link
Author

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).


Great, you should be able to submit a pull request. If it doesn't work, point me to your branch and I can create the pull request for you.

@osrf-migration
Copy link
Author

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).


See pull request #99.

@osrf-migration
Copy link
Author

Original comment by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).


  • changed state from "new" to "resolved"

Implemented in pull request #99.

@osrf-migration
Copy link
Author

Original comment by Alfredo Bencomo (Bitbucket: bencomo).


  • changed state from "resolved" to "closed"

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

No branches or pull requests

1 participant