Fix problem when loading ros parameters in registerVariable

[francisc0garcia]

Method registerVariable (with callbacks) reads the corresponding ros
parameter, but it can not return the updated value [current_value]
after calling the method, because it isn't a reference. Now it is a reference.

Signed-off-by: Francisco Garcia Rosas [email protected]

[v-lopez]

Hi @francisc0garcia I apologize for not looking at this sooner.

The idea with the callback version of the functions is that you modify the variable in the callback.

[veimox]

@v-lopez I think like @francisc0garcia that is a bug. The user, as per the documentation, expects that if the value is changed on the user domain the variable is updated.

I could understand that the user might also expect that, if the variable if changed either from outside or inside, the callback will be called. This could be handled either by :

1. refactoring the library to use the reactive paradigm (e.g. using observable, boost::signals or RxCpp (checkout this benchmark ) or
2. state in the documentation that it wont call the callback but that the value will be reflected on the /parameters_update topic.

Personally I rather use option 2 as option 1 might lead to circular updates and introduce a new paradigm that the user might not feel comfortable with. If so, then the documentation should express that the callback will only be called when the parameter is updated from the outside only and not from the inside.

[veimox]

Another way that i can think of would be to extend the API and add a new type CallbackPointerRegisteredParam that would have a different signature than CallbackRegisteredParam to avoid confusions. That way we keep backwards compatibility and we achieve our desired behavior. I have made a proposal in #19

[v-lopez]

I believe I did not understand the original issue from @francisc0garcia.

The issue is that if you modify the variable inside your application, the change is not reflected on the dynamic_reconfigure API.

The registerVariable with a callback is working as intended, because the variable you provide is just an initial value, it may be that the variable itself doesn't exist.

But I agree that there is a use case where you do have a variable that you update in your code, and can be updated externally, and also you want to be notified in a callback if it's updated.

In hindsight, I should have added an optional argument to the default RegisterVariable that is a function to be called on update.

I'm trying to find the best way of adding this without breaking API/ABI.

[veimox]

For me both #19 or #20 work, what are your thoughts?