how to handle multiple comonents calling install_gpio_isr_service/uninstall_gpio_isr_service/gpio_isr_handler_add

[greenaddress]

From version 2.0.4 throughout version 2.0.8 (latest stable) esp32-camera calls uninstall_gpio_isr_service from within esp_camera_deinit which can "break" other components that relied on the isr_service presence.

This was introduced in this MR https://github.com/espressif/esp32-camera/pull/516

This can be worked around by uninit other components before camera use, and reinit after camera use. Additionally if one can change the component, and requires the component in use during camera use, one could re-add handlers after camera ini by skipping service install and likewise remove handlers and skipping service uninstall.

In general the work around doesn't seem ideal.

Maybe esp-camera init should let the library user decide whether esp_camera_deinit should uninstall the gpio isr service.

Or some other solution that allows different components to co-exist around install_gpio_isr_service/uninstall_gpio_isr_service/gpio_isr_handler_add

[AxelLin]

I asked the same (similar) question in https://github.com/espressif/esp-idf/issues/12882#issuecomment-1911614010 , but didn't get further response.

[igrr]

My opinion is that installing system-wide services should be done in application code, not in libraries. This applies also to things such as NVS, esp_event, esp_netif.

It would probably be a breaking change to remove the call to an GPIO service installation function, but we could at least add config argument specifying whether esp32_camera should install the GPIO service or not.

(We could also change the GPIO service API to have something like a reference counter, allowing it to be installed and uninstalled any number of times. But this would only be available in new versions of IDF, so we need another solution for esp32_camera anyway.)

[AxelLin]

My opinion is that installing system-wide services should be done in application code, not in libraries. This applies also to things such as NVS, esp_event, esp_netif.

Then I'm wondering if the application developer really understand what intr_alloc_flags should be set.

What happens if wrong intr_alloc_flags is used for gpio_install_isr_service? e.g. currently, only the first call to gpio_install_isr_service() takes effect. So the intr_alloc_flags called by ll_cam.c does not take effect if other component already calls gpio_install_isr_service().

[igrr]

Yeah, unfortunately that's the complexity the application developer currently needs to bear.

If we were to change gpio_isr_handler_add to also take the flags, we could at least error out on calls to gpio_isr_handler_add with incompatible flags (i.e. higher interrupt level than requested in gpio_install_isr_service, or IRAM flag, when gpio_install_isr_service hasn't requested it.)

The reference counting approach I mentioned above wouldn't solve this particular issue, I think, because we can't re-register the ISR with different flags without potentially missing some interrupts — on Xtensa, at least.

[AxelLin]

From version 2.0.4 throughout version 2.0.8 (latest stable) esp32-camera calls uninstall_gpio_isr_service from within esp_camera_deinit which can "break" other components that relied on the isr_service presence.

This was introduced in this MR #516

@me-no-dev Any update for this issue?

[me-no-dev]

@AxelLin will revert this and check for "already started" on init. I see no other options to help the situation

[AxelLin]

@AxelLin will revert this and check for "already started" on init. I see no other options to help the situation

my main concern is not about "already started" on init, but about the uninstall_gpio_isr_service from within esp_camera_deinit which can "break" other components.

[me-no-dev]

both things need to happen in order for the driver to be restarted and ISR service to not be re-initialized. The PR you linked above was attempting to fix failed re-init, because "service is already started"

[me-no-dev]

Change is reverted: https://github.com/espressif/esp32-camera/commit/28296929286584d38e0a9e3456029204898a59a7