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

wildcard filtering expression for sequences #11

Closed
iuhilnehc-ynos opened this issue Mar 23, 2021 · 5 comments
Closed

wildcard filtering expression for sequences #11

iuhilnehc-ynos opened this issue Mar 23, 2021 · 5 comments
Labels
connext Issue related to the RTI Connext DDS libraries enhancement New feature or request

Comments

@iuhilnehc-ynos
Copy link
Collaborator

Feature Description

Content filtered topic feature in rti-connext-dds is excellent, especially supporting arrys and sequences.

We decide to use this feature to filter data of ROS2 topics including "/parameter_events", "${concrete_action_name}/_action/feedback" and "${concrete_action_name}/_action/status", but we are experiencing a problem that is not filtering data for sequences of uncertain size and unknown item location.

For topic "/parameter_events", the publisher will publish data as following,

  • ParameterEvent_.idl
    struct ParameterEvent {
      ...
      sequence<rcl_interfaces::msg::Parameter> new_parameters;
      ^^^^^^^^/\ the type of new_parameters is 'sequence'
      ...
    };
  • data sample
stamp:
  sec: 1616464433
  nanosec: 595625771
node: /parameter_blackboard
new_parameters:                 # 1. sequence size is unknown for the subscriber,
                                # so we can't to use filter expression with
                                # "new_parameters[0].name.data MATCH 'use_sim_time' or ..."
- name: use_sim_time            # 2. the order of items for the 'name' is also uncertained,
                                # so we can't to use filter expression with
                                # "new_parameters[0].name.data MATCH 'use_sim_time'"
                                # e.g. if there are 10 items, 'use_sim_time' might be located on 7th, 9th, etc.
  value:
    type: 1
    bool_value: false
    integer_value: 0
    double_value: 0.0
    string_value: ''
    byte_array_value: []
    bool_array_value: []
    integer_array_value: []
    double_array_value: []
    string_array_value: []
changed_parameters: []
deleted_parameters: []
---

We can filter the node name with a filter expression "node MATCH '/parameter_blackboard'", but it seems there is no SQL grammar in rti-connext-dds to filter new_parameters with a specific name.

The publishers for topic "${concrete_action_name}/_action/status" are also publishing status data with uncertain size and index,
so we can't set the filter expression with a specific 'uuid' on the subscriber side.

status_list:             # uncertain size for 'goal_info'
- goal_info:
    goal_id:
      uuid:              # uncertain locatation(index) for 'uuid'.
      - 145
      - 2
      - 147
      - 138
      - 43
      - 172
      - 91
      - 12
      - 13
      - 111
      - 134
      - 245
      - 168
      - 79
      - 14
      - 191
    stamp:
      sec: 1616465176
      nanosec: 998640169
  status: 2

Implementation Considerations

I think it's a very useful feature to filter these data, do you plan to support the feature like wildcard filtering expression for sequences, such as "new_parameters[*].name MATCH 'use_sim_time'", "status_list[].goal_info.goal_id.uuid = &hex(%s)" or other non-number related statement.

NOTE: I'm sorry if it's not the correct place to post this feature request.

@fujitatomoya
Copy link
Collaborator

@asorbini
CC: @iuhilnehc-ynos

sorry to interrupt you all the sudden, but as you know we are trying to enable ContentFilteredTopic to ROS2.
for the use case of ros2 action, we need to come up with this requirement.
would you mind giving us some feedback as in DDS aspect and RTI implementation.

thanks in advance.

@asorbini
Copy link
Collaborator

Hi @iuhilnehc-ynos and @fujitatomoya,

thank you for reaching out, and for filing this issue. I'm glad to see you have been making progress with your work with rmw_connextdds and RTI Connext DDS, and what you have identified sounds like a very interesting use case for Connext.

I was trying to think of possible workarounds to overcome the current lack of support for wildcards, and the "unbounded" nature of these fields really precludes any good solution. If the fields were statically bounded, then one could probably mimic a wildcard by ORing an expression on every element (i.e. foo[0].name MATCH "bar" OR ... OR foo[N].name MATCH "bar"), although that would most likely end up with a very long filter expression, which would likely require tuning of QoS resource limits to properly propagate via discovery information...

In the case of unbounded sequences, one could possibly follow this strategy by establishing some hard-coded static limit, maybe tuned based on the actual needs of an application, but it would be a "hack" at best.

In general, this sounds like a "core" feature that will need to be addressed in the Connext libraries rather than in the RMW, so I have propagated the question internally at RTI, and I'll get back to you as soon as I hear back from our development team.

Meanwhile, would you mind me asking if you are planning on pushing your CFT improvements (e.g. to rcl, but also rmw_connextdds) into Galactic?

As you probably know, we are working toward replacing rmw_connext_cpp with rmw_connextdds in the next ROS2 release, and we are pretty close to finally having rmw_connextdds included in Rolling.

Once that happens, I might have some time to review any changes you made to rmw_connextdds, just in case you'd like some input, and to maybe squeeze in the code before the freeze.

@fujitatomoya
Copy link
Collaborator

@asorbini

thanks for the quick response 👍

In general, this sounds like a "core" feature that will need to be addressed in the Connext libraries rather than in the RMW, so I have propagated the question internally at RTI, and I'll get back to you as soon as I hear back from our development team.

that would be really appreciated. one thing we are not sure is that this requirement for wildcard is actually something should be supported or nice to have? i am expecting that unbound array/sequence is nice to have. if that so, probably rmw would not want to rely on specific implementation's feature but general specification. so we would need to find other way around, such as changing ros2 action status topic message type or so on.

Meanwhile, would you mind me asking if you are planning on pushing your CFT improvements (e.g. to rcl, but also rmw_connextdds) into Galactic?

that was a plan, we have been working on that. but according to the timeline and it's kinda big change for all stacks, i believe that it is unlikely to have this CFT in Galactic, we will try but it's gonna be tight.

and we are pretty close to finally having rmw_connextdds included in Rolling.

sounds great ❗ we've been watching this since our CFT is dependent on rmw_connextdds to enable. (no other implementation does support CFT currently, as far as i know)

Once that happens, I might have some time to review any changes you made to rmw_connextdds, just in case you'd like some input, and to maybe squeeze in the code before the freeze.

thanks again! no pressure at all, we just don't interrupt your work here. so when you got time, that would be appreciated!

@asorbini asorbini added the enhancement New feature or request label Mar 23, 2021
@asorbini
Copy link
Collaborator

that would be really appreciated. one thing we are not sure is that this requirement for wildcard is actually something should be supported or nice to have? i am expecting that unbound array/sequence is nice to have. if that so, probably rmw would not want to rely on specific implementation's feature but general specification. so we would need to find other way around, such as changing ros2 action status topic message type or so on.

At this point it's definitely a nice to have, because it's not a straightforward change, and it would be great to address it in the spec for interoperability, although it looks like Connext already supports several extensions, after taking a look at the SQL-like grammar in Appendix B of the DDS Spec.

One aspect I had not considered earlier is that we already use * as a wildcard to match 0 or more non-special characters. In order to support a "collection wildcard", we would probably need to select another character (maybe _?) to avoid complicating the parsing of a string.

Discussing your proposal internally, I was reminded that Connext supports custom content filters, where a user can install a content filter based on a custom implementation they provide. I know this is not a portable solution, but we have an example of how to use this facility with filters expressed as "lambdas", and it might be a way to test the effects of filtering strategies which are not yet supported by the built-in SQL filter. I haven't tested custom content filters myself, so let me know if you would be interested in trying them. I expect some possible "quirks" in using these filters on top of rmw_connextdds, mostly because of the custom "type plugin" used by the RMW implementation to handle ROS2 messages (instead of the data structures generated by rtiddsgen).

thanks again! no pressure at all, we just don't interrupt your work here. so when you got time, that would be appreciated!

Happy to help. I saw @iuhilnehc-ynos opened #12 and I'll try to provide feedback soon!

I'm going to close the issue because I don't expect any direct action item for rmw_connextdds to come out of it, but we can continue to use it to discuss the topic, and I'll use it to provide updates in the future, if any changes in Connext make it possible to use this type of content filters.

@asorbini asorbini added the connext Issue related to the RTI Connext DDS libraries label Mar 23, 2021
@fujitatomoya
Copy link
Collaborator

@asorbini thanks for the insights.

cwecht pushed a commit to cwecht/rmw_connextdds that referenced this issue Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connext Issue related to the RTI Connext DDS libraries enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants