search

ros2 / rmw_fastrtps

Implementation of the ROS Middleware (rmw) Interface using eProsima's Fast RTPS.
Apache License 2.0
157 stars 117 forks source link

Unique Client GID for Service Introspectino Event. #779

Closed fujitatomoya closed 2 months ago

fujitatomoya commented 2 months ago

part of https://github.com/ros2/rmw/issues/357

fujitatomoya commented 2 months ago

@MiguelCompany i would like to know what you think about this fix, could you take a look?

fujitatomoya commented 2 months ago

This PR fixes the issue https://github.com/ros2/rmw/issues/357

info:
  event_type: 0
  stamp:
    sec: 1726873574
    nanosec: 967201238
  client_gid: [1, 15, 163, 43, 147, 68, 18, 20, 0, 0, 0, 0, 0, 0, 21, 3]
  sequence_number: 1829
request: [{a: 2, b: 3}]
response: []
---
info:
  event_type: 1
  stamp:
    sec: 1726873574
    nanosec: 967516329
  client_gid: [1, 15, 163, 43, 147, 68, 18, 20, 0, 0, 0, 0, 0, 0, 21, 3]
  sequence_number: 1829
request: [{a: 2, b: 3}]
response: []
---
info:
  event_type: 2
  stamp:
    sec: 1726873574
    nanosec: 967806970
  client_gid: [1, 15, 163, 43, 147, 68, 18, 20, 0, 0, 0, 0, 0, 0, 21, 3]
  sequence_number: 1829
request: []
response: [{sum: 5}]
---
info:
  event_type: 3
  stamp:
    sec: 1726873574
    nanosec: 968045402
  client_gid: [1, 15, 163, 43, 147, 68, 18, 20, 0, 0, 0, 0, 0, 0, 21, 3]
  sequence_number: 1829
request: []
response: [{sum: 5}]
fujitatomoya commented 2 months ago

Pulls: ros2/rmw_fastrtps#779 Gist: https://gist.githubusercontent.com/fujitatomoya/24d1b9af9d8b36fe3501d79b78658833/raw/e9b7750a54ef63afe0206b53106c12aaec6ecc73/ros2.repos BUILD args: --packages-above-and-dependencies rmw_fastrtps_shared_cpp TEST args: --packages-above rmw_fastrtps_shared_cpp ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14587

ahcorde commented 2 months ago

Pulls: ros2/rmw_fastrtps#779 Gist: https://gist.githubusercontent.com/ahcorde/8df2c5e69dda0758460e6f1e7d63527c/raw/e9b7750a54ef63afe0206b53106c12aaec6ecc73/ros2.repos BUILD args: --packages-above-and-dependencies rmw_fastrtps_shared_cpp --packages-above-and-dependencies rmw_fastrtps_shared_cpp TEST args: --packages-above rmw_fastrtps_shared_cpp --packages-above rmw_fastrtps_shared_cpp ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14594

fujitatomoya commented 1 month ago

@MiguelCompany so this did not work, actually it generated the instability for CI. (not often happen, but reproducible with CI)

i am not 100 sure but i believe that the following behavior change can be a reason for this CI hang-up problem.

https://github.com/ros2/rmw_fastrtps/blob/6bdff0b4146ed9821008b2969740d9332d6ee6dc/rmw_fastrtps_shared_cpp/src/rmw_response.cpp#L140-L156

that said, we cannot keep the request publisher gid in rmw_request_id_t * request_header. i do not have good idea to keep (or get?) the request publisher gid during the process of receiving request and sending response, unless adding the field in rmw_request_id_t. any thoughts?

MiguelCompany commented 1 month ago

@fujitatomoya I think we could use in the client the same trick we use in the server, make the request id use the GUID of the response reader, instead of the request writer