fix: Handle pre-existing I2C driver gracefully in epd_board_v7 #460
Conversation
|
(and yes, done with the assistance of Claude and Gemini but I'm in control of the clankers and this seems sane to me) |
Bwooce
force-pushed
the
i2c-sharing-fix
branch
from
f3c7d20 to
3059a40
Compare
Check i2c_driver_install() BEFORE calling i2c_param_config() to avoid reconfiguring GPIO pins and I2C settings when Arduino Wire (or another component) has already initialized the bus. Key changes: - Reorder to check driver installation status first - Only call i2c_param_config() if we installed the driver ourselves - Track ownership with i2c_driver_installed_by_us flag - Track ISR service ownership with isr_service_installed_by_us flag - Conditional cleanup in deinit - only delete what we installed This allows epdiy to coexist with Arduino Wire for touch controllers, RTCs, and other I2C devices sharing the same bus.
Bwooce
force-pushed
the
i2c-sharing-fix
branch
from
3059a40 to
57bb2e9
Compare
martinberlin
left a comment
martinberlin
left a comment
There was a problem hiding this comment.
For me this looks fine and is always a hassle to deal with I2C. Just needs to be tested thoughfully
|
REQUESTING small update here: /Users/martin/esp/esp-idf/components/hal/esp32s3/include/hal/lcd_ll.h:726:15: error: '__DECLARE_RCC_ATOMIC_ENV' undeclared (first use in this function) For me is failing with this, please check my other PR where I fixed this in src/output_lcd/lcd_driver.h, but I'm waiting for @vroland approval otherwise cannot merge this. |
|
ack, will take a look. |
IDF 5.5 added a macro wrapper around lcd_ll_enable_interrupt() that references __DECLARE_RCC_ATOMIC_ENV, requiring the call to be inside a PERIPH_RCC_ATOMIC() block. Without this, builds fail on IDF 5.5+.
|
added the same fix, hopefully, to this PR but if I need to de-conflict it later will do. so this should pass for you now Martin. |
With all due respect dear @Bwooce : Please stop using IA for every little change and please focus on what I signaled that you in #452 Sorry but I really don't like that you need to use Claude for every little update instead of humanly looking into what I said and correcting it. |
martinberlin
left a comment
martinberlin
left a comment
There was a problem hiding this comment.
Please look thoughtfully about what I pointed out and solve it without AI, testing it with IDF 5.5 yourself.
| // IDF 5.5+ wraps lcd_ll_enable_interrupt in a macro requiring PERIPH_RCC_ATOMIC() | ||
| #if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 5, 0) | ||
| PERIPH_RCC_ATOMIC() { | ||
| lcd_ll_enable_interrupt(lcd.hal.dev, LCD_LL_EVENT_VSYNC_END, true); |
There was a problem hiding this comment.
No I didn't asked for this.
martinberlin
requested review from
martinberlin
and removed request for
vroland
2 participants
Problem
When using epdiy on boards with other I2C peripherals (touch controllers, RTCs, battery gauges), users often initialize Arduino's
Wire.begin()before callingepd_init(). This causesi2c_driver_install()to fail withESP_ERR_INVALID_STATEbecause the driver is already installed.Similarly,
gpio_install_isr_service()can fail if other code has already installed the ISR service.Solution
Check return values and accept
ESP_ERR_INVALID_STATEas success, allowing epdiy to reuse the existing I2C driver and ISR service instead of failing.Changes
epd_board_v7.c: Checki2c_param_config(),i2c_driver_install(), andgpio_install_isr_service()return valuesESP_ERR_INVALID_STATEas non-fatal (driver/service already exists)Testing
Tested on LilyGo T5 S3 E-Paper Pro sharing I2C bus with:
All peripherals coexist on the same I2C bus (SDA=39, SCL=40).