Skip to content

d500 motion: align D555/D585 gyro path with HKR physical-unit IMU encoding#15456

Draft
remibettan wants to merge 5 commits into
realsenseai:developmentfrom
remibettan:d555-hkr-physical-imu-alignment
Draft

d500 motion: align D555/D585 gyro path with HKR physical-unit IMU encoding#15456
remibettan wants to merge 5 commits into
realsenseai:developmentfrom
remibettan:d555-hkr-physical-imu-alignment

Conversation

@remibettan

Copy link
Copy Markdown
Contributor

Overview: Extend D555/D585 gyro sensitivity registration to all D585 variants and align the SDK gyro path with the FW's HKR physical-unit IMU encoding.

Tracked on [RSDEV-60196]

Why

D555 and D585 firmwares that ship the HKR physical-IMU work (IO 34cf87d6 + device-manager 1be508e0) emit gyro samples as 32-bit fixed physical units instead of 16-bit raw registers. The SDK today interprets those samples as legacy raw 16-bit output, and the D585 constructors do not register the sensitivity option at all.

Builds on top of PR #15436 (d555-usb-gyro-sensitivity).

Summary

  • Introduce gyro_sensitivity_encoding (d400_hid_token / hkr_range_index) and route the D400 conversion table + defaults through encode_gyro_sensitivity() / default_gyro_sensitivity().
  • Add PID-based HKR detection in d500_motion: supports_hkr_physical_imu(), is_imu_high_accuracy() override, and switch get_gyro_default_scale() to 0.0001 for HKR-eligible devices.
  • Set the HID gyro scale factor to 10000. in create_hid_device for HKR-eligible devices and register the sensitivity option with the HKR encoding + default range index 4.f.
  • Register RS2_OPTION_GYRO_SENSITIVITY on the four D585 constructors (rs5x5_device, rs5x5_dedicated_color_device, rs585_legacy_device, rs585s_device); D555 registration already lands via PR d500: register Gyro Sensitivity option on USB for D555 #15436, macOS guard preserved.
  • Add a Catch2 unit test for the encoding table.

Behavior change (HKR-eligible D555/D585 only)

Aspect Before After
is_imu_high_accuracy() false (base default) true
Frame converter 16-bit 32-bit (converter_32_bit)
get_gyro_default_scale() 0.003814697265625 0.0001
HID set_gyro_scale_factor not called 10000.
Sensitivity encoding n/a (D555 was d400_hid_token) hkr_range_index
Option default n/a (D555 was 1.f) 4.f
D585 variants option not registered option registered on all 4
D400 devices unchanged unchanged (encoding stays d400_hid_token)

REVIEW BLOCKER

hkr_physical_imu_min_fw is a sentinel 65535.65535.65535.65535. It must be replaced with the first released FW that contains IO 34cf87d6 + device-manager 1be508e0 before this PR can merge — otherwise HKR mode activates against firmwares that still emit 16-bit samples and gyro/accel data will be misinterpreted.

Test plan

  • Unit test test-gyro-sensitivity-encoding passes (Windows + Linux).
  • D400 regression: existing gyro sensitivity behavior unchanged on D435i / D455 (encoding stays d400_hid_token).
  • D555 live once real FW is available: option registers, gyro frames report physical units matching FW spec, pytest-gyro-sensitivity-levels.py still passes.
  • D585 live once real FW is available: option registers on all four variants, motion frames not garbled (32-bit converter agrees with FW output).
  • macOS build: no compile errors (motion paths remain #if !defined(__APPLE__) guarded).

🤖 Generated by AI

@sysrsbuild-gh-agentic sysrsbuild-gh-agentic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 rs-agentic bot automated review — bugs and notes below.


// HID metadata attributes
hid_ep->get_raw_sensor()->register_metadata(RS2_FRAME_METADATA_FRAME_TIMESTAMP, make_hid_header_parser(&hid_header::timestamp));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 [rs-agentic bot] BUGget_raw_motion_sensor() uses dynamic_pointer_cast and can return nullptr if the cast fails. Dereferencing without a null-check here will crash. Add: if (auto raw = get_raw_motion_sensor()) raw->set_gyro_scale_factor(10000.);

#endif
}

void d500_motion::register_gyro_sensitivity()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 [rs-agentic bot] BUGget_raw_motion_sensor() is called twice without a null-check. If dynamic_pointer_cast fails both calls return nullptr and the first dereference crashes. Cache the result and guard: auto raw = get_raw_motion_sensor(); if (!raw) return;

return value;

switch( static_cast< int >( value ) )
{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 [rs-agentic bot] BUGstatic_cast<int>(value) truncates without checking for fractional input (e.g. 0.9f silently falls to case 0). Consider std::lround + a range assertion, or document that only integer-valued floats are accepted.

namespace
{
// REVIEW BLOCKER: replace this sentinel with the first released FW that
// contains IO 34cf87d6 and device-manager 1be508e0.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 [rs-agentic bot] REVIEW BLOCKERhkr_physical_imu_min_fw("65535.65535.65535.65535") is a sentinel that makes supports_hkr_physical_imu() always return false today. Per the PR description this must be replaced with the real first HKR FW version before merge, otherwise the HKR path never activates (or if the sentinel is ever lowered without proper testing, will activate against legacy FW emitting 16-bit samples — misinterpreting all gyro data).


pipe = rs.pipeline(ctx)
pipe.set_device(dev)
cfg = rs.config()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 [rs-agentic bot] BUGset_option is called on motion_sensor obtained before pipe.start(). After pipe.start() the sensor is re-fetched from profile.get_device(). If the option is not persisted across pipeline restarts the readback will reflect the default, not the value set, causing intermittent failures. Set the option after pipe.start() or verify the option survives a pipeline restart.

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.

2 participants