Merge pull request #6940 from jepler/esp32-camera-lock-bus

esp32_camera: lock and unlock i2c bus around any ops that might access it

Author: microdev1

ports/espressif/bindings/esp32_camera/Camera.c

@@ -46,9 +46,9 @@
 //|         self,
 //|         *,
 //|         data_pins: List[microcontroller.Pin],
-//|         pixel_clock: microcontroller.Pin,
-//|         vsync: microcontroller.Pin,
-//|         href: microcontroller.Pin,
+//|         pixel_clock_pin: microcontroller.Pin,
+//|         vsync_pin: microcontroller.Pin,
+//|         href_pin: microcontroller.Pin,
 //|         i2c: busio.I2C,
 //|         external_clock_pin: microcontroller.Pin,
 //|         external_clock_frequency: int,
@@ -79,12 +79,12 @@
 //|             that case.
 //|
 //|         :param data_pins: The 8 data data_pins used for image data transfer from the camera module, least significant bit first
-//|         :param pixel_clock: The pixel clock output from the camera module
-//|         :param vsync: The vertical sync pulse output from the camera module
-//|         :param href: The horizontal reference output from the camera module
+//|         :param pixel_clock_pin: The pixel clock output from the camera module
+//|         :param vsync_pin: The vertical sync pulse output from the camera module
+//|         :param href_pin: The horizontal reference output from the camera module
 //|         :param i2c: The I2C bus connected to the camera module
-//|         :param external_clock_frequency: The frequency generated on the external clock pin
 //|         :param external_clock_pin: The pin on which to generate the external clock
+//|         :param external_clock_frequency: The frequency generated on the external clock pin
 //|         :param powerdown_pin: The powerdown input to the camera module
 //|         :param reset_pin: The reset input to the camera module
 //|         :param pixel_format: The pixel format of the captured image
