Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe GPIO app was modified to use the ESP-IDF GPIO driver directly instead of the HAL GPIO abstraction. CMakeLists.txt was updated to add the 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Apps/GPIO/main/Source/Gpio.cpp (1)
14-18: Callinggpio_get_levelon unconfigured pins — confirm this is intentional.
gpio_get_level((gpio_num_t)i)is called for every pin index without first configuring the pin direction viagpio_set_direction. Per the ESP-IDF docs,gpio_get_levelcan be called without prior configuration on most ESP32 variants (the input is always connected), but this is an implementation detail rather than a documented guarantee. Since this app is intentionally bypassing the HAL to read pins that may be claimed by other drivers, this is likely acceptable — just worth noting.
Recently the SDK removed GPIO functionality because pins are now exclusively claimed by a driver. That means the GPIO app can't claim pins that are already in use... except when it calls the ESP-IDF driver code directly, which is what the app now does.
Summary by CodeRabbit
Refactor
Chores