cbandera / rosparam_handler

An easy wrapper for using parameters in ROS
Other
47 stars 26 forks source link

Add Python support #20

Closed poggenhans closed 7 years ago

poggenhans commented 7 years ago

This implements the python backend for the rosparam_handler. Internally, it works quite similar to how dynamic_reconfigure handles things. A .py-file is generated from a template and installed to a param-folder in the python module destination alongside with the cfg folder from dynamic_reconfigure. That way, using it in python should feel quite natural to a rospy user.

This tries to mirror all the functionality the C++ implementation offers, including type and min/max range checks.

I extended the documentation to represent the new functionality. I will also extend the tutorial repo.

cbandera commented 7 years ago

@poggenhans Thank you very much for your contribution. At the moment I do not have a lot of spare time, but I will look at it asap.

cbandera commented 7 years ago

The default branch and the contribution guidline have changed. Please pull the latest changes from develop and create a new PullRequest against the new default branch. Thank you.

cbandera commented 7 years ago

Why is the call to fromServer() not necessary in Python? I think in the C++ version I ran into problems, when the parameter construct was a member variable and when from Server() was called before ros::init()

poggenhans commented 7 years ago

Well because in python you could just delay constructing the object by doing something like

params = None

if __name__ == '__main__':
    rospy.init_node("my_node")
    params = MyParameters()

to avoid this. This is obviously not possible in C++ due to the type system. And I think if there is a way to relieve people from having to remember to call fromServer(), we should use it.

poggenhans commented 7 years ago

I have merged your changes, added a python version of the unittests and changed the target branch to develop. This PR can be reopened, I guess

cbandera commented 7 years ago

The travis log looks like only one python test is run, but I can get them to fail if I insert a wrong assert statement in one of the other tests. So they must be run aswell. Any Idea why this happens?

poggenhans commented 7 years ago

Yes I noticed that too. It looks like a bug in rostest, because when you run rostest.rosrun, it partly shuts down the node, which as a side effect kills rosout too. And from that point the test is silent for all following calls to rostest.rosrun.

One might refactor the tests so that only one call to rostest.rosrun is necessary, but then all tests would have to go into one tests class, which deprives us of the possibility to distribute tests to individual files.

However you can reproduce the full test output by running the tests directly in rostest: rostest rosparam_handler rosparam_handler_python.test -t.

poggenhans commented 7 years ago

Ok, I integrated your requested changes.

cbandera commented 7 years ago

@poggenhans are you ok with the latest changes? Do they work with your environment?

poggenhans commented 7 years ago

Nice idea! That seems to work. I made a small change to that, because otherwise rostest would display "runTest" as test name for each test:

[rosparam_handler.rosunit-rosparam_handler_python_test/runTest][passed]
[rosparam_handler.rosunit-rosparam_handler_python_test/runTest][passed]
[rosparam_handler.rosunit-rosparam_handler_python_test/runTest][passed]
[rosparam_handler.rosunit-rosparam_handler_python_test/runTest][passed]
[rosparam_handler.rosunit-rosparam_handler_python_test/runTest][passed]

now the output makes more sense again:

[rosparam_handler.rosunit-rosparam_handler_python_test/test_defaults_at_launch][passed]
[rosparam_handler.rosunit-rosparam_handler_python_test/test_defaults_missing][passed]
[rosparam_handler.rosunit-rosparam_handler_python_test/test_defaults][passed]
[rosparam_handler.rosunit-rosparam_handler_python_test/test_defaults_on_server][passed]
[rosparam_handler.rosunit-rosparam_handler_python_test/test_min_max_parameters][passed]