Skip to content

fix: sync sensor enum when overlaying patched sensor source#815

Open
mjc wants to merge 1 commit into
sipeed:mainfrom
mjc:mjc/fix-sensor-abi-sync
Open

fix: sync sensor enum when overlaying patched sensor source#815
mjc wants to merge 1 commit into
sipeed:mainfrom
mjc:mjc/fix-sensor-abi-sync

Conversation

@mjc

@mjc mjc commented Jun 6, 2026

Copy link
Copy Markdown

Summary

When the SG2002 build script overlays NanoKVM's patched sample_common_sensor.c into MaixCDK, also synchronize MaixCDK's matching sample_comm.h sensor enum so both sides keep the same sensor ABI.

This now uses an explicit patch file:

  • support/sg2002/additional/sophgo-middleware/v2/sample/common/sample_comm.h.patch

Problem

sample_common_sensor.c and sample_comm.h form a shared sensor ABI. The enum values from sample_comm.h are used by the sensor handling implemented in sample_common_sensor.c.

NanoKVM replaces MaixCDK's stock sample_common_sensor.c during support/sg2002/build, but the corresponding MaixCDK header was previously left unchanged. If the patched source removes sensor cases while the header keeps those enum entries, later sensor IDs can shift out of alignment and a rebuilt kvm_vision / native library can select the wrong sensor branch at runtime.

Fix

Add sync_sensor_abi() to support/sg2002/build and call it anywhere the patched sample_common_sensor.c is copied into MaixCDK.

The helper now:

  • dry-runs and applies sample_comm.h.patch against MaixCDK's sample_comm.h;
  • preserves the original header mode after patching;
  • treats a matching already-patched header as success, so update_lib can be rerun safely;
  • fails loudly if the header is neither patchable nor recognizable as already patched;
  • keeps the overlay copy list in one install_nanokvm_libs() helper;
  • backs up the original sample_comm.h during first-time overlay setup.

update_lib now exits with status 0 on success.

中文说明

SG2002 构建脚本会把 NanoKVM 修改过的 sample_common_sensor.c 覆盖到 MaixCDK 中,但之前没有同步修改对应的 sample_comm.h 传感器枚举。

sample_common_sensor.csample_comm.h 需要保持一致,否则重新编译 kvm_vision / native library 时,后面的传感器 ID 可能会发生偏移,导致运行时选择错误的 sensor 处理分支。

这个 PR 新增一个明确的 sample_comm.h.patch,在同步 NanoKVM 的 sample_common_sensor.c 后,用补丁同步 MaixCDK 的 sample_comm.h。补丁可以重复运行;如果 header 已经同步过会成功退出,如果 header 内容和预期不匹配则会报错停止。

Testing

  • bash -n support/sg2002/build
  • git diff --check
  • Fake MaixCDK update_lib smoke test against upstream sample_comm.h
  • Re-ran the fake update_lib flow twice to verify idempotence
  • Verified patched sample_comm.h preserves mode 640
  • Verified an unrelated/mismatched header fails with Sensor header patch does not match

Copilot AI review requested due to automatic review settings June 6, 2026 23:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a build-step utility to keep the Sophgo sensor header in sync by removing deprecated sensor IDs, and improves library update/backup behavior.

Changes:

  • Introduced sync_sensor_abi() to delete specific sensor enum entries from sample_comm.h and validate removal.
  • Fixed update_lib to return success (exit 0) after completing.
  • Improved backup flow by creating the original directory with -p and saving a copy of sample_comm.h.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread support/sg2002/build Outdated
Comment thread support/sg2002/build Outdated
Comment thread support/sg2002/build Outdated
Comment thread support/sg2002/build Outdated
@mjc mjc force-pushed the mjc/fix-sensor-abi-sync branch from 6b5011d to 6301a31 Compare June 11, 2026 19:13
@mjc mjc marked this pull request as draft June 11, 2026 19:13
@mjc mjc force-pushed the mjc/fix-sensor-abi-sync branch 2 times, most recently from 0c51204 to 105a151 Compare June 11, 2026 19:26
@mjc mjc force-pushed the mjc/fix-sensor-abi-sync branch from 105a151 to ca38487 Compare June 11, 2026 19:28
@mjc

mjc commented Jun 11, 2026

Copy link
Copy Markdown
Author

this approach is much better I think. anyway, rebased. should be ready now

@mjc mjc marked this pull request as ready for review June 11, 2026 19:28
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