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

Dynamic dynamic reconfigure for python #62

Open
awesomebytes opened this issue May 31, 2016 · 49 comments
Open

Dynamic dynamic reconfigure for python #62

awesomebytes opened this issue May 31, 2016 · 49 comments

Comments

@awesomebytes
Copy link

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
Copy link

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
Copy link
Member

@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
Copy link
Author

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
#62 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABpFdO2i7z5706lTp8ulvFnpk2Ure0Nnks5qLeJpgaJpZM4Iqpn8
.

@mikaelarguedas
Copy link
Member

Great! thanks

@NikolausDemmel
Copy link

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
Copy link

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

@awesomebytes, any updates on the above?

@awesomebytes
Copy link
Author

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
#62 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABpFdMNk9E3LLOTekvgPHGA-W_rFBiXJks5qgM_qgaJpZM4Iqpn8
.

@NikolausDemmel
Copy link

Sure, no worries, enjoy!

@NikolausDemmel
Copy link

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

@awesomebytes
Copy link
Author

@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
Copy link

@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
Copy link

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

@awesomebytes
Copy link
Author

awesomebytes commented Jan 4, 2017 via email

@mikaelarguedas
Copy link
Member

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
Copy link

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

@linknum23
Copy link

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
Copy link

@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
Copy link
Author

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

@awesomebytes
Copy link
Author

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
Copy link

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

@NikolausDemmel
Copy link

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

@awesomebytes
Copy link
Author

@linknum23 I ignore it it will be useful but an implementation of the dynamic reconfigure client for c++ was merged relatively recently: #78

@linknum23
Copy link

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

@awesomebytes
Copy link
Author

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

@NikolausDemmel
Copy link

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

@linknum23
Copy link

linknum23 commented Feb 27, 2018 via email

@wew84
Copy link

wew84 commented May 29, 2018

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
Copy link
Author

@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
Copy link

wew84 commented Jun 6, 2018

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
Copy link
Author

awesomebytes commented Jun 6, 2018 via email

@wew84
Copy link

wew84 commented Jun 6, 2018

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
Copy link
Author

awesomebytes commented Jun 6, 2018 via email

@flixr
Copy link

flixr commented Sep 21, 2018

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
Copy link

wew84 commented Sep 21, 2018

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
Copy link

flixr commented Sep 30, 2018

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

@awesomebytes
Copy link
Author

@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
Copy link
Contributor

v-lopez commented Oct 2, 2018

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
Copy link

wew84 commented Oct 2, 2018

@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
Copy link
Contributor

v-lopez commented Oct 2, 2018

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
Copy link

@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
Copy link
Contributor

v-lopez commented Oct 2, 2018

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

@v-lopez
Copy link
Contributor

v-lopez commented Oct 2, 2018

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

@Noam-Dori
Copy link

Noam-Dori commented Oct 2, 2018

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
Copy link
Contributor

v-lopez commented Oct 3, 2018

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
Copy link
Author

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
Copy link

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
Copy link

doronhi commented Oct 15, 2018

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
Copy link
Author

@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
Copy link

doronhi commented Oct 15, 2018

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

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

No branches or pull requests