fix(vision): correct LCM serialization for detection class_id and results_length

[jongmoon-konglabs]

Problem

The robot's detection data was not being correctly transmitted via LCM due to two primary serialization bugs:

1. Type Mismatch: ObjectHypothesis.class_id is defined as a string in the LCM schema, but we were passing an int. This caused an AttributeError: 'int' object has no attribute 'encode' during the lcm_encode() process.
2. Incorrect Length: results_length in the Detection2D message was either hardcoded or incorrectly handled, leading to empty detection arrays or index errors during deserialization.
3. Incomplete Restoration: When decoding from LCM, the class_id (string) was not being converted back to an int, causing type consistency issues within the Detection2DBBox object.

Solution

• Type Casting: Modified to_ros_detection2d in both Detection2DBBox and Detection2DPoint to explicitly cast class_id to str() for LCM encoding.
• Dynamic Results Length: Updated the ROSDetection2D message creation to set results_length=len(results) dynamically, ensuring the LCM library knows exactly how many objects to serialize.
• Data Restoration: Updated from_ros_detection2d to cast the decoded string class_id back to an int to maintain compatibility with the rest of the perception pipeline.
• Unit Testing: Added a suite of mock-based unit tests in test_imageDetections2D.py that validates the full LCM round-trip (encode -> decode) without requiring a physical camera or heavy fixtures.

Breaking Changes

None

How to Test

1. Unit Tests: Run the new serialization tests to verify the fix:

   pytest dimos/perception/detection/type/detection2d/test_imageDetections2D.py

[greptile-apps]

Greptile Summary

Fixed critical LCM serialization bugs that prevented detection data transmission. The PR correctly addresses three issues: converts class_id from int to string for LCM encoding, sets results_length dynamically instead of hardcoding, and restores class_id back to int after LCM decoding.

Key Changes:

• to_ros_detection2d() in both Detection2DBBox and Detection2DPoint now explicitly converts class_id to string using str() before LCM serialization
• Added dynamic results_length=len(results) to ensure correct array sizing for LCM
• from_ros_detection2d() converts decoded string class_id back to int with validation using .lstrip("-").isdigit()
• Comprehensive unit tests validate the full encode → decode roundtrip for multiple class IDs

Minor Note:
The validation logic in bbox.py:376 uses .lstrip("-").isdigit() which could theoretically accept malformed strings like "--5". Consider using a more robust pattern like str(hypothesis.class_id).lstrip("-").replace("-", "", 1).isdigit() or exception handling with try/except for int conversion.

Confidence Score: 4/5

• This PR is safe to merge with one minor consideration regarding edge case handling
• The changes correctly address the LCM serialization issues with comprehensive unit tests. The string conversion logic handles negative numbers properly, and the dynamic results_length fixes the array length bug. One minor concern is the validation logic uses .lstrip("-").isdigit() which could accept strings like "--5", though this is unlikely in practice.
• Pay close attention to bbox.py line 376 - the string validation logic could be more robust

Important Files Changed

Filename	Overview
dimos/perception/detection/type/detection2d/bbox.py	Adds proper class_id string conversion for LCM serialization and dynamic results_length - includes string parsing logic with .lstrip("-").isdigit() for robustness
dimos/perception/detection/type/detection2d/point.py	Adds class_id string conversion and dynamic results_length for LCM serialization - straightforward change matching bbox.py pattern
dimos/perception/detection/type/detection2d/test_imageDetections2D.py	Adds comprehensive unit tests for LCM serialization roundtrip with parametrized test cases - validates encoding, decoding, and type conversions

_{Last reviewed commit: 1c0ff7d}

[greptile-apps]

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[leshy]

I remember investigating this on my end, and concluded that weirdly, ROS vision messages does define class_id as string

https://github.com/ros-perception/vision_msgs/blob/ros2/vision_msgs/msg/ObjectHypothesis.msg

https://docs.ros.org/en/rolling/p/vision_msgs/msg/ObjectHypothesis.html

[leshy]

Since String class_id is a legitimate attribute in ROS, I think perhaps we should conform to this on our end as well and simply make our class_ids strings as well in order to maintain full interop with ROS detections.

IDK what's the rationale in ROS for this, but I assume it allows us to mark things as "a chair" etc. How does this sound?

[jongmoon-konglabs]

@leshy
You're absolutely right.

I looked into the rationale behind ROS defining class_id as a string (vision_msgs/ObjectHypothesis). As you suspected, it is intentionally designed for high extensibility—it allows the use of UUIDs, semantic labels (like "chair" or "person"), and namespaces (e.g., "yolo/person") without forcing downstream nodes to rely on an external label-map database to decode integers.

ros-perception/vision_msgs#43

The reason we initially had it as an int on our end was simply because YOLO/PyTorch natively outputs detection classes as integer tensor indices. However, I completely agree that we should conform to the ROS standard and natively use strings to maintain full interoperability.

I dug into YOLO's Results object already contains a built-in label mapping table (names dict, e.g., {0: 'person', 1: 'bicycle', ...}). We are actually already using this table to extract the string for the name attribute (around bbox.py:233).

So, instead of just casting the integer to a string (e.g., "0"), our transition would look like this:

• AS-IS: class_id = 0 (int), name = "person" (str)
• TO-BE: class_id = "person" (str), name = "person" (str)

One quick question for you:
Because we already have a separate name attribute in our Detection2DBBox dataclass, updating class_id to a string means both class_id and name will hold the exact same value (e.g., "person"). Are you okay with this redundancy?
(Alternatively, we could format the class_id to be something like "0:person" like {class_id}:{name} format)