Tracker service v0.3.2: robot vision integration#968
Conversation
There was a problem hiding this comment.
Pull request overview
This PR integrates RobotVision's Kalman filter tracking library into the tracker service, replacing the previous stub tracking implementation. The change upgrades track IDs from UUID strings to integer IDs produced by RobotVision's tracking system.
Changes:
- Integrated RobotVision TrackTracker with Kalman filter for multi-object tracking
- Changed track ID type from string to int32_t to match RobotVision's output
- Added camera calibration support for pixel-to-world coordinate transformation
- Updated tests to reflect real tracking behavior (tracks require multiple detections for reliability)
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tracker/src/tracking_worker.cpp | Replaced stub tracking with RobotVision TrackTracker integration, added camera calibration handling |
| tracker/inc/tracking_worker.hpp | Added RobotVision headers and member variables for tracker and camera parameters |
| tracker/inc/tracking_types.hpp | Changed Track::id from string to int32_t |
| tracker/src/track_publisher.cpp | Updated JSON serialization to use SetInt() instead of SetString() for track ID |
| tracker/src/time_chunk_scheduler.cpp | Refactored worker creation to build camera map and pass tracking config |
| tracker/schema/scene-data.schema.json | Updated schema to enforce integer track IDs with minimum 0 |
| tracker/test/unit/tracking_worker_test.cpp | Added helper functions, new tests for edge cases, updated expectations for RobotVision behavior |
| tracker/test/unit/track_publisher_test.cpp | Updated test data to use integer track IDs |
| tracker/test/unit/time_chunk_scheduler_test.cpp | Added camera intrinsics/distortion to test fixtures |
| tracker/test/unit/message_handler_test.cpp | Added test for lagged message handling |
| tracker/test/service/test_mqtt.py | Added detection sequence helper and new tracking reliability test |
Comments suppressed due to low confidence (5)
tracker/test/service/test_mqtt.py:1
- Test code contains timestamps in January 2026, which is in the future relative to the current date (February 3, 2026). Consider using timestamps that are clearly test data or relative to the current time to avoid confusion.
#!/usr/bin/env python3
tracker/test/service/test_mqtt.py:1
- Test code contains timestamps in January 2026, which is in the future relative to the current date (February 3, 2026). Consider using timestamps that are clearly test data or relative to the current time to avoid confusion.
#!/usr/bin/env python3
tracker/test/service/test_mqtt.py:1
- Test code contains timestamps in January 2026, which is in the future relative to the current date (February 3, 2026). Consider using timestamps that are clearly test data or relative to the current time to avoid confusion.
#!/usr/bin/env python3
tracker/test/service/test_mqtt.py:1
- Test code contains timestamps in January 2026, which is in the future relative to the current date (February 3, 2026). Consider using timestamps that are clearly test data or relative to the current time to avoid confusion.
#!/usr/bin/env python3
tracker/test/service/test_mqtt.py:1
- Test code contains timestamps in January 2026, which is in the future relative to the current date (February 3, 2026). Consider using timestamps that are clearly test data or relative to the current time to avoid confusion.
#!/usr/bin/env python3
d186ad4 to
6fd555a
Compare
fc4b99c to
70db2a4
Compare
7aaba31 to
5fee258
Compare
d527ff8 to
d2031d9
Compare
Remove unused variables that were computed but never read after the loop. The batch_timestamp is still used for tracker_.track() calls. Addresses PR #968 review comment about dead code.
Change format from '{:%Y-%m-%dT%H:%M:%S}Z' to '{:%Y-%m-%dT%H:%M:%S}.000Z'
to match the ISO 8601 '.fffZ' pattern used by detection messages.
Update test to verify '.' is present in the fallback timestamp.
Addresses PR #968 review comment about missing fractional seconds.
These headers are used directly (std::optional<int> parse_int, chrono types in parse_timestamp/process_chunk) but were only included transitively. Explicit includes follow the include-what-you-use principle. Addresses PR #968 review comment about missing includes.
Use CoordinateTransformer (full intrinsics + extrinsics pipeline) instead of rv::computePixelsToMeterPlane (undistort-only). This changes published Track.translation from camera-relative normalized coords to actual world coordinates via the bboxFootToWorld() method (undistort -> pose -> ray-plane intersection with ground plane z=0). Changes: - TrackingWorker stores CoordinateTransformer per camera instead of raw intrinsics/distortion matrices - run_tracking() uses bboxFootToWorld() for pixel-to-world transform - Remove get_camera_params() method (no longer needed) - Update test cameras to include extrinsics Addresses PR #968 review comment about computePixelsToMeterPlane lacking extrinsics.
Replace unbounded while loops with deadline-bounded loops that fail with ASSERT_LT after 5 seconds. Prevents CI from hanging forever if the worker thread deadlocks. Addresses PR #968 review comment about unbounded polling in QueueDepth_ReturnsCorrectSize.
Replace unbounded while loop with deadline-bounded loop that fails with ASSERT_LT after 5 seconds. Prevents CI from hanging forever if the worker thread deadlocks. Addresses PR #968 review comment about unbounded polling in QueueFull_IncrementsDroppedCount.
The cv.wait_for() return value was discarded, so the test would silently pass even if the callback was never invoked. Wrap in ASSERT_TRUE to fail explicitly on timeout. Addresses PR #968 review comment about discarded wait_for result.
The 1e-9 tolerance leaves very little margin for differences in sin/cos implementations across compilers, libm versions, and floating-point optimization flags. 1e-8 is still strict enough to catch real bugs (wrong rotation order differs by ~0.7) while being robust across build configurations. Addresses PR #968 review comment about strict rotation tolerance.
Use std::format("{:%Y-%m-%dT%H:%M:%S}.000Z", ...) with chrono time_point
instead of POSIX-only gmtime_r + std::put_time. This is consistent with
the approach used in tracking_worker.cpp and uses only standard C++20.
Remove unused <iomanip> and <sstream> includes.
Addresses PR #968 review comment about gmtime_r portability.
The run/run-debug targets export TRACKER_MQTT_HOST, TRACKER_MQTT_PORT, TRACKER_MQTT_INSECURE, and TRACKER_MQTT_TLS_CA_CERT. If a developer runs 'make run' then 'make test-service' in the same shell, those env vars would leak into the pytest process. Add explicit unset to isolate the test environment. Addresses PR #968 review comment about removed env var isolation.
26444de to
8bbf97a
Compare
Move valid[i] = 1 outside the if/else block since both branches set it to the same value. Addresses PR #968 review feedback.
Remove redundant `&& i < 3` in fractional-seconds scaling loop. Copy-paste artifact. Addresses PR #968 review feedback.
…eTimestamp Use std::string_view and index-based access instead of const char* pointer arithmetic for fractional-seconds parsing. Addresses PR #968 review feedback.
| #include <gtest/gtest.h> | ||
|
|
||
| #include "id_map.hpp" | ||
|
|
There was a problem hiding this comment.
SequentialGenerator uses std::array, but this test file doesn't include <array>, which can cause a compile failure depending on transitive includes. Add #include <array> (or avoid std::array here) to make the test self-contained.
| const std::vector<YawToQuaternionTestCase> kYawToQuaternionTests = { | ||
| {0.0, {0.0, 0.0, 0.0, 1.0}}, | ||
| {M_PI / 2, {0.0, 0.0, 0.7071067811865475, 0.7071067811865476}}, | ||
| {M_PI, {0.0, 0.0, 1.0, 6.123233995736766e-17}}, | ||
| {-M_PI / 2, {0.0, 0.0, -0.7071067811865475, 0.7071067811865476}}, |
There was a problem hiding this comment.
This test uses M_PI, which is a non-standard extension and may be undefined on some toolchains (notably MSVC unless _USE_MATH_DEFINES is set). Prefer a portable constant like std::numbers::pi (C++20) to avoid build portability issues.
| TrackingScope scope{"scene-1", "person"}; | ||
| TrackingWorker worker(scope, "Test Scene", 2, callback); | ||
| TrackingWorker worker(scope, "Test Scene", 2, callback, tracking_config_, cameras_); | ||
|
|
||
| // Create chunk with detections | ||
| Chunk chunk; |
There was a problem hiding this comment.
The tests in this file set DetectionBatch.timestamp_iso on batches they enqueue, but don’t populate the new parsed DetectionBatch.timestamp field. Since TrackingWorker now uses batch.timestamp to advance the RobotVision tracker, these tests end up exercising tracking with a default-constructed epoch time (mismatched with timestamp_iso). Consider setting batch.timestamp (e.g., via parseTimestamp(timestamp_iso) or a helper) when building test batches.
| "id": { | ||
| "type": "string", | ||
| "description": "Persistent track ID (UUID, stable across frames while tracked)" | ||
| "format": "uuid", | ||
| "description": "Persistent track ID (UUID v4, stable across frames while tracked)" |
There was a problem hiding this comment.
PR description mentions a breaking change where the track ID changed from string to integer, but the schema (and tests) still define id as a string (now with format: uuid). Please reconcile the PR description with the actual API (or adjust the schema/output if an integer ID is intended).
No changes outside of the tracker folder. Will not be default for another release cycle. Focused on achieving MVP results both functional and performance.
📝 Description
Replaces the stub tracking in
TrackingWorkerwith Intel RobotVision's Kalman filter library, adds batch coordinate transformation, and hardens the deployment configuration.Key Changes
RobotVision Integration (
tracking_worker.cpp):stub_tracking()withrv::tracking::MultipleObjectTracker(Kalman filter + Hungarian matching, CV/CA/CTRV motion models)run_tracking()→transform_detections()→convert_tracks()pipelineNew Modules:
CoordinateTransformer— batch pixel-to-world viacv::undistortPoints+ OpenMP ray-plane intersection (matches Python controller'stransform.py)IdMap— maps RobotVisionint32_ttrack IDs to persistent UUID v4 strings at the output boundaryuuid.hpp— thread-safe UUID v4 generator (thread_local mt19937_64)time_utils.hpp/cpp— extractedparseTimestamp/formatTimestampfromMessageHandlerinto standalone module; now requires trailingZ(UTC)Configuration:
max_unreliable_time_s,non_measurement_time_dynamic_s,non_measurement_time_static_stoTrackingConfig+ JSON schema + env var overridesbroker.scenescape.intel.com,insecure: false, CA cert at/run/secrets/certs/)trackingsection totracker.jsonwith all pipeline parametersDeployment Hardening:
read_only: true,cap_drop: ALL,no-new-privileges:truefor tracker serviceDeveloper Experience:
launch.json: added proxy and TLS environment variables for both native and container debugrun/run-debug: exportTRACKER_MQTT_*and TLS vars; mount CA cert forrun-image/run-image-debugTesting:
coordinate_transformer_test.cpp(position, size, batch, horizon culling),id_map_test.cpp,uuid_test.cpp,time_utils_test.cpp(valid/invalid/round-trip)tracking_worker_test.cpp(RobotVision multi-detection behavior),message_handler_test.cpp(lag detection, empty batch buffering),time_chunk_scheduler_test.cpp(camera intrinsics in fixtures)test_tracking_produces_reliable_tracks— sends 20 detections, validates UUID v4 track outputtransformation_reference.jsonfor cross-validation with scipy/numpyFormatChecker()to enforce UUID formatDependencies:
stduuid/1.2.3(Conan),OpenMP(system)Schema:
scene-data.schema.json: track ID now"format": "uuid"with updated description✨ Type of Change
🧪 Testing Scenarios
✅ Checklist
✨ Type of Change
🧪 Testing Scenarios
✅ Checklist