Skip to content

Conversation

@vicliu624
Copy link

@vicliu624 vicliu624 commented Jan 7, 2026

Summary

Enable Chinese display and IME on T-Deck (LCD) and refine IME behavior/UX for T-Deck Pro.

Changes

  • Enable U8g2 text rendering on T-Deck (LCD) and use GB2312-capable font for Chinese display.
  • Add/enable Chinese IME for T-Deck (alongside T-Deck Pro) and keep IME disabled by default until CN mode is selected.
  • Gate IME paging to active candidate sessions; reset/disable IME on exit from freetext.
  • Adjust IME candidate bar position upward to avoid footer overlap.
  • Insert space directly when CN mode is active but IME buffer is empty.
  • Harden UTF‑8 apostrophe normalization with bounds checks.
  • Pin U8g2_for_Adafruit_GFX to an immutable commit SHA (both t-deck / t-deck-pro).

Testing

  • Built on T-Deck (ESP32-S3)
  • Built on T-Deck Pro (ESP32-S3)

Notes

  • IME is only active in CN mode; other modes remain EN/123.
  • IME buffer is cleared and disabled when leaving freetext.

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other (please specify below)

Other:

  • LilyGo T-Deck Pro (ESP32-S3)

- embed pinyin table as PROGMEM data guarded by T_DECK_PRO
- trim dictionary to GB2312-encodable characters to fit flash
- disable IME paths on non?T-Deck Pro targets
- keep candidate matching and selection logic unchanged
@CLAassistant
Copy link

CLAassistant commented Jan 7, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

@vicliu624, Welcome to Meshtastic!

Thanks for opening your first pull request. We really appreciate it.

We discuss work as a team in discord, please join us in the #firmware channel.
There's a big backlog of patches at the moment. If you have time,
please help us with some code review and testing of other PRs!

Welcome to the team 😄

@vicliu624 vicliu624 changed the title Feature unicode cjk for t deck pro Feature T-Deck Pro: add Chinese IME and built-in GB2312 pinyin table Jan 7, 2026
- guard pgmspace include behind T_DECK_PRO
- skip builtin dictionary parsing when IME is disabled
- default IME mode to EN and reset state on freetext entry
- gate CN mode by region and map 123 input to numbers/symbols
- count remaining chars by byte budget in freetext UI
- fix right alignment for mixed ASCII/CJK lines
@fifieldt fifieldt added the enhancement New feature or request label Jan 8, 2026
@fifieldt fifieldt requested review from Xaositek and Copilot January 8, 2026 06:13
Copy link
Contributor

Copilot AI left a 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 a Chinese Input Method Editor (IME) for the T-Deck Pro device with a built-in GB2312-tuned pinyin dictionary. The implementation includes support for rendering CJK text using the U8g2 font library and improves message editor UX with UTF-8-aware cursor navigation and character counting.

Key changes:

  • Embeds a ~42KB pinyin dictionary as PROGMEM data (T_DECK_PRO only)
  • Adds Chinese IME class with candidate selection and pinyin-to-character conversion
  • Integrates U8g2_for_Adafruit_GFX library for CJK font rendering
  • Implements three input modes (CN/EN/123) with mode cycling via Fn/Symbol key
  • Adds UTF-8 aware text navigation and byte-based character counting
  • Fixes text alignment for mixed ASCII/CJK content in message rendering

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
variants/esp32s3/t-deck-pro/platformio.ini Adds U8g2_for_Adafruit_GFX library dependency and USE_U8G2_EINK_TEXT build flag
src/modules/PinyinData.h Embeds GB2312 pinyin dictionary as PROGMEM for T_DECK_PRO builds
src/modules/ChineseIme.h Defines IME interface with candidate management and buffer operations
src/modules/ChineseIme.cpp Implements IME logic including candidate lookup from embedded dictionary
src/modules/CannedMessageModule.h Adds IME state variables and mode enum for message editor integration
src/modules/CannedMessageModule.cpp Integrates IME with freetext editor, adds UTF-8 navigation helpers and rendering
src/input/kbI2cBase.cpp Adds handling for new IME control key events
src/input/TDeckProKeyboard.cpp Generates IME page navigation events on shift key press
src/input/InputBroker.h Defines new input event codes for IME control
src/graphics/draw/MessageRenderer.cpp Updates text rendering to use U8g2 fonts for CJK characters
src/graphics/EInkDisplay2.h Declares U8g2 font adapter interface
src/graphics/EInkDisplay2.cpp Implements U8g2 bridge to OLEDDisplay buffer
src/graphics/Screen.cpp Adds debug logging for UI frame events

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

size_t len = utf8CharLen(c);

// Normalize fancy apostrophe to ASCII
if (c == 0xE2 && (uint8_t)messageBuf[i + 1] == 0x80 && (uint8_t)messageBuf[i + 2] == 0x99) {
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code at line 846 checks messageBuf[i + 1] and messageBuf[i + 2] without verifying that these indices are within bounds. If messageBuf[i] is 0xE2 but the string is truncated (ending at i+1 or i+2), this will read beyond the null terminator, potentially accessing invalid memory. Add bounds checks before accessing messageBuf[i + 1] and messageBuf[i + 2].

Copilot uses AI. Check for mistakes.
while (*p && *p != ' ')
++p;
if (p > start)
appendCandidate(list, String(start).substring(0, p - start));
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line creates a String from a pointer range and then calls substring(0, p - start) on it. However, the String constructor creates a String from the entire null-terminated string at start, not just the characters from start to p. This means if there are characters beyond position p, the substring call is needed, but if the String constructor only reads up to a space or null, the substring might be redundant or incorrect. Consider using String(start, p - start) constructor instead to directly create a string of the correct length without the intermediate substring call.

Suggested change
appendCandidate(list, String(start).substring(0, p - start));
appendCandidate(list, String(start, p - start));

Copilot uses AI. Check for mistakes.
Comment on lines 183 to 186
queueEvent(INPUT_BROKER_MSG_IME_PAGE_NEXT);
} else if (key == modifierLeftShiftKey) {
modifierFlag ^= modifierLeftShift;
queueEvent(INPUT_BROKER_MSG_IME_PAGE_PREV);
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updateModifierFlag function queues IME page navigation events (INPUT_BROKER_MSG_IME_PAGE_NEXT and INPUT_BROKER_MSG_IME_PAGE_PREV) every time the shift keys are pressed, regardless of whether IME is active or enabled. This will cause these events to be sent even when the user is not in freetext mode or when Chinese IME is disabled. Consider adding a conditional check to only queue these events when the IME is active and has candidates to navigate.

Suggested change
queueEvent(INPUT_BROKER_MSG_IME_PAGE_NEXT);
} else if (key == modifierLeftShiftKey) {
modifierFlag ^= modifierLeftShift;
queueEvent(INPUT_BROKER_MSG_IME_PAGE_PREV);
if (InputBroker::instance().isImeActive() && InputBroker::instance().hasImeCandidates()) {
queueEvent(INPUT_BROKER_MSG_IME_PAGE_NEXT);
}
} else if (key == modifierLeftShiftKey) {
modifierFlag ^= modifierLeftShift;
if (InputBroker::instance().isImeActive() && InputBroker::instance().hasImeCandidates()) {
queueEvent(INPUT_BROKER_MSG_IME_PAGE_PREV);
}

Copilot uses AI. Check for mistakes.
runOnce();
return true;
}
}
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When IME buffer is empty and space is pressed while in CN mode, this code doesn't handle the space character and the function continues to line 1291 where it may be treated as a regular printable character in NUM mode. This means that if a user is in CN mode with an empty IME buffer and presses space, it could be transformed by mapNumModeChar instead of being inserted as a space. Consider either inserting a space when IME buffer is empty and space is pressed, or ensuring the logic falls through correctly.

Suggested change
}
}
} else if (event->kbchar == ' ') {
// IME buffer is empty in CN mode: insert a space directly instead of
// falling through to generic printable handling (which may apply NUM mapping).
payload = ' ';
lastTouchMillis = millis();
runOnce();
return true;

Copilot uses AI. Check for mistakes.
Comment on lines 29 to 34
bool enabled =
#if defined(T_DECK_PRO)
true;
#else
false;
#endif
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ChineseIme class defaults to enabled=true for T_DECK_PRO builds. However, according to the PR description, the IME should be disabled by default and only enabled when in CN mode (which is only available when region=CN). This conflicts with the initialization in LaunchFreetextWithDestination where imeMode is set to EN and ime.setEnabled(false) is called. The default should be false to avoid confusion and ensure consistency with the intended behavior.

Suggested change
bool enabled =
#if defined(T_DECK_PRO)
true;
#else
false;
#endif
bool enabled = false;

Copilot uses AI. Check for mistakes.
Comment on lines 23 to 24
# renovate: datasource=github-tags depName=U8g2_for_Adafruit_GFX packageName=olikraus/U8g2_for_Adafruit_GFX
https://github.com/olikraus/U8g2_for_Adafruit_GFX/archive/refs/tags/1.8.0.zip
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new lib_deps entry pulls U8g2_for_Adafruit_GFX directly from a GitHub tag ZIP (.../refs/tags/1.8.0.zip), which is a mutable reference with no integrity verification. If the upstream repository or tag were compromised or force-moved, future builds could silently consume attacker-controlled code and ship compromised firmware. To reduce supply-chain risk, pin this dependency to an immutable commit (or a verified release in the PlatformIO registry) and/or add an integrity check (hash/signature) for the downloaded artifact.

Copilot uses AI. Check for mistakes.
- Gate IME paging to active candidate sessions and reset/disable IME on exit

- Lift IME candidate bar to avoid footer overlap

- Insert space in CN mode when buffer is empty

- Harden UTF-8 apostrophe normalization bounds checks

- Pin U8g2_for_Adafruit_GFX to an immutable commit SHA
@vicliu624 vicliu624 changed the title Feature T-Deck Pro: add Chinese IME and built-in GB2312 pinyin table Feature T-Deck Pro and T-Deck: add Chinese IME and built-in GB2312 pinyin table Jan 8, 2026
Copy link
Contributor

@Xaositek Xaositek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to be cautious of changing the font size in messages themselves. This should be uniform with the text size as the time / node info.

#if defined(USE_U8G2_EINK_TEXT)
if (auto *u8g2 = getU8g2Fonts(display)) {
const int baseline = y + FONT_HEIGHT_SMALL - 1;
u8g2->drawUTF8(x, baseline, text.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In testing this it appears the font or font size has changed? Message fonts are much smaller now.

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request first-contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants