-
Notifications
You must be signed in to change notification settings - Fork 913
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
roslaunch: add support for "param" tag in substitution args #723
Comments
Was just looking for a way to do this exact thing today. |
same here |
Would lift the redundant work of duplicating numbers and strings into launch file. This should also work for |
I'm searching for the same solution... |
I have implemented it. If you don't want to wait for official merge, you can directly clone my own fork and try it. I couldn't do an extensive testing, so I'm waiting feedbacks from you for any type of problems. |
@tahsinkose Thanks for implementing this! I think this would be an amazing feature to have. |
I'm glad that it helps for you @rickstaa . Unfortunately, I don't have the time to re-inspect the PR and have it merged into the master. I know that @dirk-thomas and other maintainers have shifted their focus on ROS 2 development, which has a Python-based launch system, therefore they may not be as helpful as they can for the ROS 1. If you like you can take on this simple feature and have it pass the tests. |
@tahsinkose Ah thanks I understand. Thanks again for the nice pull request! |
I would love to see it merged too |
Hello, this issue is already quite old, i saw a recent PR: #2097 which solves it. What is the state of this, is there a good chance that this will be accepted? |
I'm expecting some maintainer to have a look at it. As most of the efforts are channeled into ROS2 dev, there is not much people left to accept/reject/review stuff. Who should I ping for this? |
I would love to see it merged, too :) |
I've also been looking for a way to load an arg from a yaml file (and also for structured args, i.e. lists & dicts). That said, I have a few issues with the particular approach suggested here and implemented in #2097.
<param name="foo" value="bar" />
<node name="n1" pkg="p" type="t" args="$(param foo)" />
<param name="foo" value="baz" />
<node name="n2" pkg="p" type="t" args="$(param foo)" /> If Instead, I would suggest the fix be an extension(s) that leverages the existing |
@luisrayas3 I think your use-case is different from what is asked in this issue. From the responses, I can say that there IS definitely need for this support as ROS parameters are not just needed as run-time constructs in practice. We also need them in launch-time occasionally, if not often. Without this capability, the only way to use the same information is via specifying a duplicate
As I said, this is a different use-case. You want to be able to not bloat ROS parameter server when using Consider the following example:
<launch>
<rosparam command="load" file="$(find my_pkg)/params/grippers.yaml" />
<group if="$(eval param('/robot/gripper') == 'two_fingers')">
...
</group>
<group if="$(eval param('/robot/gripper') == 'hand')">
...
</group>
</launch> ...
ros::NodeHandle nh;
const std::string gripper = nh.param<std::string>("/robot/gripper", <def_val>);
... |
Thanks @tahsinkose for your thoughtful response. I'm still not convinced. I do agree that ostensibly, we have different use cases but I believe they can be understood as the same underlying deficiency, and that it would be better to identify it as a deficiency in I think the response to (1) belies itself. To expound my 2nd bullet, the problem is that this solution renders Upon further reflection, I actually don't think it is possible to leave Importantly, this also means And I just don't think it's needed, one can already set params from args, so with "extended" args (syntax made up) the provided example becomes: <launch>
<arg name="grippers" type="extended" value="$(eval load_yaml(find('my_pkg') + '/params/grippers.yaml'))" />
<rosparam subst_value="True">$(arg grippers)</rosparam>
<group if="$(eval robot['gripper'] == 'two_fingers')">
...
</group>
<group if="$(eval robot['gripper'] == 'hand')">
...
</group>
</launch> |
@luisrayas3 thanks for the additional clarification. I do understand your concern about the mutability of
Thanks for explicit statement. This issue was started with -and only with- this very use-case in mind. You are right that the subsequently changed
It is not programmatically possible to make this exclusion among substiution-arg I fully understood the 2nd argument in the previous comment and still my point stands. I disagree with you about the "be forced to bloat parameter space" part. If some parameter does matter also in program code, then it must naturally be a From a very similar perspective, your suggested extension could be criticized for disabling the ability of using Your hack is really nice and a little bit dirty 🙂. First, you willingly don't use the full Your suggested solution makes me think more deeply on how the Lastly, I'm open to a voting session by the authorized maintainers whether to accept this as a real issue and thus the "fix" PR. @jacobperron could you please read the discussion spanning last 4-5 comments and provide some feedback on how to react? All in all, I believe @luisrayas3 denoted serious downsides of the proposed EDIT:
On second thought, this could be actually done programmatically. Each I'll start implementing the final feature after settling the discussion in the scope of #2097. |
After reviewing the discussion above, I lean towards the position that we shouldn't allow
As I understand, this is a feature request to make reusing values from a parameters YAML file in a launch file more convenient, not necessarily to use parameter values directly (which is a tricky rabbit hole as we're learning). At a glance, It seems more straight forward to add a feature to I'd like to play around with #2097 to get a feel for how it works in different cases, but it seems I hit an exception trying to run the example in this issues description. @tahsinkose Can you run the example (perhaps it's just a problem on my end)? |
It will behave as a temporal entity. So the ROS parameter tree is being built up during the launch tree crawling. When
Unfortunately it is not as straightforward as it seems. The Another problem is the possible <arg name="some_arg" value="foo"/>
<arg name="placeholder" type="extended" value="$(eval load_yaml(find('my_pkg') + '/params/args.yaml'))" /> # args.yaml
some_arg: bar Also what
I had run it before creating the relevant PR and it was working. If you could share the exact launch and yaml file, I can rapidly test it. All in all, the addition of such extension on args require much more effort than already spent in the relevant PR. Also the missing gap for resolution of placeholder I feel like we should end this discussion with one final comment from your side @jacobperron as my cycles for returning back to this issue are sparsed enough. I wouldn't mind the declination of the PR, but I'm not going to implement @luisrayas3's suggestion either as I believe both approaches have their pros&cons and both boil down to the cautious usage from the users. |
I feel like this discussion was a little misguided unfortunately. This was (and is at least for me) specifically about how static_transform_publisher can be used. In my use case I would like to decouple the launch file from the specific transformation, as it may be occasionally updated by a script. This would be solved easily if static_transform_publisher accepted rosparams as an alternative to command line args. I will look into this tomorrowish and provide a PR. |
Amazing! |
Say the contents of a file tf_stp_area1.yaml were
I feel it would be very convenient to be able to do the following in a launch file
The text was updated successfully, but these errors were encountered: