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

YAML params in global namespace are not respected #130

Closed
jarvisschultz opened this issue Jul 22, 2020 · 3 comments
Closed

YAML params in global namespace are not respected #130

jarvisschultz opened this issue Jul 22, 2020 · 3 comments
Labels

Comments

@jarvisschultz
Copy link

Imagine I have the following files:

example.launch:

<launch>
  <group ns="example">
    <rosparam command="load" file="/path/to/test.yaml" />
  </group>
</launch>

test.yaml:

/fake_param: 20.0

Using roslaunch, this ends up with a parameter of /fake_param, using rosmon instead ends up with a parameter of /example/fake_param. Reading ROS docs on YAML restrictions, I can't find anywhere where globally resolved names in YAML files are described in a REP. So it's not clear if one should do that or what the expected outcome would be. Regardless, the behavior between rosmon and roslaunch is not the same.

xqms added a commit that referenced this issue Jul 23, 2020
See #130 for bug report. Basically, roslaunch respects leading slashes
during key resolution, we don't. We don't know what's better, but we have
to match whatever roslaunch is doing, as always.
@xqms
Copy link
Owner

xqms commented Jul 23, 2020

Yep, that's a bug (like any other non-conformance with roslaunch). Thanks for reporting and condensing it down to a minimal example 👍

Reading ROS docs on YAML restrictions, I can't find anywhere where globally resolved names in YAML files are described in a REP.

I'm not surprised, a lot of roslaunch's behavior does not appear in any spec 🤷

There's a tentative fix in #131.

@xqms xqms added the bug label Jul 23, 2020
@xqms
Copy link
Owner

xqms commented Jul 23, 2020

While setting up a test case I found some even crazier cases:

<rosparam ns="ns">
/fake_param: 20.0
namespace:
/other_param: 10.0
/global_ns:
third_param: 5.0
</rosparam>

Anyway, tests pass and roslaunch has the same behavior on the test cases, so I merged #131. Thanks again for reporting!

@xqms xqms closed this as completed Jul 23, 2020
@jarvisschultz
Copy link
Author

Awesome... thanks for the fast response! Fixes the issue I was seeing.

gauthamm added a commit to rapyuta-robotics/rosmon that referenced this issue Jul 15, 2021
* core: launch_config: parse attributes in parent context

and make it mutable (required for $(anon))

* core: launch_config: handle $(anon) in ParseContext, see xqms#100

* core: test: node_utils: more debug output

* core: test: test_subst: test for nested $(anon), see xqms#100

* fix two typos in the README

* rosmon_core: fix a lot of whitespace issues introduced in xqms#76

