fix SP2305 detection - #180
Conversation
Signed-off-by: Conke Hu <conke.hu@maxwit.com>
PR Summary by QodoFix SP2305 sensor detection mapping
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. Misleading OV2735 comment
|
| // Omnivision-SuperPix OV2735 | ||
| case 0x2735: | ||
| sprintf(ctx->sensor_id, "OV%04x", res); | ||
| strcpy(ctx->sensor_id, "SP2305"); |
There was a problem hiding this comment.
1. Misleading ov2735 comment 🐞 Bug ⚙ Maintainability
In detect_superpix_sensor(), the 0x2735 case now sets ctx->sensor_id to "SP2305" while the adjacent comment still references "OV2735", leaving the code’s documentation inconsistent with its behavior. This can mislead maintainers about what sensor/model is actually being reported for that register value.
Agent Prompt
### Issue description
The SuperPix detection switch case for `0x2735` now returns the model string `"SP2305"`, but the nearby comment still says `OV2735`, which is inconsistent and confusing for future maintenance.
### Issue Context
This PR changed the model string for `case 0x2735:` from `OV%04x` to a literal `SP2305`, but did not update the comment explaining the case.
### Fix Focus Areas
- src/sensors.c[907-910]
### Suggested fix
Update the comment to accurately describe why `0x2735` maps to `SP2305` (e.g., note that SP2305 reports an OV2735-compatible ID), or adjust the mapping/code so the comment and returned model name are consistent.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Thanks for the patch, but the PR description is empty, so I can't tell what problem this is fixing. Before we change detection behaviour I need the rationale:
The reason I'm asking rather than merging: as far as we can tell SP2305 and OV2735 are the same silicon sold under two brands, so
There's also a compatibility problem.
So as it stands this patch would drop those boards into the If your register dump shows something that genuinely differs between the two labels ( |
No description provided.