Publish serialized FlatBuffers from the tracker API by aristarkhovNV · Pull Request #865 · NVIDIA/IsaacTeleop

aristarkhovNV · 2026-07-31T22:41:19Z

Description

Replaces the generated object-API (-T) types in the DeviceIO tracker query API with
Serialized<T>, an owning handle over an encoded buffer. Reads go through the generated
accessors straight into the bytes: no unpack, no per-field allocation, and no -T in any
public signature.

No wire or recording format change.

Why

The tracker API handed out -T types. A -T is a tree of vector / string /
shared_ptr members that only exists after an UnPack, so a tracker had two options and
both were bad: deep-copy one per read, or lend out storage it would refill on the next
frame. A hand is 936 bytes of joints, read twice a frame at 90 Hz.

The lending option is what shipped, which is why the Python accessors carried a "valid
until the next session.update()" caveat.

Key decisions

1. Trackers publish their payload table directly — the ten `Tracked` wrappers are deleted (breaking)

Trackers returned a wrapper table whose only field was an optional payload. Since the handle
is itself nullable, the wrapper encoded absence a second time and every read paid a
two-step null check. Trackers now hand out the payload table, so a read loses a .data hop.

The wrapper was not redundant when it was introduced (eedaa3ee): it replaced an in-band
is_active flag, making absence structural rather than an ignorable bool. That needed a
wrapper because the API returned a -T by reference, which always exists, so absence had
to live inside the returned object. The handle removes that constraint. Absence stays
structural and is now enforced rather than merely expressed.

MessageChannelMessagesTracked stays: its data is a list, and a batch needs a table to
hold the vector. "No messages this frame" is an empty batch, not an absent one.

2. Python gets `None` for an inactive device (breaking)

An empty handle answers field reads with defaults, so exposing it to Python would let a
disconnected pedal read left_pedal == 0.0 — indistinguishable from a pedal at rest — and an
absent head pose report is_valid == False as though tracking had merely been lost. Every
missed check would fail silently.

Tracker accessors map the empty handle onto None at the binding boundary, so a missed check
raises AttributeError instead. The DeviceIO tensor types accept None as a slot value and
the source nodes test is None rather than falsiness.

3. Python builds payloads through constructors, never setters (breaking)

There are no field setters on the bound types. A constructor assembles a -T as a C++ local
and pack()s it, so the encoder is always the generated Pack and cannot drift from the C++
readers — while the -T itself stays invisible. Tests that synthesised values by assigning
fields now pass them to the constructor.

4. Python class names drop the trailing `T` (breaking, with aliases)

Deprecated aliases resolve through the schema module's existing __getattr__, so old names
keep working and warn.

5. No recording or wire format change

Every root_type under mcap/ is a Record; no wrapper was ever a recorded root. Existing
MCAP files replay unchanged.

6. Structs and the zero-copy NumPy views are untouched

flatc emits one struct type for both APIs, so Pose / HandJoints bindings and the
strided joint views from #836 keep working — now reached through a table view that owns the
buffer they alias.

7. The published type is a handle over encoded bytes

Serialized<T> (schema/serialized.hpp) is a shared_ptr to an immutable buffer plus a
pointer to one table inside it. Copying is a refcount bump. narrow() re-points within the
same buffer while sharing the owner, so one allocation backs a whole tree of views.

8. The handle is nullable, for a format reason rather than a domain one

A FlatBuffers table field is optional — the generated accessor already returns null when
unset. A non-nullable handle could therefore represent less than the pointer it wraps, and
narrow() would have to return an optional, which relocates the null rather than removing
it. It would also cost the by-reference returns and default-constructibility the tracker
impls rely on.

operator bool gates access and operator-> asserts, so an empty dereference is not silent.

9. Each `update()` publishes a new buffer instead of refilling the previous one

This is what retires the "valid until the next session.update()" caveat. Consumers hold
snapshots: a handle read this frame keeps its values after the tracker moves on.

10. `-T` survives as internal assembly scratch, named `native_`

Impls that build a payload from an OpenXR query still fill a -T and encode from it. The
rule is that it never escapes, and that every exit path of update() encodes — including
the throwing ones, otherwise a consumer keeps reading last frame's snapshot after the
device drops out.

11. Live and replay both adopt the buffer they were handed, rather than round-tripping it

This is the largest structural simplification, and it applies on both paths for the same
reason: the bytes already carry the payload table, which is now exactly what consumers read.
See Measurements for what it is and is not worth.

Live — SchemaTracker publishes the final sample by taking ownership of its buffer,
where before it unpacked and re-encoded. The only remaining unpack is for MCAP, whose
writer takes a native, and it is gated on recording being enabled.

Replay — McapTrackerViewers grows read_serialized(), which verifies the recorded
bytes and adopts them. The recorded root is a Record whose data is byte-for-byte the
payload table, so the eight single-payload impls (ten channels) narrow() to it and share
the one buffer instead of unpacking a -T and re-encoding it every frame.

read() remains for the message channel, which composes a new batch table out of the
records it drains and so genuinely needs owning members. It is now implemented as
read_serialized() plus an UnPack, so there is one iterator, one buffer, and no way for
the two to disagree about ordering.

12. Immutability is a contract, not an enforcement

The Python bindings hand out writable NumPy views over the joint arrays, because NumPy
cannot export a read-only array over DLPack before 2.1. Writing through one changes what
every holder of that buffer sees. The header says so explicitly rather than claiming an
immutability it does not enforce, and no copy protocol is offered: identity would alias those
writes across the pipelined worker boundary that asks for the copy.

Measurements

Both paths start from the same recorded buffer and end holding a Serialized<Payload>, so
only the middle differs: the round trip verifies, UnPackTos a -T and re-encodes it into a
fresh builder; adopt verifies, copies the bytes once and re-points into them. Best-of-7
over 20k iterations, -O2. Allocation counts are from a global operator new counter, not
estimated.

schema	recorded	round trip	adopt	verify only	ratio	allocations
`HandPose` (26 joints)	1000 B	~140 ns	~42 ns	5 ns	3.4×	8 → 2
`FullBodyPose` (24 joints)	832 B	~143 ns	~41 ns	5 ns	3.5×	8 → 2
`Generic3AxisPedal`	80 B	~98 ns	~36 ns	5 ns	2.7×	6 → 2

The copy is not the cost. Verify is 5 ns and the whole of adopt is ~40 ns, of which the
memcpy is under 10 ns — the 80-byte pedal costs 36 ns against the 1000-byte hand's 42 ns, so
920 extra bytes buy about 6 ns. What is left is the two allocations. Anyone wanting this
faster should pool the buffer rather than try to remove the copy.

The wall-clock saving is small; the allocation churn is the point. Replay reads ten
channels per frame, so it goes from ~1.1 µs to ~0.38 µs — 0.7 µs on an 11 ms frame, about
0.007%. That is not a frame-time improvement and should not be read as one. What changes
materially is allocation traffic: ~70 allocations per replay frame become 20, or roughly
6,300/s down to 1,800/s at 90 Hz, which is what matters for jitter and fragmentation in a
real-time loop.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update

Breaking changes are decisions 1–4 above: the Tracked wrappers are gone (a read loses its
.data hop), Python tracker accessors return None for an inactive device, bound types have
constructors instead of setters, and the Python class names drop their trailing T (aliased
and warned, not removed).

Testing

Full build and ctest: 310/310 pass, Linux / gcc 13 / Debug.
Every commit builds and passes tests independently — verified individually, since the stack
changes the same signatures in stages.
Existing coverage carried the refactor: the replay session integration tests exercise all
eight converted replay impls, and the schema Python tests were rewritten onto the
constructors (decision 3).
SKIP=check-copyright-year pre-commit run --all-files is clean.
The Measurements table above comes from a standalone benchmark compiled at -O2, best-of-7
over 20k iterations, with allocation counts taken from a global operator new counter.

Known gap: McapTrackerViewers::read_serialized() has no direct unit test. It is covered
transitively — the 15 existing read() cases in test_mcap_tracker_channels.cpp now route
through it, and the replay integration tests exercise it per frame — but nothing asserts its
own contract (empty handle means end-of-stream; non-empty handle with a null payload()
means an inactive tracker). Happy to add those cases if reviewers want them here rather than
as a follow-up.

Checklist

I have read and understood the contribution guidelines
I have run the linter and formatter with SKIP=check-copyright-year pre-commit run --all-files
I have made corresponding changes to the documentation
I have added tests that prove my fix/feature works (or explained why not) — see the known gap above
I have signed off all my commits (git commit -s) per the DCO

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Tracker APIs now provide serialized, read-only data views for hand, head, body, controller, pedal, tactile, joint, pose, metadata, haptic, and message-channel data.
- Python bindings expose encoded schema objects with constructor-based creation, safe field access, and None for unavailable data.
- MCAP playback supports serialized record access while retaining native record reading.
Documentation
- Updated API guidance, examples, and usage documentation for empty handles, nullable results, and direct data access.
Breaking Changes
- Legacy tracked-wrapper types and mutable Python schema objects have been removed or replaced.

The tracker query API handed out generated object-API (-T) types. A -T is a tree of vector/string/shared_ptr members that only exists after an UnPack, so a tracker either deep-copied one per read or lent out storage it would refill on the next frame; a hand is 936 bytes of joints, read twice a frame. Trackers now publish Serialized<T> (schema/serialized.hpp): a shared_ptr to an immutable encoded buffer plus a pointer to one table inside it. Reads go through the generated accessors straight into the bytes, so nothing unpacks and no -T appears in any public signature. Copying a handle is a refcount bump, and each update() publishes a new buffer rather than refilling the previous one, so a snapshot read this frame keeps its values afterwards -- which retires the "valid until the next session.update()" note the Python accessors carried. Impls keep a -T as internal assembly scratch, named native_, and encode from it on every exit path of update() including the throwing ones. An inactive device publishes an empty handle rather than a buffer holding a null data, so consumers test one condition. The handle is nullable deliberately: a FlatBuffers table field is optional and the generated accessor already returns null when unset, so a non-nullable handle could represent less than the pointer it wraps and would force narrow() to hand back an optional instead. operator-> asserts rather than leaving an empty dereference silent. Python no longer reaches the object API at all. Each table binds to one class over Serialized<T>, built by its own constructor with the generated Pack underneath, so producing a payload from Python uses the same encoder as C++ and cannot drift from it. Struct bindings and the zero-copy NumPy joint views are untouched -- flatc emits one struct type for both APIs -- and the names drop their trailing T, with deprecated aliases resolved through the schema module's existing __getattr__. Tests that synthesised -T values by assigning fields now pass them to the constructor, which is the only way to build these from Python. The encoded views are immutable by contract but not by enforcement, since the joint views must stay writable for DLPack, so no copy protocol is offered: identity would alias those writes across the pipelined worker boundary that asks for the copy. Signed-off-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>

Trackers returned Serialized<XTracked>, a wrapper table whose only field was an optional payload. Serialized<T> is itself nullable -- an empty handle means "no table here", mirroring FlatBuffers' own optional table fields -- so the wrapper encoded absence a second time, and every read paid a two-step null check that payload() existed to hide. The wrapper predates that. eedaa3e introduced it to replace an in-band is_active flag, making absence structural rather than an ignorable bool. That needed a wrapper because the API returned a -T by reference, which always exists, so absence had to live inside the returned object. The handle removed that constraint. Absence stays structural and is now enforced rather than merely expressed: operator bool gates it and operator-> asserts. Trackers therefore hand out their payload table directly and the ten single-payload wrappers are deleted from fbs/. Nothing recorded them -- every root_type is a Record and no wrapper appears under mcap/ -- so no recording or wire format changes. MessageChannelMessagesTracked stays: its data is a list, and a batch needs a table to hold the vector. SchemaTracker gets the real payoff. The wire already carries the payload table, which is now exactly what consumers read, so the final sample is published by adopting its buffer: no unpack and no re-encode, where before it did both. The only remaining unpack is for MCAP, whose writer takes a native, and it is gated on recording being enabled. pack_tracked() gives way to pack_optional() on the primitive, which carries no schema knowledge; tracked.hpp keeps payload() for the Record family and the message-channel batch. In Python the ten wrapper classes are gone and trackers return the payload, so a read loses a .data hop. Since the constructors always encode a payload, serialized_class() grows an absent() factory as the only way to spell the empty handle -- which is what tests need to simulate an inactive device. Signed-off-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>

…active The Python bindings handed out the empty Serialized handle directly, so an inactive device answered field reads with defaults instead of failing: a disconnected pedal read left_pedal == 0.0, indistinguishable from a pedal at rest, and an absent head pose reported is_valid == False as though tracking had merely been lost. Every scalar accessor had the shape `self ? self->field() : 0`, so omitting the truthiness check failed silently -- the same class of mistake the Tracked wrapper was originally introduced to remove. Tracker accessors now map an empty handle onto None at the boundary, so the missed check raises AttributeError instead. Absence was only ever produced by trackers and consumed by callers; nothing in the retargeting engine, examples or plugins constructed it. The absent() factory added for that existed solely so tests could fabricate an inactive device, and it goes: tests pass None, which is what they now receive. The DeviceIO tensor types accept None as a slot value, and the source nodes test `is None` rather than falsiness. Record wrappers take None for their payload for the same reason. MCAP genuinely carries payload-less records -- the message channel writes one per frame as its sentinel -- and that case had become unconstructible from Python. Also corrects a claim in serialized.hpp: the bytes were described as immutable, which the writable NumPy joint views contradict. Immutability is a contract there, not an enforcement, and the header now says so and points at the DLPack constraint that forces it. Signed-off-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>

…to helpers The move to Serialized<T> left the same few expressions spelled out at every call site, and a set of comments describing the API it replaced. Give Serialized two members the call sites were open-coding: - adopt(std::vector<uint8_t>&&), the wire-bytes counterpart to the builder overload. SchemaTracker was the only caller of the raw two-argument constructor the header says to prefer adopt()/narrow() over. - reset(), replacing 15 assignments of a default-constructed handle that had to name the table type to say the payload went away. The haptic command reader was unpacking each drained sample into a native and immediately re-encoding it, on bytes that already were a HapticCommand buffer. Adopt the sample's buffer instead, as SchemaTracker does for its own samples. Where the native is provably non-null, pack() replaces pack_optional(). Collapse three sets of copies: pose_repr() in pose_bindings.h for the Pose format string that had five identical copies, a factory for the seven TensorType subclasses that differed only in the payload class, and one encode on the replay message channel's single exit path. MessageChannelStatusType stays written out -- it is not a device payload, so the comment pasted onto it did not apply. Update the comments the refactor invalidated: nine tracker headers documented tracked->data(), which no longer compiles; AGENTS.md described an absent() factory that does not exist; two binding headers and message_channel.fbs still described the deleted Tracked wrappers. Signed-off-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>

Replay unpacked every record into a -T and immediately re-encoded it, which is the round trip the live path stopped doing when SchemaTracker began adopting its samples. The recorded root is a Record whose data is byte-for-byte the payload table consumers read, so there is nothing to translate. McapTrackerViewers grows read_serialized(), which verifies the recorded bytes and takes ownership of them. The eight single-payload impls narrow() to the payload and share that one buffer, so a frame costs one copy of the message instead of an unpack, a re-encode and the allocations behind both -- for hands and full body that copy was the joint array, twice per frame. It returns the handle directly rather than an optional. Serialized is already nullable, and a record that was read always yields a non-empty handle, so the optional could only repeat what the handle says. The two levels of absence are the handle and its payload: empty means the stream had nothing left, non-empty with a null payload means a record arrived for an inactive tracker. read() stays for the message channel, which composes a new batch table out of the records it drains and so needs owning members; a -T is not nullable, so that one does need its optional. It is now implemented as read_serialized() plus an UnPack, leaving one iterator and one buffer that cannot disagree about ordering. Signed-off-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>

coderabbitai · 2026-07-31T22:42:03Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4e074e01-0cdd-4261-b0ff-51af0fab61c8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The change introduces Serialized<T> as the nullable owning representation for FlatBuffers tracker data. It removes in-memory tracked wrapper tables and updates C++ interfaces, live trackers, replay trackers, MCAP readers, Python bindings, retargeting nodes, plugins, examples, documentation, and tests. Python schema objects now use encoded read-only views. Missing tracker data uses empty handles in C++ and None in Python.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

NVIDIA/IsaacTeleop#742: Introduces full-body tracker plumbing affected by this serialized-data migration.
NVIDIA/IsaacTeleop#853: Modifies overlapping tracker APIs and passthrough implementations.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: tracker APIs now publish serialized FlatBuffers.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch aaristarkhov/use-serialized-types

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/core/deviceio_trackers/cpp/inc/deviceio_trackers/frame_metadata_tracker_oak.hpp (1)
24-31: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Stale Tracked-era doc prose after the Serialized<T> migration. Both class docs still describe the removed TrackedT .data-field shape even though this PR changed the corresponding accessor's return type to Serialized<T> in the same file. Serialized<T> has no .data member, and is_valid is an accessor method, not a field.

src/core/deviceio_trackers/cpp/inc/deviceio_trackers/frame_metadata_tracker_oak.hpp#L24-L31: update the @code example from if (color.data) ... color.data->stream ... color.data->sequence_number to if (color) ... color->stream ... color->sequence_number.

src/core/deviceio_trackers/cpp/inc/deviceio_trackers/se3_tracker.hpp#L25-L31: update the prose from "data stays non-null... data is null only..." and "data->is_valid == false" to describe the handle itself as empty/non-empty and call out pose()->is_valid(), matching the corrected wording already used at lines 72-76 in the same file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/core/deviceio_trackers/cpp/inc/deviceio_trackers/frame_metadata_tracker_oak.hpp`
around lines 24 - 31, Update the documentation examples for the Serialized<T>
accessor migration: in
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/frame_metadata_tracker_oak.hpp
lines 24-31, use the handle’s truthiness and arrow access for stream and
sequence fields; in
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/se3_tracker.hpp lines
25-31, describe the handle as empty or non-empty and use pose()->is_valid()
instead of the removed data field and field-style validity check.
src/core/live_trackers/cpp/live_controller_tracker_impl.cpp (1)
368-457: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Published controller state stays stale after a locate failure.

The sync-failure branch (lines 361-365) correctly resets both the native scratch and the published left_tracked_/right_tracked_ before throwing. The update_controller lambda does not do the same: on xrLocateSpace(grip) failure (line 385) or xrLocateSpace(aim) failure (line 406), it resets only the native scratch (tracked.reset()), then throws. left_tracked_/right_tracked_ are published only at lines 448-449, after both update_controller calls return normally — code the throw skips entirely.

After a locate failure for one controller, get_left_controller()/get_right_controller() keep returning last frame's snapshot instead of the reset state. src/plugins/manus/core/manus_hand_tracking_plugin.cpp reads these getters directly for wrist-pose injection, so hand tracking keeps using a stale controller pose after the failure. This is the exact scenario the AGENTS.md "Publishing tracker output" section (added in this PR) warns against: "Encode on every exit path of update(), including early returns and the throwing ones... otherwise a consumer keeps reading last frame's snapshot after the device drops out."

Publish the tracked state before throwing, not just after both controllers finish successfully.
🐛 Proposed fix to publish reset state before throwing
     auto update_controller = [&](XrPath hand_path, const XrSpacePtr& grip_space, const XrSpacePtr& aim_space,
-                                 std::shared_ptr<ControllerSnapshotT>& tracked)
+                                 std::shared_ptr<ControllerSnapshotT>& native,
+                                 Serialized<ControllerSnapshot>& tracked)
     {
+        auto sync_tracked = [&] { tracked = pack_optional<ControllerSnapshot>(native); };
+
         if (!get_pose_action_active(session_, core_funcs_, grip_pose_action_, hand_path))
         {
-            tracked.reset();
+            native.reset();
+            sync_tracked();
             return;
         }
         ...
         result = core_funcs_.xrLocateSpace(grip_space.get(), base_space_, xr_time, &grip_location);
         if (XR_FAILED(result))
         {
-            tracked.reset();
+            native.reset();
+            sync_tracked();
             throw std::runtime_error("[ControllerTracker] xrLocateSpace(grip) failed: " + std::to_string(result));
         }
         ...
         result = core_funcs_.xrLocateSpace(aim_space.get(), base_space_, xr_time, &aim_location);
         if (XR_FAILED(result))
         {
-            tracked.reset();
+            native.reset();
+            sync_tracked();
             throw std::runtime_error("[ControllerTracker] xrLocateSpace(aim) failed: " + std::to_string(result));
         }
         ...
-        if (!tracked)
+        if (!native)
         {
-            tracked = std::make_shared<ControllerSnapshotT>();
+            native = std::make_shared<ControllerSnapshotT>();
         }
-        tracked->grip_pose = std::make_shared<ControllerPose>(grip_pose);
-        tracked->aim_pose = std::make_shared<ControllerPose>(aim_pose);
-        tracked->inputs = std::make_shared<ControllerInputState>(inputs);
+        native->grip_pose = std::make_shared<ControllerPose>(grip_pose);
+        native->aim_pose = std::make_shared<ControllerPose>(aim_pose);
+        native->inputs = std::make_shared<ControllerInputState>(inputs);
+        sync_tracked();
     };

-    update_controller(left_hand_path_, left_grip_space_, left_aim_space_, left_native_);
-    update_controller(right_hand_path_, right_grip_space_, right_aim_space_, right_native_);
-
-    left_tracked_ = pack_optional<ControllerSnapshot>(left_native_);
-    right_tracked_ = pack_optional<ControllerSnapshot>(right_native_);
+    update_controller(left_hand_path_, left_grip_space_, left_aim_space_, left_native_, left_tracked_);
+    update_controller(right_hand_path_, right_grip_space_, right_aim_space_, right_native_, right_tracked_);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/live_trackers/cpp/live_controller_tracker_impl.cpp` around lines 368
- 457, Update the xrLocateSpace failure branches inside update_controller so
they publish the reset controller state to the corresponding left_tracked_ or
right_tracked_ output before throwing. Ensure both grip and aim failures encode
the cleared state, while preserving the existing successful publication path and
exception behavior.
src/core/live_trackers/cpp/inc/live_trackers/schema_tracker.hpp (1)
73-86: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reset out when read_all_samples throws.

ensure_collection() and read_next_sample() can throw. Since update() writes out only after read_all_samples(), an exception leaves the previous snapshot published. Catch the exception, reset out, then rethrow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/live_trackers/cpp/inc/live_trackers/schema_tracker.hpp` around lines
73 - 86, Update LiveTrackers schema_tracker’s update method around
read_all_samples so exceptions from ensure_collection or read_next_sample reset
out before being rethrown. Preserve the existing normal return and empty-sample
handling, while ensuring no prior snapshot remains published after a failed
read.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/source/device/add_device.rst`:
- Around line 131-134: Update the SchemaTracker documentation’s update()
contract to describe calling m_schema_reader.update(m_tracked), which refreshes
the published Serialized<T> handle. Remove the obsolete
read_all_samples(pending_records) and manual latest-buffer publication guidance,
while retaining the behavior for an absent collection if applicable.

In `@src/core/replay_trackers/cpp/replay_controller_tracker_impl.cpp`:
- Around line 43-66: Guard the missing-data logs in
ReplayControllerTrackerImpl::update so each left/right controller warning is
emitted only once per replay session, using the existing warned_no_data_ pattern
from the other replay trackers while preserving tracked-state resets. Apply the
same warn-once guard to the glove-data log in
ReplayOgloTactileTrackerImpl::update; update
src/core/replay_trackers/cpp/replay_controller_tracker_impl.cpp lines 43-66 and
src/core/replay_trackers/cpp/replay_oglo_tactile_tracker_impl.cpp lines 38-50.

In `@src/core/schema/cpp/inc/schema/serialized.hpp`:
- Around line 84-95: Add FlatBuffers verification with
flatbuffers::Verifier::VerifyBuffer<T>() in SchemaTracker and
LiveHapticCommandReaderTrackerImpl before any GetRoot call or Serialized::adopt
for live tensor samples. Preserve the existing fixed-size checks, but reject
samples when buffer verification fails; do not rely on Serialized::adopt alone
to validate offsets or vtables.

In `@src/core/schema/python/schema_serialized.h`:
- Around line 59-67: The vector packing paths must not pass null native tables
produced by to_native. In src/core/schema/python/schema_serialized.h:59-67,
document the nullptr contract or provide a helper for rejecting empty handles in
vector contexts; in src/core/schema/python/joint_state_bindings.h:74-79, reject
or skip empty handles before native.joints.push_back(...); and in
src/core/schema/python/message_channel_bindings.h:60-66, reject or skip them
before native.data.push_back(...).

In
`@src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/hands_source.py`:
- Around line 128-130: Update _update_hand_data to handle hand_data.joints being
None before dereferencing it, matching the guard used in full_body_source.py.
Fall back to the existing zero-valued joint representation for missing joints
while preserving the current processing path for populated HandPose.joints.

---

Outside diff comments:
In
`@src/core/deviceio_trackers/cpp/inc/deviceio_trackers/frame_metadata_tracker_oak.hpp`:
- Around line 24-31: Update the documentation examples for the Serialized<T>
accessor migration: in
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/frame_metadata_tracker_oak.hpp
lines 24-31, use the handle’s truthiness and arrow access for stream and
sequence fields; in
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/se3_tracker.hpp lines
25-31, describe the handle as empty or non-empty and use pose()->is_valid()
instead of the removed data field and field-style validity check.

In `@src/core/live_trackers/cpp/inc/live_trackers/schema_tracker.hpp`:
- Around line 73-86: Update LiveTrackers schema_tracker’s update method around
read_all_samples so exceptions from ensure_collection or read_next_sample reset
out before being rethrown. Preserve the existing normal return and empty-sample
handling, while ensuring no prior snapshot remains published after a failed
read.

In `@src/core/live_trackers/cpp/live_controller_tracker_impl.cpp`:
- Around line 368-457: Update the xrLocateSpace failure branches inside
update_controller so they publish the reset controller state to the
corresponding left_tracked_ or right_tracked_ output before throwing. Ensure
both grip and aim failures encode the cleared state, while preserving the
existing successful publication path and exception behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a67545da-3b71-4613-b0c0-b91b7d0be0aa

📥 Commits

Reviewing files that changed from the base of the PR and between bb0e575 and b20b773.

📒 Files selected for processing (161)

docs/source/device/add_device.rst
examples/lerobot/record.py
examples/mcap_record_replay/cpp/record_full_body.cpp
examples/oglo_tactile/oglo_teleop_record.py
examples/oxr/cpp/oxr_session_sharing.cpp
examples/oxr/cpp/oxr_simple_api_demo.cpp
examples/oxr/python/modular_example.py
examples/oxr/python/test_controller_tracker.py
examples/oxr/python/test_extensions.py
examples/oxr/python/test_full_body_tracker.py
examples/oxr/python/test_modular.py
examples/oxr/python/test_oak_camera.py
examples/oxr/python/test_session_sharing.py
examples/oxr/python/test_synthetic_hands.py
examples/schemaio/frame_metadata_printer.cpp
examples/schemaio/full_body_printer.cpp
examples/schemaio/pedal_printer.cpp
examples/schemaio/se3_printer.cpp
examples/teleop_session_manager/python/message_channel_example.py
src/core/deviceio_base/AGENTS.md
src/core/deviceio_base/cpp/inc/deviceio_base/controller_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/frame_metadata_tracker_oak_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/full_body_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/generic_3axis_pedal_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/hand_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/haptic_command_reader_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/head_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/joint_state_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/message_channel_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/oglo_tactile_tracker_base.hpp
src/core/deviceio_base/cpp/inc/deviceio_base/se3_tracker_base.hpp
src/core/deviceio_trackers/AGENTS.md
src/core/deviceio_trackers/cpp/controller_tracker.cpp
src/core/deviceio_trackers/cpp/frame_metadata_tracker_oak.cpp
src/core/deviceio_trackers/cpp/full_body_tracker.cpp
src/core/deviceio_trackers/cpp/generic_3axis_pedal_tracker.cpp
src/core/deviceio_trackers/cpp/hand_tracker.cpp
src/core/deviceio_trackers/cpp/haptic_command_reader_tracker.cpp
src/core/deviceio_trackers/cpp/head_tracker.cpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/controller_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/frame_metadata_tracker_oak.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/full_body_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/generic_3axis_pedal_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/hand_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/haptic_command_reader_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/head_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/joint_state_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/message_channel_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/oglo_tactile_tracker.hpp
src/core/deviceio_trackers/cpp/inc/deviceio_trackers/se3_tracker.hpp
src/core/deviceio_trackers/cpp/joint_state_tracker.cpp
src/core/deviceio_trackers/cpp/message_channel_tracker.cpp
src/core/deviceio_trackers/cpp/oglo_tactile_tracker.cpp
src/core/deviceio_trackers/cpp/se3_tracker.cpp
src/core/deviceio_trackers/python/tracker_bindings.cpp
src/core/live_trackers/AGENTS.md
src/core/live_trackers/cpp/inc/live_trackers/schema_tracker.hpp
src/core/live_trackers/cpp/live_controller_tracker_impl.cpp
src/core/live_trackers/cpp/live_controller_tracker_impl.hpp
src/core/live_trackers/cpp/live_frame_metadata_tracker_oak_impl.cpp
src/core/live_trackers/cpp/live_frame_metadata_tracker_oak_impl.hpp
src/core/live_trackers/cpp/live_full_body_tracker_noitom_impl.cpp
src/core/live_trackers/cpp/live_full_body_tracker_noitom_impl.hpp
src/core/live_trackers/cpp/live_full_body_tracker_pico_impl.cpp
src/core/live_trackers/cpp/live_full_body_tracker_pico_impl.hpp
src/core/live_trackers/cpp/live_generic_3axis_pedal_tracker_impl.cpp
src/core/live_trackers/cpp/live_generic_3axis_pedal_tracker_impl.hpp
src/core/live_trackers/cpp/live_hand_tracker_impl.cpp
src/core/live_trackers/cpp/live_hand_tracker_impl.hpp
src/core/live_trackers/cpp/live_haptic_command_reader_tracker_impl.cpp
src/core/live_trackers/cpp/live_haptic_command_reader_tracker_impl.hpp
src/core/live_trackers/cpp/live_head_tracker_impl.cpp
src/core/live_trackers/cpp/live_head_tracker_impl.hpp
src/core/live_trackers/cpp/live_joint_state_tracker_impl.cpp
src/core/live_trackers/cpp/live_joint_state_tracker_impl.hpp
src/core/live_trackers/cpp/live_message_channel_tracker_impl.cpp
src/core/live_trackers/cpp/live_message_channel_tracker_impl.hpp
src/core/live_trackers/cpp/live_oglo_tactile_tracker_impl.cpp
src/core/live_trackers/cpp/live_oglo_tactile_tracker_impl.hpp
src/core/live_trackers/cpp/live_se3_tracker_impl.cpp
src/core/live_trackers/cpp/live_se3_tracker_impl.hpp
src/core/mcap/cpp/inc/mcap/tracker_channels.hpp
src/core/oxr_utils/cpp/inc/oxr_utils/pose_conversions.hpp
src/core/replay_deviceio_session_tests/cpp/test_replay_session.cpp
src/core/replay_trackers/cpp/replay_controller_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_controller_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_full_body_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_full_body_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_generic_3axis_pedal_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_generic_3axis_pedal_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_hand_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_hand_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_haptic_command_reader_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_haptic_command_reader_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_head_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_head_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_joint_state_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_joint_state_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_message_channel_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_message_channel_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_oglo_tactile_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_oglo_tactile_tracker_impl.hpp
src/core/replay_trackers/cpp/replay_se3_tracker_impl.cpp
src/core/replay_trackers/cpp/replay_se3_tracker_impl.hpp
src/core/retargeting_engine_tests/python/test_joint_state_source.py
src/core/retargeting_engine_tests/python/test_message_channel_nodes.py
src/core/retargeting_engine_tests/python/test_sources.py
src/core/schema/cpp/inc/schema/full_body_compat.hpp
src/core/schema/cpp/inc/schema/serialized.hpp
src/core/schema/cpp/inc/schema/tracked.hpp
src/core/schema/fbs/controller.fbs
src/core/schema/fbs/full_body.fbs
src/core/schema/fbs/hand.fbs
src/core/schema/fbs/haptic_command.fbs
src/core/schema/fbs/head.fbs
src/core/schema/fbs/joint_state.fbs
src/core/schema/fbs/message_channel.fbs
src/core/schema/fbs/oak.fbs
src/core/schema/fbs/oglo_tactile.fbs
src/core/schema/fbs/pedals.fbs
src/core/schema/fbs/se3_tracker.fbs
src/core/schema/python/CMakeLists.txt
src/core/schema/python/controller_bindings.h
src/core/schema/python/full_body_bindings.h
src/core/schema/python/hand_bindings.h
src/core/schema/python/haptic_command_bindings.h
src/core/schema/python/head_bindings.h
src/core/schema/python/joint_state_bindings.h
src/core/schema/python/message_channel_bindings.h
src/core/schema/python/oak_bindings.h
src/core/schema/python/oglo_tactile_bindings.h
src/core/schema/python/pedals_bindings.h
src/core/schema/python/pose_bindings.h
src/core/schema/python/schema_serialized.h
src/core/schema/python/se3_tracker_bindings.h
src/core/schema_tests/cpp/test_full_body.cpp
src/core/schema_tests/cpp/test_pedals.cpp
src/core/schema_tests/cpp/test_se3_tracker.cpp
src/core/schema_tests/python/test_camera.py
src/core/schema_tests/python/test_full_body.py
src/core/schema_tests/python/test_hand.py
src/core/schema_tests/python/test_head.py
src/core/schema_tests/python/test_oglo_tactile.py
src/core/schema_tests/python/test_pedals.py
src/core/schema_tests/python/test_se3_tracker.py
src/plugins/controller_se3_tracker/controller_se3_tracker_plugin.cpp
src/plugins/controller_synthetic_hands/synthetic_hands_plugin.cpp
src/plugins/haptikos/haptikos_hands_plugin.cpp
src/plugins/manus/core/manus_hand_tracking_plugin.cpp
src/plugins/plugin_utils/wrist_pose_source.cpp
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/controllers_source.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/deviceio_tensor_types.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/full_body_source.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/hands_source.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/head_source.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/joint_state_source.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/message_channel_config.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/message_channel_sink.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/message_channel_source.py
src/python/isaacteleop/retargeting_engine/deviceio_source_nodes/pedals_source.py
src/python/isaacteleop/schema/__init__.py

💤 Files with no reviewable changes (4)

src/core/schema/fbs/oglo_tactile.fbs
src/core/schema/fbs/haptic_command.fbs
src/core/schema_tests/cpp/test_se3_tracker.cpp
src/core/schema/cpp/inc/schema/full_body_compat.hpp

…ed to be The Serialized refactor left the tracker docs describing the wrapper tables it deleted. trackers.rst still taught the three-tier schema convention and told readers to check `data` for null; oglo.rst listed a schema type that no longer exists; four facade doc comments still spelled absence as a null `data` field, one of them also promising an unpack that the adopt path no longer performs. Replaces all of it with the handle: absence is the empty handle itself, and a new subsection covers what a caller gets back in each language, the per-frame buffer guarantee, and the message-channel batch that still needs a table. Signed-off-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>

…ct in add_device Two findings from PR review. A vector field is the one place a null element is fatal: the generated Pack null-checks an optional table field but dereferences every vector element unconditionally. The constructors for `JointStateOutput.joints` and `MessageChannelMessagesTracked.data` fed that vector from a conversion that returns null for an empty handle. Not reachable today — every payload constructor encodes, so Python cannot produce an empty view — but the correct-looking call was the wrong one. `to_native_vector` takes the whole vector, so there is no per-element call to get wrong. It stays in the binding layer, where its only callers are, and the optional-table-field conversion is inlined into `bind_record` rather than kept as a second function that differs only in what an empty handle means. That leaves it untestable — an empty view is not constructible from Python, and this header is private to schema_py — so the new schema test covers the `Serialized` handle itself (ownership, narrowing, absence) and not the conversion. add_device.rst still walked new device authors through `read_all_samples()` plus a hand-written `serialize_all()`, which no longer exists anywhere in the tree. The impl now delegates a tick to a single `SchemaTracker::update()` that reads, records, and publishes; describe that, and what a caller may assume about the handle it gets back. Signed-off-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>

aristarkhovNV added 5 commits July 31, 2026 14:37

github-code-quality Bot found potential problems Jul 31, 2026

View reviewed changes

coderabbitai Bot mentioned this pull request Jul 31, 2026

feat(deviceio): generate passthrough trackers from a TOML manifest #853

Open

6 tasks

coderabbitai Bot reviewed Jul 31, 2026

View reviewed changes

aristarkhovNV force-pushed the aaristarkhov/use-serialized-types branch from b3e8f3b to 2ebb2bb Compare August 1, 2026 00:16

aristarkhovNV force-pushed the aaristarkhov/use-serialized-types branch from 2ebb2bb to 72256bc Compare August 1, 2026 00:29

aristarkhovNV requested review from jiwenc-nv, nv-jakob and nvddr August 1, 2026 00:34

aristarkhovNV deployed to dev August 1, 2026 00:41 — with GitHub Actions Active

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Publish serialized FlatBuffers from the tracker API - #865

Publish serialized FlatBuffers from the tracker API#865
aristarkhovNV wants to merge 7 commits into
mainfrom
aaristarkhov/use-serialized-types

aristarkhovNV commented Jul 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading

Review skipped

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

aristarkhovNV commented Jul 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Why

Key decisions

1. Trackers publish their payload table directly — the ten Tracked wrappers are deleted (breaking)

2. Python gets None for an inactive device (breaking)

3. Python builds payloads through constructors, never setters (breaking)

4. Python class names drop the trailing T (breaking, with aliases)

5. No recording or wire format change

6. Structs and the zero-copy NumPy views are untouched

7. The published type is a handle over encoded bytes

8. The handle is nullable, for a format reason rather than a domain one

9. Each update() publishes a new buffer instead of refilling the previous one

10. -T survives as internal assembly scratch, named native_

11. Live and replay both adopt the buffer they were handed, rather than round-tripping it

12. Immutability is a contract, not an enforcement

Measurements

Type of change

Testing

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

aristarkhovNV commented Jul 31, 2026 •

edited by coderabbitai Bot

Loading

1. Trackers publish their payload table directly — the ten `Tracked` wrappers are deleted (breaking)

2. Python gets `None` for an inactive device (breaking)

4. Python class names drop the trailing `T` (breaking, with aliases)

9. Each `update()` publishes a new buffer instead of refilling the previous one

10. `-T` survives as internal assembly scratch, named `native_`

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading