Transform between same frames always exist, even when frame ids are empty strings

[mak22223]

Generated by Generative AI

No response

Operating System:

Linux lightweight 5.15.88-051588-generic #202301141034 SMP Sat Jan 14 10:39:07 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

ROS version or commit hash:

humble

RMW implementation (if applicable):

No response

RMW Configuration (if applicable):

No response

Client library (if applicable):

rclcpp

'ros2 doctor --report' output

ros2 doctor --report

   NETWORK CONFIGURATION
inet         : 127.0.0.1
inet4        : ['127.0.0.1']
inet6        : ['::1']
netmask      : 255.0.0.0
device       : lo
flags        : 73<LOOPBACK,UP,RUNNING>
mtu          : 65536
inet         : 172.17.0.3
inet4        : ['172.17.0.3']
ether        : 36:76:1d:a9:52:d1
netmask      : 255.255.0.0
device       : eth0
flags        : 4163<MULTICAST,UP,BROADCAST,RUNNING>
mtu          : 1500
broadcast    : 172.17.255.255

   PLATFORM INFORMATION
system           : Linux
platform info    : Linux-5.15.88-051588-generic-x86_64-with-glibc2.35
release          : 5.15.88-051588-generic
processor        : x86_64

   QOS COMPATIBILITY LIST
compatibility status    : No publisher/subscriber pairs found

   RMW MIDDLEWARE
middleware name    : rmw_fastrtps_cpp

   ROS 2 INFORMATION
distribution name      : humble
distribution type      : ros2
distribution status    : active
release platforms      : {'rhel': ['8'], 'ubuntu': ['jammy']}

   TOPIC LIST
topic               : none
publisher count     : 0
subscriber count    : 0

Steps to reproduce issue

1. Try to call canTransform() between (C++) "" and "" at tf2::TimePointZero.
2. Try to call lookupTransform() between same frames and at the same time as in step 1.

Expected behavior

1. Return false
2. Throw exception

Actual behavior

1. It returns true.
2. It will return default "zero" transform.

Additional information

This might be an intentional 'feature' to speed up lookups, but it results in returning transforms between non-existent frames, which is misleading.

[andschi]

I checked the implementation of BufferCore::canTransform. The expected behaviour would be achieved if the "Short circuit" and validateFrameId were swapped:

  CompactFrameID target_id = validateFrameId(
    "canTransform argument target_frame", target_frame, error_msg);
  if (target_id == 0) {
    return false;
  }
  CompactFrameID source_id = validateFrameId(
    "canTransform argument source_frame", source_frame, error_msg);
  if (source_id == 0) {
    return false;
  }

  // Short circuit if target_frame == source_frame
  if (target_frame == source_frame) {
    return true;
  }

Also, BufferCore::lookupTransformImpl should do validateFrameId before handling the identity tf case.

If this is intentional for performance reasons, I would say the benefit is really minor as validateFrameId does check if frame_id.empty() right at the start.