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

Allow to recursively set alists with string keys in ros::set-param #710

Merged
merged 5 commits into from
Jul 27, 2022

Conversation

Affonso-Gui
Copy link
Member

Allows to recursively set values organized in alists at ros::set-param.

# test.yaml
that:
  test:
    a: 1
    b:
      - 1
      - 2
      - 3
(ros::roseus "test")
(ros::get-param "/that")
;; (("test" ("a" . 1) ("b" 1 2 3)))

(ros::set-param "/this" (ros::get-param "/that"))

(ros::get-param "/this")
;; (("test" ("a" . 1) ("b" 1 2 3)))

Needs discussion & testing.

Partially related to #709

@k-okada
Copy link
Member

k-okada commented May 1, 2022

if this feature is supported by roscpp/rospy, we'd better to support in roseus

@Affonso-Gui
Copy link
Member Author

Yes, this is supported in both rospy and roscpp:

# test.py
rospy.init_node("py")

rospy.get_param("/that")
# {'test': {'a': 1, 'b': [1, 2, 3]}}

rospy.set_param("/this", rospy.get_param("/that"))

rospy.get_param("/this")
# {'test': {'a': 1, 'b': [1, 2, 3]}}
// test.cpp
#include <ros/ros.h>

int main(int argc, char **argv)
{
  ros::init(argc, argv, "cpp");
  ros::NodeHandle n;

  XmlRpc::XmlRpcValue val;
  n.getParam("/that", val);

  n.setParam("/another", val);

}
$ rosparam get this
test:
  a: 1
  b: [1, 2, 3]

$ rosparam get that
test:
  a: 1
  b: [1, 2, 3]

$ rosparam get another
test:
  a: 1
  b: [1, 2, 3]

@Affonso-Gui
Copy link
Member Author

Actually, roseus does support recursive ros::set-param through alists, it is only not able to interpret string names, which happen to be the format returned by ros::get-param.

I have adjusted the code to accept string params as well, and added test code.

@Affonso-Gui Affonso-Gui changed the title Allow to recursively set alists in ros::set-param Allow to recursively set alists with string keys in ros::set-param May 2, 2022
@Affonso-Gui
Copy link
Member Author

Affonso-Gui commented May 12, 2022

Found another problem:

irteusgl$ (ros::set-param "test" '(((a . 1) (b . 2))))
[ERROR] [1652371517.860218315] [${node}]: ROSEUS_SET_PARAM: EusValueToXmlRpc: assuming symbol
(a . 1)
t

This corresponds to a list of dictionaries, as in the following:

test:
  - {a: 1, b: 2}

This is because in https://github.com/jsk-ros-pkg/jsk_roseus/blob/master/roseus/roseus.cpp#L1282 an alist is defined as a cons with a cons car. A list of dictionaries meets this criteria, but does not have a key element, thus leading to the error.

Added tests & fixed.

@Kanazawanaoaki

@k-okada k-okada merged commit d84c4d2 into jsk-ros-pkg:master Jul 27, 2022
@Affonso-Gui Affonso-Gui deleted the recursive-set-param branch July 27, 2022 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants