Smart tab5keyboard#533
Conversation
wrong logo was selected if rotation was changed from the default... yep it was weird.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR delivers several hardware improvements for the M5Stack Tab5. The Tab5 keyboard gains hot-plug detection: new 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Devices/m5stack-tab5/Source/module.cpp (1)
124-134:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRoll back
hp_detect_timerwhen keyboard timer setup fails.
start()can returnERROR_RESOURCEafterhp_detect_timeris already running. That leaves a partially started module state and can block correct re-init behavior on the nextstart()call.Suggested fix
kb_detect_timer = xTimerCreate("kb_detect", pdMS_TO_TICKS(KB_DETECT_POLL_MS), pdTRUE, nullptr, keyboardDetectCallback); if (!kb_detect_timer) { LOG_E(TAG, "Failed to create kb_detect timer"); + xTimerStop(hp_detect_timer, pdMS_TO_TICKS(100)); + xTimerDelete(hp_detect_timer, pdMS_TO_TICKS(100)); + hp_detect_timer = nullptr; return ERROR_RESOURCE; } if (xTimerStart(kb_detect_timer, pdMS_TO_TICKS(100)) != pdPASS) { LOG_E(TAG, "Failed to start kb_detect timer"); xTimerDelete(kb_detect_timer, pdMS_TO_TICKS(100)); kb_detect_timer = nullptr; + xTimerStop(hp_detect_timer, pdMS_TO_TICKS(100)); + xTimerDelete(hp_detect_timer, pdMS_TO_TICKS(100)); + hp_detect_timer = nullptr; return ERROR_RESOURCE; }Also applies to: 138-148
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7ce668b-27ed-459f-84c9-14cd0d272d22
📒 Files selected for processing (22)
Devices/m5stack-tab5/Source/Configuration.cppDevices/m5stack-tab5/Source/devices/Ili9881cDisplay.cppDevices/m5stack-tab5/Source/devices/SdCard.cppDevices/m5stack-tab5/Source/devices/SdCard.hDevices/m5stack-tab5/Source/devices/St7123Display.cppDevices/m5stack-tab5/Source/devices/Tab5Keyboard.cppDevices/m5stack-tab5/Source/devices/Tab5Keyboard.hDevices/m5stack-tab5/Source/module.cppDevices/m5stack-tab5/m5stack,tab5.dtsDrivers/EspLcdCompat/Source/EspLcdDisplayDriver.hPlatforms/platform-esp32/bindings/espressif,esp32-sdmmc.yamlPlatforms/platform-esp32/include/tactility/drivers/esp32_sdmmc.hPlatforms/platform-esp32/source/drivers/esp32_sdmmc_fs.cppTactility/Include/Tactility/hal/display/DisplayDriver.hTactility/Source/app/boot/Boot.cppTactility/Source/app/launcher/Launcher.cppTactility/Source/hal/usb/UsbTusb.cppTactilityC/CMakeLists.txtTactilityC/Include/tt_hal_display.hTactilityC/Source/symbols/gcc_soft_float_p4.cppTactilityC/Source/tt_hal_display.cppTactilityC/Source/tt_init.cpp
💤 Files with no reviewable changes (2)
- Devices/m5stack-tab5/Source/devices/SdCard.h
- Devices/m5stack-tab5/Source/devices/SdCard.cpp
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ab40414-e598-4267-8318-6f55de2aab62
📒 Files selected for processing (9)
Devices/m5stack-tab5/Source/devices/Tab5Keyboard.cppDevices/m5stack-tab5/Source/devices/Tab5Keyboard.hDevices/m5stack-tab5/Source/module.cppPlatforms/platform-esp32/bindings/espressif,esp32-sdmmc.yamlTactility/Include/Tactility/hal/usb/Usb.hTactility/Private/Tactility/hal/usb/UsbTusb.hTactility/Source/app/boot/Boot.cppTactility/Source/hal/usb/Usb.cppTactility/Source/hal/usb/UsbTusb.cpp
🚧 Files skipped from review as they are similar to previous changes (5)
- Platforms/platform-esp32/bindings/espressif,esp32-sdmmc.yaml
- Devices/m5stack-tab5/Source/devices/Tab5Keyboard.h
- Tactility/Source/app/boot/Boot.cpp
- Devices/m5stack-tab5/Source/devices/Tab5Keyboard.cpp
- Devices/m5stack-tab5/Source/module.cpp
Tab5 + Tab5 Keyboard are now "smart" 🧠
Misc changes:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes