Added C++ Client class

[progtologist]

Added templated dynamic_reconfigure::Client for C++ users as requested in #4 .

[NikolausDemmel]

I don't really have authority here, but from a brief code review the implementation looks good.

Thanks for the contribution!

Seems like some unit tests are failing.

[progtologist]

The unit test that fails is insignificant, it's just because the test is sleeping for a fixed amount of time to wait for the reconfiguration to happen and that amount is not enough for the build server!

[jacquelinekay]

Regarding the failing test, perhaps you could extend the hardcoded sleep time, retry the test if failed for a fixed number of iterations, or (complicated but awesome) add an option to the API to asynchronously notify when reconfiguration is complete.

[progtologist]

I wouldn't want to over-complicate the API, doing that might discourage potential users. If necessary, I could do something more complicated to make sure the tests pass on all occasions, but I wouldn't want to add "features" to the Client just to make tests easier (that's the wrong way of testing).

[mikaelarguedas]

Thanks @progtologist for the contribution!
Sorry for the tremendous delay I'll review this asap

[v4hn]

@mikaelarguedas ping. Did this fall of your todo list? :)
It would be great to see this merged and released soon!
A c++ client makes dynamic reconfigure so much nicer to use in practice!

[mikaelarguedas]

unfortunately it did get lost in my everyday growing todo list. I'm planning on getting this in for the lunar release and will finish reviewing it in the next few days / week. It looks good and I'm not expecting much change to get this merged. Agreed that a c++ client would be great!

I wouldn't want to add "features" to the Client just to make tests easier (that's the wrong way of testing).

Agreed

sorry all for the delay

[v4hn]

Thanks a lot!

On Thu, Mar 02, 2017 at 12:56:51AM -0800, Mikael Arguedas wrote: I'm planning on getting this in for the lunar release

Please consider adding this to kinetic (and maybe even indigo) too. This is a new submodule, so there shouldn't be any problems with releasing it into existing ROS distros.

[mikaelarguedas]

Yes I'm not planning on branching out, and if I end up doing so this will be backported and will land in every active rosdistro

[mikaelarguedas]

@progtologist Looks good to me. Thanks for contributing this!
I made a few cosmetic changes and modified the tests a bit. I couldn't push on your branch, would you mind having a look at https://github.com/ros/dynamic_reconfigure/compare/ReconfigureClientCpp and comment + cherry-pick accordingly ? Thanks!

[progtologist]

@mikaelarguedas They all look good, thanks for the corrections! You may squash them all and merge!

[mikaelarguedas]

Sounds good, closing in favor of #78 then