* rosmon_core: option to obey output="XY" tags (see xqms#108)

Co-authored-by: Roger Pi <u1926981@campus.udg.edu>

* rosmon_core: unit test for output-attr parsing, refs xqms#108

* fix xqms#110

* rosmon_core: do not print node prefix in --disable-ui mode

--disable-ui is mostly used with ncurses-like nodes that have problems
with such additional outputs.

* rosmon_core: node_monitor: flush out remaining node output after node exit

* rosmon_core: new rosmon-disable-ui attribute for problematic launch files

* core: launch_config: accept empty yaml node

* rosmon_core: LaunchConfig warnings to stderr & make them captureable

* rosmon_core: test: unit tests for the warnings we trigger

* rosmon_core: Add YAML merge key parsing functionality.

* rosmon_core: Adding fix for rosparam parsing to equivilate with undocumented behavior of roslaunch

* rosmon_core: test: unit tests for merge keys feature

* rosmon_core: ui: UI DrawStatus optimized to only refresh on events.

* core: launch: add some comments & fix whitespace

* rosmon_core: depend on python2/3 conditioned on ROS_PYTHON_VERSION

* core: launch: add comments

* core: ui: just redraw on every keystroke

* core: launch: shift some node attributes into context

See xqms#116

* rosmon_core: parse scope attributes also for launch, include, group tags

* core: launch: make "enable-coredumps" a scoped attribute

* core: test: add test for scoped attributes on include tags

* core: monitor: support for systemd-coredump

* core: monitor: delete working directories from two runs ago

* capture stderr separately and correctly do output=log

* roscore: test: Fixing unit tests for stdout_stderr mod

* Adding alt. stderr output filter

* CHANGELOG

* CHANGELOG

* 2.3.0

* rosmon_core: cmake: request specific boost_python3 version

* rosmon_core: test: adapt test_node to python 3

* CHANGELOG

* 2.3.1

* rosmon_core: *really* fix search for boost_python for python 3

* rosmon_core: cmake: increase minimum version to address policy warnings

* CHANGELOG

* 2.3.2

* rosmon_core: cmake: older cmake versions don't have GREATER_EQUAL

* all: increase cmake minimum version to 3.4

* README: add installation hint for rosmon_core

* core: launch_config: loadYAMLParams: don't scope keys with leading slash

See xqms#130 for bug report. Basically, roslaunch respects leading slashes
during key resolution, we don't. We don't know what's better, but we have
to match whatever roslaunch is doing, as always.

* core: test: xml: rosparam: test leading slashes in keys

* core: consolidate floating point types used for stats

Fixes problems identified with -Wfloat-conversion

* core: ui: explicit float casts

Fixes problems identified with -Wfloat-conversion.

* rqt: make float/int casts explicit

Identified with -Wfloat-conversion.

* rosmon_core: env-hooks: follow symbolic links during file name suggestion

This fixes bash completion for colcon linked install spaces.

* core: substitution_python: catch by reference

Fixes a compiler warning on Ubuntu Focal.

* core: ui: display & search full name including namespace

* core: ui: use full name everywhere

* Start all and stop all options are added
F6 -> start all
F7 -> stop all

* core: launch: bytes_parser: add missing include

* Added option --stdout-print-source to print out node name when --disable-ui is used. Addresses xqms#139.

* Removed stdout-print-source option and print node name by default when --disable-ui is used. Right justify node names to the max node name length.

* core: fix compilation on Kinetic (boost::placeholders came later)

* core: monitor: do not leak stderr file descriptors

* core: launch_config: add warning for type= attrs on <param> tags on Kinetic

See xqms#138.

* update warning

* core: launch_config: strip off leading/trailing whitespace of params

* core: add utility to precisely dump ROS parameters

This is useful to analyze differences between roslaunch and rosmon in
parameter loading.

* core: launch_config: use double precision for auto type parameters

Previously we used floats, but XmlRpc stores doubles, so we should
make use of the full precision.

* core: launch_config: convert tabs + newlines in parameters

* core: test: add unit test for all sorts of whitespace in parameters

* core: test: basic.launch: make it work with roslaunch

* core: test: basic.py: adapt to fixed whitespace behavior

* core: fix aliasing problem using std::memcpy

* Resolves xqms#145 by providing a number of restarts threshold per node via ROS-launch arguments

* core: fix signed/unsigned comparison warning

* core: install tests (necessary for testing with colcon)

* core: monitor: use a second PTY for stderr, see xqms#147

* Inidicate when a node is actually Idle (e.g. stopped)

* Add missing dependency on libboost-pyhton-dev

Co-authored-by: Max Schwarz <max.schwarz@ais.uni-bonn.de>
Co-authored-by: Max Schwarz <max.schwarz@online.de>
Co-authored-by: Roger Pi <u1926981@campus.udg.edu>
Co-authored-by: Kazuhiro Hiratsuka <papino0319@gmail.com>
Co-authored-by: marco-tranzatto <marcot@ethz.ch>
Co-authored-by: Carl Colena <carl.colena@gmail.com>
Co-authored-by: Kutay YILMAZ <ktyylmz035@gmail.com>
Co-authored-by: Steve Golton <stevegolton@gmail.com>
Co-authored-by: mcfurry <f.b.f.schoenmakers@gmail.com>
Co-authored-by: Ferry Schoenmakers <ferry.schoenmakers@nobleo.nl>
Co-authored-by: Tim Clephas <tim.clephas@nobleo.nl>
gauthamm pushed a commit to rapyuta-robotics/rosmon that referenced this issue Jul 15, 2021
See xqms#130 for bug report. Basically, roslaunch respects leading slashes
during key resolution, we don't. We don't know what's better, but we have
to match whatever roslaunch is doing, as always.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants