Skip to content

Fix camera probe detecting non-camera peripheral #756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rjoly-getraline
Copy link

@rjoly-getraline rjoly-getraline commented Jun 30, 2025

Description

Esp32-camera library fails to detect a camera on the I2C/SCCB bus when another peripheral has the same address of one of the sensors on the sensor list.
In my case, I have a custom board with a OV5640 camera connected. Esp32-camera library detects a camera at the address 0x21 (OV7725 on the list), which is not a camera on my custom board, and then logically fails to initialize it.

In this PR, I changed the process behind "camera_probe" and "SCCB_Probe" to take into account the case where an I2C address is successfully probed but camera detection fails in which case the rest of the sensor list will be tested.

Related

Testing

This PR was successfully tested on my custom board using a 0V5640.
Using ESP-IDF v5.4.1.


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

@rjoly-getraline rjoly-getraline marked this pull request as ready for review July 1, 2025 06:50
@me-no-dev me-no-dev requested a review from Copilot July 1, 2025 07:41
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the camera probing logic to handle cases where non-camera peripherals share sensor I2C addresses by probing each known sensor address and only accepting it once the actual camera is detected.

  • SCCB_Probe now takes an explicit address parameter and returns an ESP error code.
  • Updated both SCCB implementations (sccb.c and sccb-ng.c) and the header to match the new signature.
  • camera_probe in esp_camera.c now loops through all sensor addresses, retries on false positives, and logs sensor IDs.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
driver/sccb.c Changed SCCB_Probe to int SCCB_Probe(uint8_t slave_addr) and return esp_err_t.
driver/sccb-ng.c Updated SCCB_Probe to probe a given address and call SCCB_Install_Device.
driver/private_include/sccb.h Updated prototype for SCCB_Probe to take uint8_t slave_addr and return int.
driver/esp_camera.c Replaced single probe call with a loop over camera_sensor[], retrying until success.
Comments suppressed due to low confidence (3)

driver/sccb-ng.c:201

  • [nitpick] Parameter name slv_addr here differs from the header's slave_addr; consider using a consistent name across prototype and implementation for clarity.
int SCCB_Probe(uint8_t slv_addr)

driver/esp_camera.c:218

  • [nitpick] Add a brief comment explaining that this loop probes each known sensor address and continues until a supported camera is detected, to aid future maintainers.
    for(camera_model_id = 0; (*out_camera_model == CAMERA_NONE && camera_model_id < CAMERA_MODEL_MAX) ; camera_model_id++) {

driver/esp_camera.c:218

  • There's no existing test covering the fallback behavior when an SCCB address probe succeeds but sensor detection fails; consider adding unit or integration tests for this scenario.
    for(camera_model_id = 0; (*out_camera_model == CAMERA_NONE && camera_model_id < CAMERA_MODEL_MAX) ; camera_model_id++) {

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.

1 participant