[OhadMeir]

Tracked on [RSDEV-12511]

[sysrsbuild-gh-agentic]

Automated review of PR #15352 — Add NV12 support to D500.

Overall the change is clean: the rename from d500_dual_rgb → d500_dual_color is well-scoped, the new fourcc registrations are consistent, and the pin-detection heuristic update is logically correct. A few points below need attention before merge.

🤖 Generated by rs-agentic-bot

[sysrsbuild-gh-agentic]

[rs-agentic-bot] Missing test coverage for NV12 support

This PR adds NV12 as a supported format for D500 (D585/D535) color streams but does not include any new unit tests. The following scenarios should be covered:

1. Format enumeration: verify that NV12 profiles are enumerated on a D585/D535 device alongside existing M420/YUY2 profiles.
2. NV12 → RGB conversion: verify that nv12_converter correctly converts a synthetic NV12 frame to each of RS2_FORMAT_RGB8, RS2_FORMAT_RGBA8, RS2_FORMAT_BGR8, RS2_FORMAT_BGRA8.
3. Pin classification (is_color_pin): add a unit test with a synthesised platform::stream_profile vector that exercises the NV12 + YUY2 (color pin) vs NV12 + UYVY (IR pin) paths to guard against the firmware-contract assumption described in the inline comment.

Existing tests under unit-tests/ should be checked to confirm none of the above is already covered before adding new ones.

🤖 Generated by rs-agentic-bot

[OhadMeir]

[rs-agentic-bot] Missing test coverage for NV12 support

This will be tested under the D585 requirements tests. Not in scope for this PR

[sysrsbuild-gh-agentic]

[rs-agentic-bot] Synchronize-push review — commit 16a289fac ("Handle PR#15352 comments")

This push touches only d500-color.cpp (2 additions / 5 deletions), condensing the 5-line comment in the M420/NV12 case block into 2 lines as requested in the previous review round.

Threads resolved by this push:

• ✅ Thread 2 (comment length) — comment rephrased to 2 concise lines; no information lost.

Threads resolved in prior commits / accepted as-is:

• ✅ Thread 1 (unconditional cross-format registration) — developer confirmed both NV12 and M420 co-exist on the same firmware; comment now makes that explicit.
• ✅ Thread 3 (silent insert() no-op for YUY2/YUYV) — developer consciously accepts the risk; acceptable for additive-only initialisation code.
• ✅ Thread 4 (is_color_pin implicit firmware contract) — invariant is now stated in the updated comment; see reply in that thread.

Remaining open item (non-blocking):

• 📋 Unit test for is_color_pin (suggested in Thread 4) was not added. The existing comment now makes the contract clear, so this is low-risk for the current change, but adding a synthesised-profile-list test would provide regression safety if future firmware changes alter the YUY2/UYVY companion pattern. Recommend tracking as a follow-up.

No new bugs or guideline violations found in this push. The change is safe to proceed to the next review stage.

🤖 Generated by rs-agentic-bot

[Nir-Az]

LGTM, lets see CI results