search

ros / ros_comm

ROS communications-related packages, including core client libraries (roscpp, rospy, roslisp) and graph introspection tools (rostopic, rosnode, rosservice, rosparam).
http://wiki.ros.org/ros_comm
750 stars 912 forks source link

roscpp param::init() needs to parse mappings using yaml-cpp to be consistent with rospy #1498

Open peci1 opened 5 years ago

peci1 commented 5 years ago

ROS users have many times observed weird behavior inconsistencies between rospy/rosparam and roscpp when using parameters (https://github.com/ros/ros_comm/issues/1339, https://github.com/ros/ros_comm/issues/1153, https://github.com/ros/ros_comm/issues/445, https://github.com/ros/ros_comm/issues/390, https://github.com/ros/ros_comm/issues/436). I'm not sure about the other client libraries.

I think there needs to be an agreement made that this is unacceptable: users of nodes really should not care whether the nodes are written using rospy, roscpp, rosrust or whatever client library... to just know what kind of behavior they can expect from rosrun and rosparam. AFAIK, requirement for stable param API across all client libraries is not covered by any REP. If it is, please, point me to it. The only mention I found is that ROS uses YAML syntax to determine the parameter typing, which is an optional feature of client libraries.

I hope everybody would agree that rospy should be taken as the reference implementation (as e.g. rosparam is written using rospy). Other client libraries should therefore test their API interoperability with rospy.

I see two action points now:

Knowing about https://github.com/ros/ros_comm/issues/1496, I can work on the implementation. Just before I start doing so, I'd like to know it is something that has a high probability of being merged (not necessarily to melodic or older releases, but at least to current master). And another question - is it okay to introduce yaml-cpp as a dependency to libros? I know this library is a bit cursed with all its version/API incompatibilities, but at least rviz also depends on it. However, introducing this dependency to libros would mean yaml-cpp will get installed even with ros-DISTRO-ros-base, whereas now I think it only gets installed with -desktop.

This change will definitely change behavior of some roscpp programs, but as I said in https://github.com/ros/ros_comm/issues/436 , it should only break code that is already broken (but the authors did not notice/care).

dirk-thomas commented 5 years ago

Maybe it would be good to start with a REP to formalize this. That document could then also clearly describe what breaking changes are necessary to make the different client libraries consistent. Depending how these breaking changes look like the decision if / when it can be merged will be made.

peci1 commented 5 years ago

Sounds reasonable. Should I prepare a draft, or does Open robotics want to lead this effort?

dirk-thomas commented 5 years ago

Please go ahead. I don't think anyone at OSRF will be able to spend time on this (beside providing feedback of course).

peci1 commented 5 years ago

Okay, I'll try my best :)