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

Feature load from another package #36

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

awesomebytes
Copy link

@artivis asked me to take a look into how to expand one .params file with another one. The easiest way I found after tinkering a bit was to load from another .params file from another package, and then you can add your additional parameters.

As the documentation says, there is an example package: https://github.com/awesomebytes/imported_rosparam_handler_test that imports from https://github.com/cbandera/rosparam_handler_tutorial and adds a couple of parameters extra.

I needed to do a few hacky things for the sake of not touching any already written .params file but it works nicely (as far as I've tried).

As improvements, instead of initializing, we could add a way of just expanding the parameters, and even remove some... but if that's not a real use case, there is no reason to implement it.

Also with this change one can test it's writing the .params file correctly by using an python (ipython please?) console as I needed to move to the generation step the error about having != 5 arguments.

pkg_path = rp.get_path(package_name)
except ResourceNotFound:
return None
full_file_path = pkg_path + '/cfg/' + params_file_name
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be handy to have the possibility to specify the relative path.
An extra param to the load_generator (and affiliated functions) with default value : path='/cfg/'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in the last commit. Updated docs accordingly

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@artivis
Copy link
Collaborator

artivis commented Apr 12, 2018

Hi @awesomebytes,
would it be possible for you to rebase your branch on top of branch feature/parameters_base ?? Those two branches are going toward the same goal and are plained for an eventual 0.2 version.
Thanks !

@awesomebytes
Copy link
Author

@artivis I clicked on Update, would that be enough? (CI is pending).

@artivis
Copy link
Collaborator

artivis commented Apr 12, 2018

I actually meant that your branch is based on branch develop while it would be better based on branch feature/parameters_base. I will have a look see how much work does that actually implies.

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

Successfully merging this pull request may close these issues.

2 participants