Update safety flash tables structs and tests to flash v0.95#14967
Conversation
There was a problem hiding this comment.
Pull request overview
Updates D500/D585S safety flash table structs, JSON serialization, and live unit tests to align with flash spec v0.95 layout/version changes.
Changes:
- Updated
safety_interface_config/occupancy_grid_paramsJSON + struct layout (new float fields, larger reserved tail) and bumped write version to0x0400. - Updated
application_configlayout to includehw_configuration_setup, adjusted reserved sizing, and bumped write version to0x0102. - Removed
surface_heightfrom Safety Preset environment JSON and repurposed its slot as reserved bytes; added new D500 table IDs; updated corresponding live tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| unit-tests/live/d500/safety/test-preset-get-set.py | Removes surface_height from environment JSON used in preset get/set test. |
| unit-tests/live/d500/safety/test-interface-config-get-set.py | Updates SIC JSON schema to new occupancy_grid_params fields and modifies test mutation logic. |
| unit-tests/live/d500/safety/test-app-config-get-set-hwm-cmd.py | Inserts hw_configuration_setup into generated binary app-config payload; bumps version bytes. |
| src/ds/d500/d500-types/safety-preset.h | Removes surface_height from JSON mapping/validation; keeps 4-byte reserved placeholder in struct. |
| src/ds/d500/d500-types/safety-interface-config.h | Reworks occupancy_grid_params struct/JSON fields and expands SIC reserved tail. |
| src/ds/d500/d500-types/application-config.h | Adds hw_configuration_setup field, adjusts reserved sizing, updates version comment, and JSON mapping/validation. |
| src/ds/d500/d500-safety.cpp | Bumps SIC and AppConfig write-version constants used in table headers. |
| src/ds/d500/d500-private.h | Adds new calibration table IDs for frame sequence and projector laser calibration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
See: Review of PR #14967 vs. v0.95 specI read the xlsx and cross-checked every changed field. Authoritative sources used:
✅ Verified correct against spec
Outer 🔴 Concerns to address before merging1. Safety Preset (0xC0DB) version bump — likely incorrect.
Only 0xC0DA (the outer table) is bumped to
The PR description ("Safety Preset (0xC0DB) - Safety Zones version 0x0300 → 0x0302") appears to conflate the outer 0xC0DA bump with the inner 0xC0DB writer. Recommend reverting that one line back to 2. PR description typo. It says preset went 🟡 Nice-to-have3. Add 4. The class-level docblock for Status of Copilot review threads
RecommendationRequest changes with one ask: have remibettan confirm with the spec author whether the inner Safety Preset (0xC0DB) version should remain |
7d812a4 to
be76cb5
Compare
- Add static_assert(sizeof(safety_interface_config) == 464) to catch layout regressions at compile time - Fix Safety Preset write version from 0x0301 to 0x0302 per flash v0.95 spec - Update application_config docblock to document hw_configuration_setup field Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Added missing hw_configuration_setup field to the test JSON to match the updated application_config struct from flash v0.95. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
be76cb5 to
0d15aa6
Compare
Tracked by: RSDEV-6640
Summary
Updates the D585S Safety Camera C++ struct definitions, JSON API, version constants, and unit tests to match the flash memory layout changes introduced in flash spec v0.95 (from v0.93).
Changes
Safety Interface Config (
0xC0DC) - version0x0301?0x0400occupancy_grid_params: Replaced the three quorum threshold fields (close_range_quorum,mid_range_quorum,long_range_quorum) and renamedgrid_cell_seed?grid_cell_size. Added three new float fields:cell_threshold_factor,polynomial_bias, andsurface_height(moved here from the Safety Preset table).safety_interface_config: Grew reserved tail from 17 ? 324 bytes to match the expanded table size (148 ? 464 bytes).set_safety_interface_config(): version constant updated to0x04 0x00.Safety Preset (
0xC0DB) - Safety Zones version0x0300?0x0302safety_environment:surface_height(float) transferred to Safety Interface Config. The 4-byte slot is now a reserved array (m_reserved_surface_height[4]). Removed from JSON API and validation.Application Config (
0xC0DE) - version0x0100?0x0102application_config: Addedhw_configuration_setup(uint8_t, Feat Frame pointer ownership? #16) for Pixter/RVP HW platform selection (0 = nominal, 1 = Pixter MIPI injection, 2 = RVP dev board).reserved3shrunk from 265 ? 264 bytes.set_application_config(): version constant updated to0x01 0x02.New Table IDs (
d500-private.h)frame_sequence_id = 0xc0e0(ctFrameSequence, new in v0.95 for SC1.2)projector_laser_id = 0xc0e8(ctProjectorLaser, Laser Projector calibration GOLD)Tests
test-interface-config-get-set.py: Updatedoccupancy_grid_paramsJSON fields to match new layout.test-app-config-get-set-hwm-cmd.py: Addedhw_configuration_setupbyte, reducedreserved3to 264, updated version to[1, 2].test-preset-get-set.py: Removedsurface_heightfrom environment section.Test plan
realsense-viewercompiles without errors (verified locally)test-interface-config-get-set.pyagainst D585S device with flash v0.95test-app-config-get-set-hwm-cmd.pyagainst D585S device with flash v0.95test-preset-get-set.pyagainst D585S device with flash v0.95occupancy_grid_paramsfieldshw_configuration_setupfield