Repository navigation
Cherry Pick PR #15155: Rename MinZ to Improved Close Range Depth - #15158
Conversation
(cherry picked from commit 2e12262)
Nir-Az
left a comment
Nir-Az
left a comment
There was a problem hiding this comment.
Automated review by rs-agentic bot 🤖
Overall this cherry-pick is clean — the rename is systematic and the conflict resolution in range_depth.py is well-documented. A few items flagged below.
| if( ! _handle ) | ||
| { | ||
| LOG_WARNING( "MinZ: dlopen(librs_depth_range.so) failed: " << ::dlerror() ); | ||
| LOG_WARNING( "Close Range Depth: dlopen(librs_depth_range.so) failed: " << ::dlerror() ); |
There was a problem hiding this comment.
🤖 [rs-agentic bot] BUG / Inconsistency — The log prefix here says "Close Range Depth:" but the feature's official name (and what close-range-depth-improver.cpp uses at line 77 in its own LOG_WARNING) is "Improved Close Range Depth". Inconsistent prefixes make grepping the log for this feature unreliable.
Suggested fix:
LOG_WARNING( "Improved Close Range Depth: dlopen(librs_depth_range.so) failed: " << ::dlerror() );| if( ! _create || ! _destroy || ! _process || ! _last_error ) | ||
| { | ||
| LOG_WARNING( "MinZ: missing symbols in librs_depth_range.so: " << ::dlerror() ); | ||
| LOG_WARNING( "Close Range Depth: missing symbols in librs_depth_range.so: " << ::dlerror() ); |
There was a problem hiding this comment.
🤖 [rs-agentic bot] BUG / Inconsistency — Same prefix mismatch as line 37 above: "Close Range Depth:" should be "Improved Close Range Depth:" to stay consistent with the official feature name used everywhere else (UI toggle, CMake status message, close-range-depth-improver.cpp LOG_WARNING).
Suggested fix:
LOG_WARNING( "Improved Close Range Depth: missing symbols in librs_depth_range.so: " << ::dlerror() );|
|
||
| std::printf("\rframe %5lld | close-range: HW %5.2f%% -> improved %5.2f%% " | ||
| "| MinZ recovered %5.1f%% | +%6d px", | ||
| "| recovered %5.1f%% | +%6d px", |
There was a problem hiding this comment.
🤖 [rs-agentic bot] Enhancement — The previous label "MinZ recovered" carried context about what was recovered. The replacement "recovered" is ambiguous when reading headless output in isolation — recovered what? Consider a more descriptive label such as:
"| close-range recovered %5.1f%% | +%6d px",This is a minor UX point but matters for users interpreting the stats printout.
| f"\rframe {f.get_frame_number():>5} │ close-range: " | ||
| f"HW {hw_pct:5.2f}% → improved {imp_pct:5.2f}% " | ||
| f"│ MinZ recovered {recovery_pct:5.1f}% │ +{rescued:>6} px" | ||
| f"│ recovered {recovery_pct:5.1f}% │ +{rescued:>6} px" |
There was a problem hiding this comment.
🤖 [rs-agentic bot] Enhancement — Same as range_depth.cpp line 115: "recovered" alone is ambiguous. The Python and C++ examples should also stay in sync:
f"│ close-range recovered {recovery_pct:5.1f}% │ +{rescued:>6} px"| @@ -46,7 +46,7 @@ | |||
| improver = DepthRangeImprover(calib) | |||
| T = improver.min_z_threshold_mm | |||
| N = 640 * 480 | |||
There was a problem hiding this comment.
🤖 [rs-agentic bot] Note (conflict resolution tracking) — N = 640 * 480 was intentionally kept from the r/2.58.2 branch per the PR description (the development branch uses 1280×720). The decision is well-documented in the PR body. Please ensure a follow-up ticket or note is tracked to harmonise this with development post-release, or confirm this is the intended permanent value for the release branch.
Overview: Cherry-picks the MinZ → Improved Close Range Depth rename (#15155) onto the
r/2.58.2release branch so the release carries the official feature name.Tracked on [RSDSO-21589]
Cherry-pick of #15155 (commit
2e1226249).Summary
BUILD_WITH_MINZ→BUILD_WITH_CLOSE_RANGE_DEPTH(option, compile definition,MINZ_*configure vars →CLOSE_RANGE_DEPTH_*)minz_filter/min_z_depth_improver→close_range_depth_filter/close_range_depth_improver; files renamed via git mvexamples/enhanced-depth-range/README.md,examples/readme.md,.github/skills/build.mdExternal artifact names unchanged:
librealsense2-enhanced-depthdeb,librs_depth_range.so,/optpaths, readme-media GIF URL.Conflicts resolved
examples/enhanced-depth-range/range_depth.py: kept this branch'sN = 640 * 480resolution (development uses 1280×720); applied only theMinZ→Min-Zstring rename.Test plan
realsense-viewer(flag OFF, default config) — cleangit grep -i minz— only intentional leftovers (external GIF URL, labview physical min-Z)