[ros-users] Best way to set the parameters for the controllers (param or message?)

[Jack O'Quin]

On Wed, Apr 21, 2010 at 3:35 PM, Blaise Gassend <blaise at willowgarage.com> wrote:
>> What thread does the server callback run in?
>
> Whatever thread a service call would get made from.
>
>> I suppose it must be invoked in the main thread at the point where it
>> calls ros::spinOnce(). But, I have not been able to find any
>> definitive answer on my own. If my code all runs in the same thread, I
>> should not need any locking for updating internal parameter variables.
>> Otherwise, I should worry.
>
> Correct. If you are using spinOnce then that is where it will be called
> from.
>
>> Is this just a regular ROS service call?
>
> Yes.
>
>> I see my node advertising a set_parameters service. It's also
>> advertising parameter_descriptions and parameter_updates topics.
>>
>> What is the recommended way to update internal parameter variables?
>
> They are passed to the callback. I usually just keep a copy of the
> parameters that are given to the callback.
>
> Note that if you can the parameters that are passed to the callback, and
> your changes will get propagated to the parameter server and any
> listening dynamic_reconfigure clients (example, the GUI).
>
>> I find that calling node.getParamCached() in the main loop actually
>> works, because the reconfigure_gui script is updating the parameter
>> server to keep things in sync. That allows other updating methods,
>> such as "rosparam set" to work. But, it's probably not the best way to
>> operate.
>
> Reconfigure_gui isn't updating the parameters, the dynamic_reconfigure
> server is doing the update.
>
> I would recommend using the parameters that are passed to the callback.
>
>> My callback always seems to get invoked at startup with level set to
>> 0xffffffff. Is that guaranteed? Does it happen immediately? Can I use
>> that to save an initial config structure? Is there a way to make sure
>> those fields get set before my main loop starts executing? Do those
>> values come directly from the parameter server, so I don't need to
>> read them separately?
>
> The callback gets called as soon as it is registered.
>
>> When reconfigure_gui selects my node, it seems to generate a
>> set_parameters call with level 0x0. What does that signify?
>
> The level of the callback is the bitwise-OR of the levels for parameters
> that have changed since the previous time the callback was run. You
> specify the level for a parameter in the .cfg file. This is allows you
> to check whether parameters in a certain set have been modified (but the
> number of sets is limited by the size of an integer).
>
> When you first set the callback, it is called with 0xFFFFFFFF to
> indicate that everything is new.
>
>> How should the reconfigure callback detect which parameters have
>> changed? The obvious solution is to compare the new config structure
>> with one saved on the initial or previous call. Is there a better
>> solution? Most changes don't need to be detected, but some do.
>
> This is what the level is for. Unfortunately the number of parameters
> that can be individually detected this way is limited. If you are going
> to be checking on a parameter per parameter basis, I would recommend
> doing the comparison.
>
>> The level parameter seems to interact with the states of the
>> driver_base package. That goes somewhat deeper than I am ready to
>> venture at the moment. I'm just using dynamic_reconfigure to update
>> values that can easily change during execution of the node, because my
>> code is currently not structured to handle
>> SensorLevels::RECONFIGURE_CLOSE updates. That seems to require a whole
>> different node structure which continues to run after the device is
>> closed.
>
> driver_base uses level to indicate how much it needs to deconfigure the
> device before the parameter can be changed. Effectively, there are
> different sets of parameters. Ones you can change at any time, ones you
> can change when the device is open but not streaming, and ones that
> require the device to be closed before they can be changed.
>
> Users of driver_base have to respect those conventions, but other users
> of dynamic_reconfigure do not.
>
> I hope this helps.

It helps a great deal. Thanks for your usual clear and complete
response, Blaise.

I've got it working pretty well now, using the saved configuration for
most of the parameters. I am pleased with the level of function
available using only a small amount of code. This is a really good
productivity tool. Now I want to use it for a bunch of other drivers.

There was actually only one parameter I needed to check explicitly, so
putting that on a separate levels bit would actually work fine.

Most of my guesses were close enough. I was mainly worried about the
threading model. I wanted to know for sure that no locking was needed,
even though it appeared to work without locks I hate depending on
testing to find concurrency problems. The interface for simple
single-threaded drivers is quite clean.

I did encounter one awkward design issue. My code uses a couple of
subordinate classes for speed control and PID. I could not figure out
a clean way to pass the parameters to the PID class (which is shared
by other drivers), so it still reads its parameters using
getParamCached() on every cycle. That works, after I renamed the
parameters from brake/kp, brake/ki, etc. to brake_kp, brake_ki, etc.
(because dynamic_reconfigure requires valid C++ identifiers for all
parameter names). But, I would prefer to use dynamic reconfigure for
everything.

Is there some way to build a separate parameter server for each
instance of the PID class? I have not tried that yet. It would be
nice, if workable. All those parameters can be updated at any time, so
they would not need to interact with the main driver state. There are
other ways to solve that problem using explicit calls from the
reconfigure callback to subordinate controller methods, but in this
case it's two layers deep, making for messy code. I like using
subordinate parameter names (like brake/kp) for subordinate classes,
it tends to avoid name collisions.
-- 
 joq