GPIO descriptor fixes#578
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe GPIO API now accepts acquisition-time flags, stores descriptor configuration, supports pin-spec acquisition, and exposes flag queries. Drivers and ESP32 peripherals pass direction, pull, interrupt, ownership, and active-low settings during acquisition. GPIO level handling was updated for active-low expanders and ESP32 pins. Display and touch bindings remove configurable polarity fields, while reset and interrupt implementations use fixed active-low behavior. Board GPIO hogs and device configurations were updated accordingly. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 9
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04cc9dcd-10b8-468b-997e-14364ac74d44
📒 Files selected for processing (112)
Devices/m5stack-cores3/m5stack,cores3.dtsDevices/m5stack-papers3/source/drivers/papers3_power.cppDevices/m5stack-stackchan/m5stack,stackchan.dtsDevices/m5stack-tab5/Source/devices/devices_v1.cppDevices/m5stack-tab5/Source/devices/devices_v2.cppDevices/m5stack-tab5/Source/devices/display_detect.cppDevices/m5stack-tab5/Source/devices/tab5_headphone_detect.cppDevices/m5stack-tab5/Source/devices/tab5_power_control.cppDevices/m5stack-tab5/Source/module.cppDevices/unphone/source/BatteryManager.cppDevices/unphone/source/drivers/unphone_nav_buttons.cppDevices/unphone/source/drivers/unphone_power_switch.cppDrivers/audio-codec-module/source/audio_codec_gpio_if.cDrivers/aw88298-module/source/aw88298.cppDrivers/aw9523b-module/source/aw9523b.cppDrivers/axs5106-module/bindings/axs,axs5106.yamlDrivers/axs5106-module/include/drivers/axs5106.hDrivers/axs5106-module/source/axs5106.cppDrivers/button-control-module/source/button_control.cppDrivers/cst328-module/bindings/hynitron,cst328.yamlDrivers/cst328-module/include/drivers/cst328.hDrivers/cst328-module/source/cst328.cppDrivers/cst66xx-module/bindings/hynitron,cst66xx.yamlDrivers/cst66xx-module/include/drivers/cst66xx.hDrivers/cst66xx-module/source/cst66xx.cppDrivers/cst816s-module/bindings/hynitron,cst816s.yamlDrivers/cst816s-module/include/drivers/cst816s.hDrivers/cst816s-module/source/cst816s.cppDrivers/cst816t-module/bindings/hynitron,cst816t.yamlDrivers/cst816t-module/include/drivers/cst816t.hDrivers/cst816t-module/source/cst816t.cppDrivers/ft5x06-module/bindings/focaltech,ft5x06.yamlDrivers/ft5x06-module/include/drivers/ft5x06.hDrivers/ft5x06-module/source/ft5x06.cppDrivers/ft6x36-module/bindings/focaltech,ft6x36.yamlDrivers/ft6x36-module/include/drivers/ft6x36.hDrivers/ft6x36-module/source/ft6x36.cppDrivers/gc9a01-module/bindings/galaxycore,gc9a01.yamlDrivers/gc9a01-module/include/drivers/gc9a01.hDrivers/gc9a01-module/source/gc9a01.cppDrivers/gt911-module/bindings/goodix,gt911.yamlDrivers/gt911-module/include/drivers/gt911.hDrivers/gt911-module/source/gt911.cppDrivers/hx8357-module/bindings/himax,hx8357.yamlDrivers/hx8357-module/include/drivers/hx8357.hDrivers/hx8357-module/source/hx8357.cppDrivers/ili9341-module/source/ili9341.cppDrivers/ili9488-module/bindings/ilitek,ili9488.yamlDrivers/ili9488-module/include/drivers/ili9488.hDrivers/ili9488-module/source/ili9488.cppDrivers/ili9881c-module/bindings/ilitek,ili9881c.yamlDrivers/ili9881c-module/include/drivers/ili9881c.hDrivers/ili9881c-module/source/ili9881c.cppDrivers/jd9165-module/bindings/jdi,jd9165.yamlDrivers/jd9165-module/include/drivers/jd9165.hDrivers/jd9165-module/source/jd9165.cppDrivers/jd9853-module/bindings/jadard,jd9853.yamlDrivers/jd9853-module/include/drivers/jd9853.hDrivers/jd9853-module/source/jd9853.cppDrivers/lilygo-module/source/tdeck_trackball.cppDrivers/lilygo-module/source/tpager_encoder.cppDrivers/m5stack-module/source/cardputer_keyboard.cppDrivers/pi4ioe5v6408-module/source/pi4ioe5v6408.cppDrivers/rgb-display-module/source/rgb_display.cppDrivers/ssd1306-module/bindings/solomon,ssd1306.yamlDrivers/ssd1306-module/include/drivers/ssd1306.hDrivers/ssd1306-module/source/ssd1306.cppDrivers/st7123-module/bindings/sitronix,st7123-touch.yamlDrivers/st7123-module/bindings/sitronix,st7123.yamlDrivers/st7123-module/include/drivers/st7123.hDrivers/st7123-module/include/drivers/st7123_touch.hDrivers/st7123-module/source/st7123.cppDrivers/st7123-module/source/st7123_touch.cppDrivers/st7701-module/bindings/sitronix,st7701.yamlDrivers/st7701-module/include/drivers/st7701.hDrivers/st7701-module/source/st7701.cppDrivers/st7735-module/bindings/sitronix,st7735.yamlDrivers/st7735-module/include/drivers/st7735.hDrivers/st7735-module/source/st7735.cppDrivers/st7789-i8080-module/bindings/sitronix,st7789-i8080.yamlDrivers/st7789-i8080-module/include/drivers/st7789_i8080.hDrivers/st7789-i8080-module/source/st7789_i8080.cppDrivers/st7789-module/bindings/sitronix,st7789.yamlDrivers/st7789-module/include/drivers/st7789.hDrivers/st7789-module/source/st7789.cppDrivers/st7796-i8080-module/bindings/sitronix,st7796-i8080.yamlDrivers/st7796-i8080-module/include/drivers/st7796_i8080.hDrivers/st7796-i8080-module/source/st7796_i8080.cppDrivers/st7796-module/bindings/sitronix,st7796.yamlDrivers/st7796-module/include/drivers/st7796.hDrivers/st7796-module/source/st7796.cppDrivers/tca8418-module/bindings/ti,tca8418.yamlDrivers/tca8418-module/include/drivers/tca8418.hDrivers/tca8418-module/source/tca8418.cppDrivers/xpt2046-softspi-module/source/xpt2046_softspi.cppPlatforms/platform-esp32/private/tactility/drivers/esp32_gpio_helpers.hPlatforms/platform-esp32/source/drivers/esp32_gpio.cppPlatforms/platform-esp32/source/drivers/esp32_gpio_helpers.cppPlatforms/platform-esp32/source/drivers/esp32_i2c.cppPlatforms/platform-esp32/source/drivers/esp32_i2c_master.cppPlatforms/platform-esp32/source/drivers/esp32_i2s.cppPlatforms/platform-esp32/source/drivers/esp32_i8080.cppPlatforms/platform-esp32/source/drivers/esp32_sdmmc.cppPlatforms/platform-esp32/source/drivers/esp32_sdspi.cppPlatforms/platform-esp32/source/drivers/esp32_spi.cppPlatforms/platform-esp32/source/drivers/esp32_uart.cppTactilityKernel/include/tactility/drivers/gpio_controller.hTactilityKernel/include/tactility/drivers/gpio_descriptor.hTactilityKernel/source/drivers/gpio_backlight.cppTactilityKernel/source/drivers/gpio_controller.cppTactilityKernel/source/drivers/gpio_hog.cppTactilityKernel/source/drivers/rgb_led_gpio.cpp
💤 Files with no reviewable changes (49)
- Drivers/st7796-i8080-module/include/drivers/st7796_i8080.h
- Drivers/ili9881c-module/bindings/ilitek,ili9881c.yaml
- Drivers/jd9165-module/include/drivers/jd9165.h
- Drivers/ili9488-module/include/drivers/ili9488.h
- Drivers/st7123-module/include/drivers/st7123_touch.h
- Drivers/st7796-module/include/drivers/st7796.h
- Drivers/hx8357-module/bindings/himax,hx8357.yaml
- Drivers/st7701-module/include/drivers/st7701.h
- Drivers/st7789-module/bindings/sitronix,st7789.yaml
- Drivers/ili9881c-module/include/drivers/ili9881c.h
- Drivers/cst328-module/include/drivers/cst328.h
- Drivers/st7789-module/include/drivers/st7789.h
- Drivers/st7123-module/bindings/sitronix,st7123.yaml
- Drivers/gt911-module/include/drivers/gt911.h
- Drivers/cst66xx-module/include/drivers/cst66xx.h
- Drivers/cst66xx-module/bindings/hynitron,cst66xx.yaml
- Drivers/st7735-module/include/drivers/st7735.h
- Drivers/ssd1306-module/include/drivers/ssd1306.h
- Drivers/gc9a01-module/bindings/galaxycore,gc9a01.yaml
- Drivers/cst816t-module/include/drivers/cst816t.h
- Drivers/ili9488-module/bindings/ilitek,ili9488.yaml
- Drivers/ft5x06-module/include/drivers/ft5x06.h
- Drivers/st7789-i8080-module/bindings/sitronix,st7789-i8080.yaml
- Drivers/hx8357-module/include/drivers/hx8357.h
- Drivers/st7123-module/include/drivers/st7123.h
- Drivers/tca8418-module/bindings/ti,tca8418.yaml
- Drivers/jd9853-module/include/drivers/jd9853.h
- Drivers/jd9165-module/bindings/jdi,jd9165.yaml
- Drivers/gc9a01-module/include/drivers/gc9a01.h
- Drivers/st7735-module/bindings/sitronix,st7735.yaml
- Drivers/cst328-module/bindings/hynitron,cst328.yaml
- Drivers/st7789-i8080-module/include/drivers/st7789_i8080.h
- Drivers/axs5106-module/bindings/axs,axs5106.yaml
- Drivers/tca8418-module/include/drivers/tca8418.h
- Drivers/ft6x36-module/bindings/focaltech,ft6x36.yaml
- Drivers/cst816s-module/bindings/hynitron,cst816s.yaml
- Drivers/ssd1306-module/bindings/solomon,ssd1306.yaml
- Drivers/axs5106-module/include/drivers/axs5106.h
- Drivers/cst816t-module/bindings/hynitron,cst816t.yaml
- Drivers/st7796-module/bindings/sitronix,st7796.yaml
- Drivers/st7123-module/bindings/sitronix,st7123-touch.yaml
- Drivers/ft5x06-module/bindings/focaltech,ft5x06.yaml
- Drivers/jd9853-module/bindings/jadard,jd9853.yaml
- Drivers/ft6x36-module/include/drivers/ft6x36.h
- Drivers/cst816s-module/include/drivers/cst816s.h
- Drivers/st7796-i8080-module/bindings/sitronix,st7796-i8080.yaml
- Devices/m5stack-tab5/Source/devices/devices_v2.cpp
- Drivers/gt911-module/bindings/goodix,gt911.yaml
- Drivers/st7701-module/bindings/sitronix,st7701.yaml
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
TactilityKernel/source/drivers/gpio_hog.cpp (1)
44-48: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInitialize output-low hogs explicitly.
For
GPIO_HOG_MODE_OUTPUT_LOW,initial_highis false, so Line 44 skipsgpio_descriptor_set_level(). Acquiring output direction does not guarantee the requested low state; preserve the explicit level initialization for both output modes while leaving input mode untouched.Proposed fix
- if (initial_high && gpio_descriptor_set_level(descriptor, initial_high) != ERROR_NONE) { + if (config->mode != GPIO_HOG_MODE_INPUT && + gpio_descriptor_set_level(descriptor, initial_high) != ERROR_NONE) {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40d1d315-65ed-4f38-890f-df7bd9fe89ca
📒 Files selected for processing (21)
Devices/m5stack-papers3/source/drivers/papers3_power.cppDevices/unphone/source/drivers/unphone_nav_buttons.cppDevices/waveshare-s3-touch-lcd-43/waveshare,s3-touch-lcd-43.dtsDocumentation/ideas.mdDrivers/audio-codec-module/source/audio_codec_gpio_if.cDrivers/axs5106-module/source/axs5106.cppDrivers/button-control-module/source/button_control.cppDrivers/cst66xx-module/source/cst66xx.cppDrivers/cst816t-module/source/cst816t.cppDrivers/ft6x36-module/source/ft6x36.cppDrivers/ili9341-module/source/ili9341.cppDrivers/ili9488-module/source/ili9488.cppDrivers/st7735-module/source/st7735.cppDrivers/st7789-i8080-module/source/st7789_i8080.cppDrivers/st7789-module/source/st7789.cppPlatforms/platform-esp32/source/drivers/esp32_i2c.cppPlatforms/platform-esp32/source/drivers/esp32_i2c_master.cppPlatforms/platform-esp32/source/drivers/esp32_i8080.cppPlatforms/platform-esp32/source/drivers/esp32_sdmmc.cppTactilityKernel/source/drivers/gpio_controller.cppTactilityKernel/source/drivers/gpio_hog.cpp
🚧 Files skipped from review as they are similar to previous changes (14)
- Drivers/ili9488-module/source/ili9488.cpp
- Drivers/st7735-module/source/st7735.cpp
- Drivers/st7789-i8080-module/source/st7789_i8080.cpp
- Devices/unphone/source/drivers/unphone_nav_buttons.cpp
- Drivers/cst66xx-module/source/cst66xx.cpp
- Drivers/st7789-module/source/st7789.cpp
- Platforms/platform-esp32/source/drivers/esp32_i2c.cpp
- Drivers/button-control-module/source/button_control.cpp
- Drivers/cst816t-module/source/cst816t.cpp
- Drivers/audio-codec-module/source/audio_codec_gpio_if.c
- TactilityKernel/source/drivers/gpio_controller.cpp
- Platforms/platform-esp32/source/drivers/esp32_i2c_master.cpp
- Drivers/ft6x36-module/source/ft6x36.cpp
- Drivers/ili9341-module/source/ili9341.cpp
Summary by CodeRabbit
Bug Fixes
Compatibility
reset-active-high/interrupt-active-highoptions from multiple bindings/configs; polarity is now defined via the GPIO specs (e.g.,pin-reset/pin-interrupt).Device Configuration