Skip to content

feat(oak): make oak frame tracker track 1 stream - #874

Closed
shaosu-nvidia wants to merge 1 commit into
mainfrom
ssx/oak_frame_tracker
Closed

feat(oak): make oak frame tracker track 1 stream#874
shaosu-nvidia wants to merge 1 commit into
mainfrom
ssx/oak_frame_tracker

Conversation

@shaosu-nvidia

@shaosu-nvidia shaosu-nvidia commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Description

Change FrameMetadataTrackerOak to only track one stream per tracker so it will match the pattern from #853
Fixes #868

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

Testing

Tested with examples/oxr/python/test_oak_camera.py with schema-pusher mode and confirmed the live trackers are able to receive the messages from tensor API and write to the mcap file.

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)
  • I have signed off all my commits (git commit -s) per the DCO

Summary by CodeRabbit

  • New Features

    • OAK frame metadata tracking now uses one tracker per camera stream.
    • Added support for stream-specific collection IDs and MCAP channel mappings.
    • OAK recordings use the standard frame_metadata channel.
  • Documentation

    • Updated OAK setup examples and tracker guidance to reflect the simplified per-stream configuration.
    • Updated Python and C++ examples for the new metadata access pattern.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

FrameMetadataTrackerOak now tracks one OAK stream per instance. Its constructor accepts a complete collection ID, and get_data(session) replaces indexed stream access. The live implementation stores one schema tracker and uses the fixed frame_metadata channel. Python and C++ examples create tracker lists for separate streams and configure per-stream MCAP mappings. Documentation reflects the new construction and access patterns.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TeleopSession
  participant FrameMetadataTrackerOak
  participant LiveFrameMetadataTrackerOakImpl
  participant OakSchemaTracker
  participant MCAP writer
  TeleopSession->>FrameMetadataTrackerOak: create one tracker per collection_id
  LiveFrameMetadataTrackerOakImpl->>MCAP writer: create frame_metadata channel
  LiveFrameMetadataTrackerOakImpl->>OakSchemaTracker: initialize and update one stream
  TeleopSession->>FrameMetadataTrackerOak: get_data(session)
  FrameMetadataTrackerOak-->>TeleopSession: return tracked metadata
Loading

Possibly related PRs

  • NVIDIA/IsaacTeleop#870: Both changes modify OAK/Orbbec frame-metadata tracker construction and MCAP integration.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implements single-stream trackers and updates call sites, but it does not move the tracker into the TOML manifest and codegen path required by issue #868. Add the TOML manifest and generated tracker integration, then remove the hand-written boilerplate.
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making the OAK frame tracker handle one stream.
Out of Scope Changes check ✅ Passed The code, documentation, bindings, and examples changed directly support the single-stream OAK tracker refactor; no unrelated changes are evident.
✨ 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 ssx/oak_frame_tracker

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

Signed-off-by: Shao Su <shaoxiangs@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/trackers.rst`:
- Around line 223-231: Add the repository-standard reStructuredText SPDX
copyright and license header at the beginning of the documentation file, before
the existing tracker content. Preserve all current documentation text and
formatting after the header.
🪄 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: d2ab9c01-5829-4ee7-92b5-7dab51ac9e17

📥 Commits

Reviewing files that changed from the base of the PR and between 01e1b80 and bd47fbb.

📒 Files selected for processing (13)
  • docs/source/device/oak.rst
  • docs/source/device/trackers.rst
  • examples/oxr/python/test_oak_camera.py
  • examples/schemaio/frame_metadata_printer.cpp
  • src/core/deviceio_base/cpp/inc/deviceio_base/frame_metadata_tracker_oak_base.hpp
  • src/core/deviceio_trackers/cpp/frame_metadata_tracker_oak.cpp
  • src/core/deviceio_trackers/cpp/inc/deviceio_trackers/frame_metadata_tracker_oak.hpp
  • src/core/deviceio_trackers/python/tracker_bindings.cpp
  • src/core/live_trackers/cpp/live_deviceio_factory.cpp
  • 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/mcap/cpp/inc/mcap/recording_traits.hpp
  • src/plugins/oak/README.md

Comment on lines +223 to +231
Single-stream tracker for per-frame metadata from an OAK camera stream.
Create one instance per stream (e.g. ``"oak_camera/Color"``, ``"oak_camera/MonoLeft"``).
Uses the :code-file:`SchemaTracker <src/core/live_trackers/cpp/inc/live_trackers/schema_tracker.hpp>`
utility internally.

- Schema: :code-file:`src/core/schema/fbs/oak.fbs`
- C++ header: ``#include <deviceio/frame_metadata_tracker_oak.hpp>``
- Python import: ``from isaacteleop.deviceio import FrameMetadataTrackerOak``
- Record channels: one per configured stream (e.g. ``Color``, ``MonoLeft``) | MCAP schema: ``core.FrameMetadataOakRecord``
- Record channels: ``frame_metadata`` | MCAP schema: ``core.FrameMetadataOakRecord``

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add the required SPDX header.

docs/source/device/trackers.rst lacks the repository SPDX copyright and license header. Add the standard reStructuredText SPDX block at the start of the file.

Proposed fix
+.. SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+.. SPDX-License-Identifier: Apache-2.0
+
 Device Trackers

As per coding guidelines, “Files covered by REUSE policy must include the repository’s standard SPDX copyright and license headers.”

🤖 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 `@docs/source/device/trackers.rst` around lines 223 - 231, Add the
repository-standard reStructuredText SPDX copyright and license header at the
beginning of the documentation file, before the existing tracker content.
Preserve all current documentation text and formatting after the header.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor FrameMetadataTrackerOak to single-stream to fit codegen template

1 participant