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

unable to namespace parameters #373

Closed
Karsten1987 opened this issue Jun 19, 2019 · 6 comments
Closed

unable to namespace parameters #373

Karsten1987 opened this issue Jun 19, 2019 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@Karsten1987
Copy link
Contributor

Bug report

Just like in the cpp demos, I'd like to be able to namespace parameters in the form of <namespace>.<parameter_name>. However, in python I believe the wrong validation check is performed. Looking at the error output, it looks like the validation for topic names is applied rather than parameters.

Steps to reproduce issue

node.declare_parameter(node.get_name() + '.diagnostics_update', 1).value

Expected behavior

Able to declare a parameter with namespace in the form of 'namespace.parameter_name'.

Actual behavior

result = _rclpy.rclpy_get_validation_error_for_topic_name(name)
if result is None:
  return True
error_msg, invalid_index = result
raise InvalidParameterException(name, error_msg, invalid_index)
rclpy.exceptions.InvalidParameterException: ('Invalid parameter name', 
  'test_node.diagnostics_update', "topic name must not contain characters other than alphanumerics, '_', '~', '{', or '}'", 9)
@Karsten1987 Karsten1987 added the bug Something isn't working label Jun 19, 2019
@wjwwood
Copy link
Member

wjwwood commented Jun 20, 2019

@jubeira FYI

@jubeira
Copy link
Contributor

jubeira commented Jun 21, 2019

Thanks for the report.
In rclcpp the only validation being performed ATM is checking that the name is not empty:

https://github.com/ros2/rclcpp/blob/e7c463dc74849aa64cb8a74e13f2a57b4ed59862/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp#L376-L379

Unlike topic name validation, AFAIK there is no function to validate a parameter name at rcl level and perhaps it would be a good idea to have it.

I see two ways of proceeding:

  • Make the validation just an empty check temporarily as in rclcpp, and file a feature request in rcl to add topic name validation. Once implemented, start using that functionality in rclpy.
    • Note: I suggest making the check empty temporarily rather than waiting for the fix at rcl level since this issue might be blocking other developments in the meantime.
  • Implement the check in Python. Perhaps this would be faster than implementing it in rcl, but it would be not related with the eventual solution in rclcpp, which is not consistent with the current solution to validate topic names.

Personally, I suggest going for the first one. @wjwwood thoughts?
/cc @sloretz

@jubeira jubeira self-assigned this Jun 21, 2019
@joncppl
Copy link

joncppl commented Jun 21, 2019

Not sure if this is this is the same issue, related, or a separate issue, but my team has observed what we believe to be incorrect namespacing behaviour in declare_parameters.

The parameter names returned are namespace concatanated with the parameter name, without a separating .

ie. The full name returned from declare_parameters(ns, [(name, default)]) is ns + name but we expect ns + '.' + name

@jubeira
Copy link
Contributor

jubeira commented Jun 21, 2019

The parameter names returned are namespace concatanated with the parameter name, without a separating .

@joncppl That seems to be a separate issue; thanks for pointing it out.
I'll file a ticket and send a PR with a fix.

@joncppl
Copy link

joncppl commented Jun 21, 2019

Thanks!

@jubeira
Copy link
Contributor

jubeira commented Jun 26, 2019

The problem described in this report has been addressed in #377.
I've opened a ticket to track the implementation for parameter name validation in #380 as a feature request.

@Karsten1987 shall we close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants