ros / dynamic_reconfigure

BSD 3-Clause "New" or "Revised" License
48 stars 111 forks source link

Dynamic dynamic reconfigure for python #62

Open awesomebytes opened 8 years ago

awesomebytes commented 8 years ago

I implemented a tiny layer to be able to do a "dynamic dynamic reconfigure" server at https://github.com/pal-robotics/ddynamic_reconfigure_python

It allows to add some dynamic reconfigure variables and then just call a start method with a callback to have a dynamic reconfigure server running (instead of using a .cfg file and all the stuff necessary currently for having the server). Give a look to the README with an example. Maybe we want this to be added to the dynamic_reconfigure package itself?

If you give a look at the code I needed to write to achieve this, it's very short: https://github.com/pal-robotics/ddynamic_reconfigure_python/blob/master/src/ddynamic_reconfigure_python/ddynamic_reconfigure.py

@ibaranov-cp I think you already tried it and liked it too :)

ibaranov-cp commented 8 years ago

I tired it, quite liked it actually. From a demo/quick prototype phase, very nice to be able to just write pure Python, and still use dynamic reconfigure for tuning.

mikaelarguedas commented 8 years ago

@awesomebytes Nice work, that's a cool feature addition. In order to integrate it to the dynamic_reconfigure package we would need to provide the same feature for c++ to keep the feature set symmetrical. If nobody has time to implement the c++ counterpart, it can be released as a standalone package (like ddynamic-reconfigure-python) depending on dynamic_reconfigure.

awesomebytes commented 8 years ago

Internally at PAL we have a c++ version. I'll check if it can be released somehow :) On Jun 14, 2016 01:06, "Mikael Arguedas" notifications@github.com wrote:

@awesomebytes https://github.com/awesomebytes Nice work, that's a cool feature addition. In order to integrate it to the dynamic_reconfigure package we would need to provide the same feature for c++ to keep the feature set symmetrical. If nobody has time to implement the c++ counterpart, it can be released as a standalone package (like ddynamic-reconfigure-python) depending on dynamic_reconfigure.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros/dynamic_reconfigure/issues/62#issuecomment-225734664, or mute the thread https://github.com/notifications/unsubscribe/ABpFdO2i7z5706lTp8ulvFnpk2Ure0Nnks5qLeJpgaJpZM4Iqpn8 .

mikaelarguedas commented 8 years ago

Great! thanks

NikolausDemmel commented 8 years ago

Internally at PAL we have a c++ version. I'll check if it can be released somehow :)

I was just looking for something like this and came across this issue. My use-case is exposing the configuration of an existing application to ROS (as a developer interface). The configuration is currently specified programmatically in the source code itself such that all the type information is available at runtime and could be used to feed dynamic reconfigure. Your ddynamic-reconfigure looks like exactly the tool I would need for that, so if releasing it would be a possibilty, that would be pretty neat!

NikolausDemmel commented 7 years ago

Internally at PAL we have a c++ version. I'll check if it can be released somehow :)

@awesomebytes, any updates on the above?

awesomebytes commented 7 years ago

I'm on holidays, I don't expect to get near a computer soon, sorry! On Aug 16, 2016 05:45, "Nikolaus Demmel" notifications@github.com wrote:

Internally at PAL we have a c++ version. I'll check if it can be released somehow :)

@awesomebytes https://github.com/awesomebytes, any updates on the above?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros/dynamic_reconfigure/issues/62#issuecomment-239923698, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpFdMNk9E3LLOTekvgPHGA-W_rFBiXJks5qgM_qgaJpZM4Iqpn8 .

NikolausDemmel commented 7 years ago

Sure, no worries, enjoy!

NikolausDemmel commented 7 years ago

@awesomebytes, could you somehow get me access to that C++ version?

awesomebytes commented 7 years ago

@NikolausDemmel & @mikaelarguedas as spoken personally in ROSCON, I've given you access to a bitbucket repo of the current implementation so we can move it from supporting int's and double's to support all of dynamic reconfigure types (bools, strings, evil_enumerates). Let's see if we can make it work soon and add everything directly to dynamic_reconfigure.

NikolausDemmel commented 7 years ago

@awesomebytes, cheers, I started work on this. Since I need a bit more than just the basic "here is a pointer, please update the variable directly when the reconfigure service is called", I'm considering to take ideas from the currently implementation but effectively write it from scratch. If course I would like to retain the simple use cases in the same style as they are now.

Where would be a good place to discuss design and implementation (in case someone is interested)?

cc @mikaelarguedas

ubuntuslave commented 7 years ago

@NikolausDemmel, @awesomebytes, How is it going? Any plans on releasing the C++ version of ddynamic_reconfigure?

awesomebytes commented 7 years ago

Hello Carlos,

I haven't looked into it again, it got some interest at the moment but nothing was finished. Maybe your comment will ping my friends? :)

On Jan 5, 2017 09:43, "Carlos Jaramillo" notifications@github.com wrote:

@NikolausDemmel https://github.com/NikolausDemmel, @awesomebytes https://github.com/awesomebytes, How is it going? Any plans on releasing the C++ version of ddynamic_reconfigure https://github.com/pal-robotics/ddynamic_reconfigure_python?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros/dynamic_reconfigure/issues/62#issuecomment-270508952, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpFdPLg6wu8VwXKA3lh-pMetpAiSt0lks5rPCClgaJpZM4Iqpn8 .

mikaelarguedas commented 7 years ago

I didn't have any time to work on the C++ implementation since we mentionned it at ROSCon. @NikolausDemmel started working on it a while back but I don't know what the status is

NikolausDemmel commented 7 years ago

Unfortunatly I didn't ever find the time to continue work here. Concepts I have, and use cases, just no time to implement :-|.

linknum23 commented 6 years ago

As mainly a c++ dev I have been interested in adding this for a bit now. @NikolausDemmel it seems like you have some ideas on the implementation and use cases. I am willing to start from scratch on this but if theres something available to start with it might come together faster...

NikolausDemmel commented 6 years ago

@linknum23, I have nothing to base any further work on. Maybe @awesomebytes can give you access to his internal version from a few years ago.

awesomebytes commented 6 years ago

@linknum23 @NikolausDemmel let me give it a try asking.

awesomebytes commented 6 years ago

Ok, I didn't really remember how was the status of this. I added you to a private repository with the partial implementation. The caveats of it:

the current implementation so we can move it from supporting int's and double's to support all of dynamic reconfigure types (bools, strings, evil_enumerates)

There is a branch by @NikolausDemmel called niko-devel with some changes.

linknum23 commented 6 years ago

Fantastic. Thanks @awesomebytes! I will start diving into this.

NikolausDemmel commented 6 years ago

Ah, see, I didn't even remember I pushed this :-). Thanks @awesomebytes and have fun @linknum23.

awesomebytes commented 6 years ago

@linknum23 I ignore it it will be useful but an implementation of the dynamic reconfigure client for c++ was merged relatively recently: https://github.com/ros/dynamic_reconfigure/pull/78

linknum23 commented 6 years ago

Thanks, this has the potential to be useful for dd_reconfigure. The c++ client looks well documented.

awesomebytes commented 6 years ago

Hey @NikolausDemmel @linknum23 did you get any work done on this? :)

NikolausDemmel commented 6 years ago

I haven't worked on it... :-/

linknum23 commented 6 years ago

I have no progress to report. The extra time I had to work on this faded away quickly. I have some hope of working on it in the future but no concrete plans.

On Tue, Feb 27, 2018, 1:37 AM Nikolaus Demmel notifications@github.com wrote:

I haven't worked on it... :-/

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros/dynamic_reconfigure/issues/62#issuecomment-368808866, or mute the thread https://github.com/notifications/unsubscribe-auth/AAmXu9OzjQG7MB3xW-ZIgNP65wY6b-Gkks5tY8zlgaJpZM4Iqpn8 .

wew84 commented 6 years ago

My team and I are encountering these limits to dynamic reconfigure as well. We would would like to build a full python/cpp fix to it. @awesomebytes it would be great to have something to start with. Any chance of getting access to your Bitbucket as well?

awesomebytes commented 6 years ago

@wew84 hello! I can give you access, could you drop me a line of who you are and what you do (maybe by email if you prefer) just to have a reference of who has access to it. I don't own the specific intellectual property of it (even tho I'm allowed to share it privately for the purpose of creating this open source tool) so I need to at least know who you are. Thank you :)

wew84 commented 6 years ago

Thank you @awesomebytes for giving me access to the repo. We are currently working on a fairly large overhaul of dynamic_reconfigure where we will be implementing the following:

  1. C++ API for dynamic_reconfigure client.
  2. dDynamic_reconfigure in all of its functionality for both python and C++
  3. 3d_reconfigure: a new feature where the clients will be able to change the limits, defaults, and whether a param is enabled or not

We are trying to do this over the course of the next month or two. And would be happy to hear your input on this

awesomebytes commented 6 years ago

I believe there is a C++ client API already, I'm on mobile so I can't link it... But it should be in this repo!

And there is a Python dynamic dynamic reconfigure server too (which I wrote), are you going to rewrite it? Why?

3d configure sounds cool! (3d as dynamic dynamic dynamic? Haha)

On Wed, Jun 6, 2018, 19:26 Noam C. Golombek notifications@github.com wrote:

Thank you @awesomebytes https://github.com/awesomebytes for giving me access to the repo. We are currently working on a fairly large overhaul of dynamic_reconfigure where we will be implementing the following:

  1. C++ API for dynamic_reconfigure client.
  2. dDynamic_reconfigure in all of its functionality for both python and C++
  3. 3d_reconfigure: a new feature where the clients will be able to change the limits, defaults, and whether a param is enabled or not

We are trying to do this over the course of the next month or two. And would be happy to hear your input on this

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros/dynamic_reconfigure/issues/62#issuecomment-395005308, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpFdAgFPx2dm_MiWD5x4NQD1k9HUS2Rks5t56AugaJpZM4Iqpn8 .

wew84 commented 6 years ago

Looked more at the source of the repo and see the C++ client API. It doesn't seem to be documented (at least not that I could find), will look into it...

I more meant to write the C++ side and merge the two repos and make sure that they work seamlessly together. No reason to reinvent the wheel.

awesomebytes commented 6 years ago

Sounds great Noam!

Count with me if you need any help with the Python part or we need to discuss anything to keep the same API. Currently the Python package is a separate package, so we can expose a similar API in both languages if needed.

About the C++ client API, documentation, maybe it's time to improve it! Haha

On Wed, Jun 6, 2018, 23:00 Noam C. Golombek notifications@github.com wrote:

Looked more at the source of the repo and see the C++ client API. It doesn't seem to be documented (at least not that I could find), will look into it...

I more meant to write the C++ side and merge the two repos and make sure that they work seamlessly together. No reason to reinvent the wheel.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros/dynamic_reconfigure/issues/62#issuecomment-395059782, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpFdL6nLKCA7OS2YoWa35jXhmLC00_Rks5t59J7gaJpZM4Iqpn8 .

flixr commented 5 years ago

I'm also quite interested in this... any updates here?

Didn't even know the pal-robotics dynamic dynamic_reconfigure, but I basically also implemented something similar in python. Now we would want to use this in some of our C++ nodes as well, so before I start implementing the same again...

wew84 commented 5 years ago

Our C++ side is finished, and created a pull request to @awesomebytes repo. I haven't had time to implement further changes in the Python API, but would be happy to discuss it.

The C++ side of 3d_reconfigure is also done and can be found here.

flixr commented 5 years ago

@wew84 sorry, but which repo exactly do you mean? Seems it is not https://github.com/pal-robotics/ddynamic_reconfigure

awesomebytes commented 5 years ago

@flixr I wasn't aware PAL Robotics released a version, I had a private copy I had permission to share with interested people to develop a fully working solution. And that's what I did with @wew84 @NikolausDemmel @linknum23

Now I presume I can open publicly the repo (please confirm @v-lopez even tho I contacted you thru other means) as they share the same commit history up until the latest version I had access.

Here you can find the version from @wew84 : https://github.com/awesomebytes/ddynamic_reconfigure based on the 0.0.5 version from PAL. Now they are up to 0.1.4.

I think there should be a quick discussion in between @v-lopez and @wew84 about the capabilities of both (given the one in the ddynamic_reconfigure in the PAL repo has no README right now).

v-lopez commented 5 years ago

Yes @awesomebytes no problems in publishing it now.

I've glanced over @wew84's implementation, and my impression is that its purpose and use differs significantly.

Our view of ddynamic_reconfigure was to make it less verbose to use (probably some documentation about it on our side would've made it more evident).

So for instance:

  int int_variable = 0;
  ros::NodeHandle nh("~");
  ddynamic_reconfigure::DDynamicReconfigure dd(nh);
  dd.RegisterVariable(&int_variable, "My int variable", -10000, 10000);
  dd.PublishServicesTopics();

These 5 lines will make the int_variable modifiable from the standard dynamic_reconfigure interfaces. Considering that the first line is the variable declaration and the DDynamicReconfigure object needs only to be created once, the last line needs to be only called once after registering everything, you can register N variables with N+2 new lines of code.

Since we provide the address of the variable, the variable is modified in-place automatically. You still can control when this happens if you control the queue associated to the nodehandle provided to the DDynamicReconfigure.

Our documentation is completely lackluster and that is something we'll work on, but the code is reliable and used extensively in all our robots.

wew84 commented 5 years ago

@v-lopez I think that our purposes don't differ significantly. We built our version to create a fully programmable interface instead of the configurations and the sort currently needed. Here is an excerpt from our README doing what you described above.

ros::NodeHandle nh;
DDynamicReconfigure dd(nh);
dd.add(new DDInt("int_param", 0, "An Integer parameter", 50, 0, 100));
dd.start(callback);

If you take a look at the documentation for our libraries (this one and the expansion 3d_reconfigure). We can work to standardizing the APIs and purposes.

@flixr and @awesomebytes I would be happy to hear your input on the matter.

I am tagging @noam-dori who was the primary developer of our implementations

v-lopez commented 5 years ago

Thanks for clarifying, but from my understanding, you still need to define a callback and in the code of that callback associate write it to the variable.

It may not seem much but it makes it so much more comfortable to use. We can try to merge your changes into ours, but at the moment we cannot afford to break our API.

Noam-Dori commented 5 years ago

@v-lopez there is no need to declare a callback. Instead of doing dd.start(callback);, you can write dd.start();

I don't believe any of the changes will break the API which cannot be fixed via refactoring & renaming.

If there are any other issues be sure to let me know

v-lopez commented 5 years ago

If there is no callback how do you receive the modified values? Do you need to query the dd object?

v-lopez commented 5 years ago

Feel free to open a merge request and we can discuss this there, and see how we can make it work.

Noam-Dori commented 5 years ago

First, the modified values are automatically changed whether or not a callback exists. This behavior is defined both in the original dynamic_reconfigure (for C++ at least) and in the original code of ddynamic_reconfigure. The callback is an extra.

From there, there are two additional ways to fetch the modified values: directly from the DDynamicReconfigure instance, or via a callback.

if you want the variables right when they change, you use the callback, for which you need to define a function with the signature:

void callback(const DDMap& map, int level)

and within said callback, use either of the following utility methods:

// option 1 - get: retrieves a physical value of a parameter by name
    int number = ddynamic_reconfigure::get(map,"int_param").toInt();
// option 2 - at: retrieves a parameter pointer so that you can modify the parameter's behavior (or get its value)
    int number_again = ddynamic_reconfigure::at(map,"int_param")->getValue().toInt();

This way is very similar to the way dynamic_reconfigure defines its callbacks.

In a similar manner, you can get the parameters/values directly from the DDynamicReconfigure instance:

// assume we have fully defined DDynamicReconfigure instance dd
// option 1 - get: retrieves a physical value of a parameter by name
int number = dd.get("int_param").toInt();
// option 2 - at: retrieves a parameter pointer so that you can modify the parameter's behavior (or get its value)
int number_again = dd.at("int_param")->getValue().toInt();

With any of the strategies listed above you can do what you wish with them.

v-lopez commented 5 years ago

In the code segment I posted, you'll see that in our API you are able to provide a pointer to the variable that you want to modify via the ddynamic_reconfigure. So that is the convenience I was referring, that you don't need a callback, or a get function, to get the new values.

About the API, we have been distributing this to our customers for years, we cannot ask all of them to update their code at this moment. We do change API at some points, but tied to our internal release cycle, and in a not radical way.

I believe that we should discuss this somewhere else, and find a way to integrate your changes into the mainstream, in a backwards compatible way.

I understand that your C++ version of ddynamic_reconfigure is a more direct representation of the Python API, but the C++ version was created two years before the Python one, but I guess that some of it's behavior was harder or impossible to replicate in Python.

awesomebytes commented 5 years ago

I'd like to note that there may be ways of extending the Python version to also replicate C++ capabilities. I personally like the idea of adding an already existing variable to a DynReconf server... I've toy-played with an implementation but I haven't had time to get into anything that works and it's as friendly to use. After the 10th of October I may revisit all this.

Noam-Dori commented 5 years ago

Ok, I now understand the issue at hand.

Luckily, the generic parameter class may have multiple implementations and extensions, some of which can support both the legacy implementation of parameter registration and the current one: To simulate the pointer strategy we can extend any instance of DDParam to a sort of DDPointerParam. For example, here I implemented two ways to do so for integers: option 1 - DDIntRef:

#include <ddynamic_reconfigure/param/dd_int_param.h>
#include <ddynamic_reconfigure/dd_param.h>

namespace my_dd_reconfig {
    // interface definition
    class DDRef : virtual public DDParam {
        virtual void *getReference();
    };
    // class definition
    class DDIntRef : public DDRef, public DDInt {
    public:

        int *getReference();

        /**
         * creates a new int reference param
         * @param description details about the parameter
         * @param max the maximum allowed value. Defaults to INT32_MAX
         * @param min the minimum allowed value. Defaults to INT32_MIN
         */
        inline DDIntRef(const string &description, int min = INT32_MIN, int max = INT32_MAX) :
                DDInt(description,0,description,0,min,max) {};
    };

    int *DDIntRef::getReference() {
        return &val_;
    };
}

this strategy makes sure that the DDynamic instance is the one tasked with the memory management of the parameters and uses a bit less memory than option 2. Example of how to use it:

ros::NodeHandle nh;
ddynamic_reconfigure::DDynamicReconfigure dd(nh);
DDIntRef int_param("My int variable", -10000, 10000);
int *int_variable = int_param.getReference(); // pointer memory managed by int_param
dd.add(int_param); // at this point int_param is now managed by the DDynamic instance
dd.start();

the disadvantege of this is that this strategy does not fully match your old API (as the variable is now a pointer and not a reference)

option 2 - DDIntPointer:

#include <ddynamic_reconfigure/param/dd_int_param.h>

namespace my_dd_reconfig {
    class DDPointer : virtual public DDParam {};
    // class definition
    class DDIntPointer : public DDPointer, public DDInt {
    public:

        void setValue(Value val);

        /**
         * creates a new int pointer param
         * @param pointer a pointer to an integer to update.
         * @param description details about the parameter
         * @param max the maximum allowed value. Defaults to INT32_MAX
         * @param min the minimum allowed value. Defaults to INT32_MIN
         */
        inline DDIntPointer(int *pointer, const string &description, int min = INT32_MIN, int max = INT32_MAX) :
                DDInt(description,0,description,0,min,max) {
            other_pointer_ = pointer;
        };

    protected:
        int *other_pointer_;
    };

    void DDIntPointer::setValue(Value val) {
        val_ = val.toInt();
        if(other_pointer_ != NULL) { // prevents some unwanted segfaults
            *other_pointer_ = val_;
        }
    };
}

this strategy matches exactly what existed before in the old API, and can be modified a bit to store additional pointers. The pointer you provide is managed by you so you have the power over it. Example of how to use it:

int int_variable = 0;
ros::NodeHandle nh;
ddynamic_reconfigure::DDynamicReconfigure dd(nh);
dd.add(new DDIntPointer(&int_variable, "My int variable", -10000, 10000));
dd.start();

the disadvantage is that there is additional memory to create per instance, you need to manage pointer memory.

doronhi commented 5 years ago

Hi @wew84, Is a python version also available? I couldn't find it.

I did: sudo apt-get install ros-kinetic-ddynamic-reconfigure-python but got an empty package (from: http://packages.ros.org/ros/ubuntu xenial/main amd64 ros-kinetic-ddynamic-reconfigure-python amd64 0.0.1-0xenia)

awesomebytes commented 5 years ago

@doronhi The main repo is at https://github.com/pal-robotics/ddynamic_reconfigure_python

Hmm it's weird you got an empty package. I just tried in a fresh machine to do your command and it installed correctly. I could run rosrun ddynamic_reconfigure_python example.py successfully

doronhi commented 5 years ago

I did catkin_make and now it works. Do you know if it also works with ROS2?