@@ -125,7 +125,7 @@ STATIC mp_obj_t esp32_camera_camera_make_new(const mp_obj_type_t *type, size_t n
     const mcu_pin_obj_t *pixel_clock_pin = validate_obj_is_free_pin(args[ARG_pixel_clock_pin].u_obj);
     const mcu_pin_obj_t *vsync_pin = validate_obj_is_free_pin(args[ARG_vsync_pin].u_obj);
     const mcu_pin_obj_t *href_pin = validate_obj_is_free_pin(args[ARG_href_pin].u_obj);
-    const busio_i2c_obj_t *i2c = MP_OBJ_TO_PTR(mp_arg_validate_type(args[ARG_i2c].u_obj, &busio_i2c_type, MP_QSTR_i2c));
+    busio_i2c_obj_t *i2c = MP_OBJ_TO_PTR(mp_arg_validate_type(args[ARG_i2c].u_obj, &busio_i2c_type, MP_QSTR_i2c));
     const mcu_pin_obj_t *external_clock_pin = validate_obj_is_free_pin(args[ARG_external_clock_pin].u_obj);
     const mcu_pin_obj_t *powerdown_pin = validate_obj_is_free_pin_or_none(args[ARG_powerdown_pin].u_obj);
     const mcu_pin_obj_t *reset_pin = validate_obj_is_free_pin_or_none(args[ARG_reset_pin].u_obj);

ports/espressif/bindings/esp32_camera/Camera.h

@@ -45,7 +45,7 @@ extern void common_hal_esp32_camera_camera_construct(
     const mcu_pin_obj_t *href_pin,
     const mcu_pin_obj_t *powerdown_pin,
     const mcu_pin_obj_t *reset_pin,
-    const busio_i2c_obj_t *i2c,
+    busio_i2c_obj_t *i2c,
     mp_int_t external_clock_frequency,
     pixformat_t pixel_format,
     framesize_t frame_size,

ports/espressif/common-hal/esp32_camera/Camera.c

@@ -24,12 +24,15 @@
  * THE SOFTWARE.
  */
 
+#include "py/mperrno.h"
 #include "py/runtime.h"
 
 #include "bindings/esp32_camera/Camera.h"
 #include "bindings/espidf/__init__.h"
 #include "common-hal/esp32_camera/Camera.h"
+#include "shared-bindings/busio/I2C.h"
 #include "shared-bindings/microcontroller/Pin.h"
+#include "shared-bindings/util.h"
 #include "common-hal/microcontroller/Pin.h"
 
 #include "esp32-camera/driver/private_include/cam_hal.h"
@@ -38,6 +41,19 @@
 #error esp32_camera only works on boards configured with spiram, disable it in mpconfigboard.mk
 #endif
 
+static void i2c_lock(esp32_camera_camera_obj_t *self) {
+    if (common_hal_busio_i2c_deinited(self->i2c)) {
+        raise_deinited_error();
+    }
+    if (!common_hal_busio_i2c_try_lock(self->i2c)) {
+        mp_raise_OSError(MP_EWOULDBLOCK);
+    }
+}
+
+static void i2c_unlock(esp32_camera_camera_obj_t *self) {
+    common_hal_busio_i2c_unlock(self->i2c);
+}
+
 static void maybe_claim_pin(const mcu_pin_obj_t *pin) {
     if (pin) {
         claim_pin(pin);
@@ -53,7 +69,7 @@ void common_hal_esp32_camera_camera_construct(
     const mcu_pin_obj_t *href_pin,
     const mcu_pin_obj_t *powerdown_pin,
     const mcu_pin_obj_t *reset_pin,
-    const busio_i2c_obj_t *i2c,
+    busio_i2c_obj_t *i2c,
     mp_int_t external_clock_frequency,
     pixformat_t pixel_format,
     framesize_t frame_size,
@@ -78,6 +94,8 @@ void common_hal_esp32_camera_camera_construct(
 
     common_hal_pwmio_pwmout_construct(&self->pwm, external_clock_pin, 1, external_clock_frequency, true);
 
+    self->i2c = i2c;
+
     self->camera_config.pin_pwdn = common_hal_mcu_pin_number(powerdown_pin);
     self->camera_config.pin_reset = common_hal_mcu_pin_number(reset_pin);
     self->camera_config.pin_xclk = common_hal_mcu_pin_number(external_clock_pin);
@@ -112,7 +130,11 @@ void common_hal_esp32_camera_camera_construct(
 
     self->camera_config.sccb_i2c_port = i2c->i2c_num;
 
-    CHECK_ESP_RESULT(esp_camera_init(&self->camera_config));
+    i2c_lock(self);
+    esp_err_t result = esp_camera_init(&self->camera_config);
+    i2c_unlock(self);
+
+    CHECK_ESP_RESULT(result);
 }
 
 extern void common_hal_esp32_camera_camera_deinit(esp32_camera_camera_obj_t *self) {
@@ -165,7 +187,9 @@ camera_fb_t *common_hal_esp32_camera_camera_take(esp32_camera_camera_obj_t *self
 
 #define SENSOR_GET(type, name, status_field_name, setter_function_name) \
     type common_hal_esp32_camera_camera_get_##name(esp32_camera_camera_obj_t * self) { \
+        i2c_lock(self); \
         sensor_t *sensor = esp_camera_sensor_get(); \
+        i2c_unlock(self); \
         if (!sensor->setter_function_name) { \
             mp_raise_AttributeError(translate("no such attribute")); \
         } \
@@ -174,7 +198,9 @@ camera_fb_t *common_hal_esp32_camera_camera_take(esp32_camera_camera_obj_t *self
 
 #define SENSOR_SET(type, name, setter_function_name) \
     void common_hal_esp32_camera_camera_set_##name(esp32_camera_camera_obj_t * self, type value) { \
+        i2c_lock(self); \
         sensor_t *sensor = esp_camera_sensor_get(); \
+        i2c_unlock(self); \
         if (!sensor->setter_function_name) { \
             mp_raise_AttributeError(translate("no such attribute")); \
         } \
@@ -203,6 +229,7 @@ void common_hal_esp32_camera_camera_reconfigure(esp32_camera_camera_obj_t *self,
         frame_size = sensor_info->max_size;
     }
 
+    i2c_lock(self);
     cam_deinit();
     self->camera_config.pixel_format = pixel_format;
     self->camera_config.frame_size = frame_size;
@@ -212,6 +239,7 @@ void common_hal_esp32_camera_camera_reconfigure(esp32_camera_camera_obj_t *self,
     sensor->set_framesize(sensor, self->camera_config.frame_size);
     cam_init(&self->camera_config);
     cam_config(&self->camera_config, frame_size, sensor_info->pid);
+    i2c_unlock(self);
     cam_start();
 }
 

ports/espressif/common-hal/esp32_camera/Camera.h

@@ -29,10 +29,12 @@
 #include "py/obj.h"
 #include "esp_camera.h"
 #include "shared-bindings/pwmio/PWMOut.h"
+#include "common-hal/busio/I2C.h"
 
 typedef struct esp32_camera_camera_obj {
     mp_obj_base_t base;
     camera_config_t camera_config;
     camera_fb_t *buffer_to_return;
     pwmio_pwmout_obj_t pwm;
+    busio_i2c_obj_t *i2c;
 } esp32_camera_obj_t;