-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Claude/esp32 liliygo adaptation i44 uq #2030
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Claude/esp32 liliygo adaptation i44 uq #2030
Conversation
Add new board configuration for the LilyGo T-Deck Pro with E-Paper display (GDEQ031T10, 3.1" 320x240 touchscreen) and 4G LTE modem (A7682E). Hardware features: - ESP32-S3FN16R8 (16MB Flash, 8MB PSRAM) - 3.1" E-Paper display with CST328 capacitive touch - TCA8418 QWERTY keyboard controller - SX1262 LoRa radio - MIA-M10Q GPS module - A7682E 4G LTE modem (optional) - 1400mAh battery with BQ25896/BQ27220 management Known limitations: - E-Paper display has slow refresh rate (1-2s) - Temporary ST7789 driver config (needs GxEPD2 integration) - TCA8418 keyboard driver needs full implementation - Battery management requires BQ driver implementation This is an experimental configuration. Full E-Paper support requires integrating the GxEPD2 library and optimizing the UI for slow refresh rates. See boards/lilygo-t-deck-pro-epaper/README.md for details.
Add complete keyboard support for the LilyGo T-Deck Pro E-Paper board using the Adafruit TCA8418 library. Features implemented: - Full QWERTY keyboard layout (4 rows × 10 columns, 40 keys) - Three keyboard layers: Normal, Shift, and Fn - Modifier key support: Fn, Shift, Caps Lock - Special key combinations (Fn + Shift = Caps Lock toggle) - Key event handling with proper press/release detection - Keyboard LED feedback on key presses - Debouncing for stable input - Integration with Bruce's KeyStroke API Keyboard layout: - Normal: Standard QWERTY letters and basic punctuation - Shift: Uppercase letters and symbols (!, @, #, etc.) - Fn: Numbers (0-9) and extended symbols ([], (), etc.) Technical changes: - Added Adafruit_TCA8418 library dependency - Implemented getKeyChar() for multi-layer key mapping - Implemented handleSpecialKeys() for modifier key logic - Updated InputHandler() to prioritize keyboard over touch - Added keyboard initialization in _setup_gpio() - Documented complete keyboard layout in README The keyboard is now fully functional and ready for use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds experimental support for the LilyGo T-Deck Pro E-Paper board to the Bruce firmware. The board features an ESP32-S3 MCU, a 3.1" E-Paper display, capacitive touch, QWERTY keyboard controller, LoRa radio, GPS, and various peripherals.
Changes:
- New board configuration for LilyGo T-Deck Pro E-Paper with comprehensive hardware support
- Implementation of TCA8418 QWERTY keyboard controller with Fn, Shift, and Caps Lock support
- Touch screen support using CST328 controller
- Pin definitions and GPIO setup for E-Paper display, LoRa SX1262, GPS, and peripherals
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| boards/lilygo-t-deck-pro-epaper/pins_arduino.h | Arduino pin definitions and USB identifiers for the board |
| boards/lilygo-t-deck-pro-epaper/lilygo-t-deck-pro-epaper.ini | PlatformIO build configuration with feature flags and peripheral pin assignments |
| boards/lilygo-t-deck-pro-epaper/interface.cpp | Board-specific hardware initialization and input handling (keyboard, touch, power management) |
| boards/lilygo-t-deck-pro-epaper/README.md | Comprehensive documentation of hardware specs, pin configuration, keyboard layout, and known limitations |
| boards/_boards_json/lilygo-t-deck-pro-epaper.json | PlatformIO board definition with build parameters and hardware capabilities |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| // Clear keystroke if no keys in buffer | ||
| if (millis() - lastKeyTime > 100) { | ||
| KeyStroke.pressed = false; | ||
| } |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable 'lastKeyTime' is declared but appears to only be used for writing (lines 310, 346) and checking elapsed time (line 352). However, there's a potential logic issue: if the keyboard buffer is empty, the code sets KeyStroke.pressed to false after 100ms. This could incorrectly clear a keystroke that's still being processed if there's any delay in the keyboard event queue, leading to missed key presses.
| } else { | |
| // Clear keystroke if no keys in buffer | |
| if (millis() - lastKeyTime > 100) { | |
| KeyStroke.pressed = false; | |
| } |
| if (keyChar == KEY_FN) { | ||
| KeyStroke.fn = true; | ||
| } | ||
|
|
||
| KeyStroke.word.push_back(keyChar); |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The KEY_FN special character is being added to both the hid_keys vector and the word vector when the Fn key is pressed. However, based on the handleSpecialKeys function (lines 144-146), KEY_FN is a modifier key that should only update the fn_key_pressed state, not be added as an actual character to the keystroke buffers. This will result in invalid characters being added to user input.
| // LiPo voltage range: 3.3V (0%) to 4.2V (100%) | ||
| int percent = ((voltage - 3.3) / (4.2 - 3.3)) * 100; | ||
|
|
||
| return (percent < 0) ? 1 : (percent > 100) ? 100 : percent; |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The battery percentage calculation has a potential division issue. When the calculated percent is less than 0, it returns 1 instead of 0. This means the battery indicator will never show 0%, making it impossible to accurately represent a completely depleted battery. The minimum should be 0, not 1.
| return (percent < 0) ? 1 : (percent > 100) ? 100 : percent; | |
| return (percent < 0) ? 0 : (percent > 100) ? 100 : percent; |
| case KEY_SHIFT: | ||
| shift_key_pressed = pressed; | ||
| // Fn + Shift = Caps Lock toggle | ||
| if (fn_key_pressed && shift_key_pressed) { | ||
| caps_lock = !caps_lock; | ||
| } | ||
| return 1; |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Caps Lock toggle logic has a bug. On line 151, caps_lock is toggled when both fn_key_pressed and shift_key_pressed are true. However, this check happens inside the KEY_SHIFT case when a key is pressed. The issue is that shift_key_pressed is set to true on line 149 BEFORE the caps lock check on line 151. This means if Fn is already held and Shift is pressed, caps_lock will toggle. But if Shift is already held and Fn is pressed, the caps_lock won't toggle because we're in the KEY_FN case (line 144-146), not the KEY_SHIFT case. This creates asymmetric behavior depending on which key is pressed first.
| -DGPS_SERIAL_TX=16 | ||
| -DGPS_SERIAL_RX=17 |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pin definition inconsistency: In pins_arduino.h, TX is defined as GPIO 16 and RX as GPIO 17. However, in the .ini file, SERIAL_TX is defined as 16 and SERIAL_RX as 17, which matches. But then GPS_SERIAL_TX and GPS_SERIAL_RX are also set to the same pins (16 and 17). This means the UART pins are being shared between generic serial and GPS, which could cause conflicts if both are used simultaneously. This should be clarified or separate UART pins should be assigned if available.
| -DGPS_SERIAL_TX=16 | |
| -DGPS_SERIAL_RX=17 | |
| -DGPS_SERIAL_TX=SERIAL_TX | |
| -DGPS_SERIAL_RX=SERIAL_RX |
| // E-Paper has no backlight, so no PWM setup needed | ||
| // Turn on keyboard LED for status indication |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment indicates "E-Paper has no backlight, so no PWM setup needed", but then the code proceeds to turn the keyboard LED on and off. This is misleading - the function is actually setting up the keyboard LED for status indication, not doing nothing. The comment should be updated to reflect that this function initializes the keyboard LED instead.
| // E-Paper has no backlight, so no PWM setup needed | |
| // Turn on keyboard LED for status indication | |
| // E-Paper has no backlight (no display PWM); initialize keyboard LED for status indication | |
| // Blink keyboard LED to indicate that GPIO initialization has completed |
| ** Description: Set brightness (controls keyboard LED for E-Paper) | ||
| ***************************************************************************************/ | ||
| void _setBrightness(uint8_t brightval) { | ||
| // E-Paper displays have no backlight | ||
| // Use keyboard LED to indicate brightness setting | ||
| if (brightval > 0) { | ||
| analogWrite(BOARD_KEYBOARD_LED, map(brightval, 0, 100, 0, 255)); | ||
| } else { | ||
| digitalWrite(BOARD_KEYBOARD_LED, LOW); | ||
| } |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _setBrightness function is documented to "Set brightness (controls keyboard LED for E-Paper)", but controlling the keyboard LED brightness as a proxy for display brightness is confusing and misleading to users. The keyboard LED should provide keyboard feedback, not represent display brightness. This function should either do nothing for E-Paper displays or be clearly documented that brightness control is not supported rather than repurposing the keyboard LED.
| ** Description: Set brightness (controls keyboard LED for E-Paper) | |
| ***************************************************************************************/ | |
| void _setBrightness(uint8_t brightval) { | |
| // E-Paper displays have no backlight | |
| // Use keyboard LED to indicate brightness setting | |
| if (brightval > 0) { | |
| analogWrite(BOARD_KEYBOARD_LED, map(brightval, 0, 100, 0, 255)); | |
| } else { | |
| digitalWrite(BOARD_KEYBOARD_LED, LOW); | |
| } | |
| ** Description: Set brightness (not supported on E-Paper; no-op) | |
| ***************************************************************************************/ | |
| void _setBrightness(uint8_t brightval) { | |
| // E-Paper displays have no backlight, so brightness control is not supported. | |
| // This function is intentionally a no-op on this board variant. | |
| (void)brightval; // suppress unused parameter warning |
| int keyEvent = keyboard->getEvent(); | ||
| bool pressed = keyEvent & 0x80; // Bit 7: 1=pressed, 0=released | ||
| uint8_t keyValue = keyEvent & 0x7F; // Bits 0-6: key position | ||
| keyValue--; // Adjust for 0-based indexing |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keyValue is decremented by 1 for "0-based indexing" (line 305), but this assumes the TCA8418 returns 1-based key positions. This should be verified against the TCA8418 datasheet. If the hardware already returns 0-based indices, this decrement will cause all key positions to be off by one, resulting in incorrect key mapping.
| ;SD Card Setup pins - shares SPI with E-Paper | ||
| -DSDCARD_CS=48 | ||
| -DSDCARD_SCK=36 | ||
| -DSDCARD_MISO=-1 | ||
| -DSDCARD_MOSI=33 |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MISO pin is defined as -1 (not connected) for both the E-Paper display and SD Card. However, SD cards require bidirectional SPI communication and MUST have a MISO pin to read data from the card. Without MISO, SD card read operations will fail. Please verify the hardware schematic and add the correct MISO pin for SD card functionality.
| lib_deps = | ||
| ${env.lib_deps} | ||
| lewisxhe/SensorLib | ||
| adafruit/Adafruit TCA8418 @ ^1.0.2 ; Keyboard controller |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lib_deps section adds an external third-party dependency lewisxhe/SensorLib without any version pinning, so builds will always pull the latest mutable version of this library. If that upstream library is compromised or a malicious version is published, your firmware build could silently include attacker-controlled code. To reduce this supply chain risk, pin this dependency to a specific immutable version (or commit) or vendor the code locally so that builds are reproducible and not dependent on a moving upstream target.
Proposed Changes
Types of Changes
Verification
Testing
Linked Issues
User-Facing Change
Further Comments