feat: replace TFT bitmap icons with colored text labels (#66)#73
Conversation
📝 WalkthroughWalkthroughThe changes implement text-based tag type labels in the TFT display to replace bitmap icons, and add BambuTag support to the tag type mapping system. Bitmap icon data is removed and the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ApplicationManager.cpp`:
- Around line 356-360: The single-line if/else-if chain setting spool.tagType
based on s.tag_format (checks like strcmp(s.tag_format, "OpenPrintTag") etc.)
needs braces added around each branch to avoid edit-time logic slips; update the
block that assigns spool.tagType so each if and else if and the final else use {
... } around their assignments (e.g., for the strcmp(...) == 0 branches and the
final else) while keeping the same comparisons and resulting spool.tagType
values.
In `@src/TFTManager.cpp`:
- Around line 536-569: The label is being drawn with a hardcoded x + 22 which
anchors it into the old icon slot and can clip longer labels; in
drawTagIcon(uint8_t tagType, int x, int y) replace the magic 22 with a true
right-edge x coordinate (e.g. x + TAG_ICON_WIDTH - PADDING) and keep TR_DATUM so
the text is right-aligned; define or use a constant like TAG_ICON_WIDTH (or
compute the icon box width) and a small PADDING, then call
_sprite.drawString(label, x + TAG_ICON_WIDTH - PADDING, y) so labels like
"Bambu" won't be truncated (refer to drawTagIcon, _sprite.drawString, TR_DATUM,
label).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1531258-ebbb-4658-a9f3-80af0b7445a4
📒 Files selected for processing (2)
src/ApplicationManager.cppsrc/TFTManager.cpp
| if (strcmp(s.tag_format, "OpenPrintTag") == 0) spool.tagType = 1; | ||
| else if (strcmp(s.tag_format, "TigerTag") == 0) spool.tagType = 2; | ||
| else if (strcmp(s.tag_format, "OpenTag3D") == 0) spool.tagType = 3; | ||
| else if (strcmp(s.tag_format, "BambuTag") == 0) spool.tagType = 4; | ||
| else spool.tagType = 0; |
There was a problem hiding this comment.
Add braces for the tag-format mapping chain.
Line 359 is in a single-line if/else if chain, and this is already flagged by static analysis. Please wrap each branch in braces to prevent future edit-time logic slips.
Suggested fix
- if (strcmp(s.tag_format, "OpenPrintTag") == 0) spool.tagType = 1;
- else if (strcmp(s.tag_format, "TigerTag") == 0) spool.tagType = 2;
- else if (strcmp(s.tag_format, "OpenTag3D") == 0) spool.tagType = 3;
- else if (strcmp(s.tag_format, "BambuTag") == 0) spool.tagType = 4;
- else spool.tagType = 0;
+ if (strcmp(s.tag_format, "OpenPrintTag") == 0) {
+ spool.tagType = 1;
+ } else if (strcmp(s.tag_format, "TigerTag") == 0) {
+ spool.tagType = 2;
+ } else if (strcmp(s.tag_format, "OpenTag3D") == 0) {
+ spool.tagType = 3;
+ } else if (strcmp(s.tag_format, "BambuTag") == 0) {
+ spool.tagType = 4;
+ } else {
+ spool.tagType = 0;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (strcmp(s.tag_format, "OpenPrintTag") == 0) spool.tagType = 1; | |
| else if (strcmp(s.tag_format, "TigerTag") == 0) spool.tagType = 2; | |
| else if (strcmp(s.tag_format, "OpenTag3D") == 0) spool.tagType = 3; | |
| else if (strcmp(s.tag_format, "BambuTag") == 0) spool.tagType = 4; | |
| else spool.tagType = 0; | |
| if (strcmp(s.tag_format, "OpenPrintTag") == 0) { | |
| spool.tagType = 1; | |
| } else if (strcmp(s.tag_format, "TigerTag") == 0) { | |
| spool.tagType = 2; | |
| } else if (strcmp(s.tag_format, "OpenTag3D") == 0) { | |
| spool.tagType = 3; | |
| } else if (strcmp(s.tag_format, "BambuTag") == 0) { | |
| spool.tagType = 4; | |
| } else { | |
| spool.tagType = 0; | |
| } |
🧰 Tools
🪛 Clang (14.0.6)
[warning] 356-356: statement should be inside braces
(readability-braces-around-statements)
[warning] 357-357: statement should be inside braces
(readability-braces-around-statements)
[warning] 358-358: statement should be inside braces
(readability-braces-around-statements)
[warning] 359-359: statement should be inside braces
(readability-braces-around-statements)
[warning] 360-360: statement should be inside braces
(readability-braces-around-statements)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ApplicationManager.cpp` around lines 356 - 360, The single-line
if/else-if chain setting spool.tagType based on s.tag_format (checks like
strcmp(s.tag_format, "OpenPrintTag") etc.) needs braces added around each branch
to avoid edit-time logic slips; update the block that assigns spool.tagType so
each if and else if and the final else use { ... } around their assignments
(e.g., for the strcmp(...) == 0 branches and the final else) while keeping the
same comparisons and resulting spool.tagType values.
| void TFTManager::drawTagIcon(uint8_t tagType, int x, int y) { | ||
| const uint8_t (*icon)[32] = nullptr; | ||
| uint32_t fgColor = COLOR_ACCENT; | ||
| const char* label = nullptr; | ||
| uint32_t color = COLOR_SUBTEXT; | ||
|
|
||
| switch (tagType) { | ||
| case TAG_TYPE_OPENPRINTTAG: | ||
| icon = ICON_OPENPRINTTAG; | ||
| fgColor = 0x00BFFF; | ||
| label = "OPT"; | ||
| color = 0x4FC3F7; | ||
| break; | ||
| case TAG_TYPE_TIGERTAG: | ||
| icon = ICON_TIGERTAG; | ||
| fgColor = 0xFF8C00; | ||
| label = "TT"; | ||
| color = 0xFF9800; | ||
| break; | ||
| case TAG_TYPE_OPENTAG3D: | ||
| icon = ICON_OPENTAG3D; | ||
| fgColor = 0x00CC66; | ||
| label = "OT3D"; | ||
| color = 0x4CAF50; | ||
| break; | ||
| case TAG_TYPE_BAMBU: | ||
| icon = ICON_BAMBU; | ||
| fgColor = 0x1DB954; | ||
| label = "Bambu"; | ||
| color = 0x1DB954; | ||
| break; | ||
| case TAG_TYPE_NFC_PLAIN: | ||
| default: | ||
| icon = ICON_NFC_PLAIN; | ||
| fgColor = COLOR_SUBTEXT; | ||
| label = "NFC+"; | ||
| color = 0x00BCD4; | ||
| break; | ||
| default: | ||
| return; | ||
| } | ||
|
|
||
| if (!icon) return; | ||
|
|
||
| // Draw 32x32 bitmap from PROGMEM — scale to 22x22 by skipping every ~3rd pixel | ||
| for (int row = 0; row < 22; row++) { | ||
| int srcRow = (row * 32) / 22; | ||
| for (int col = 0; col < 22; col++) { | ||
| int srcCol = (col * 32) / 22; | ||
| uint8_t pixel = pgm_read_byte(&icon[srcRow][srcCol]); | ||
| if (pixel) { | ||
| _sprite.drawPixel(x + col, y + row, fgColor); | ||
| } | ||
| } | ||
| } | ||
| _sprite.setTextColor(color); | ||
| _sprite.setTextSize(1); | ||
| _sprite.setTextDatum(TR_DATUM); | ||
| _sprite.drawString(label, x + 22, y); | ||
| } |
There was a problem hiding this comment.
Use a true right-edge anchor; x + 22 keeps label in the old icon slot and can clip longer text.
At Line 568, anchoring to x + 22 assumes the previous 22px icon box. With current call sites, this does not place labels top-right and can truncate longer labels like "Bambu".
Suggested fix
--- a/src/TFTManager.cpp
+++ b/src/TFTManager.cpp
@@
- // Tag type icon top-left in header
- drawTagIcon(spool.tagType, 4, 2);
+ // Tag type label top-right in header
+ drawTagIcon(spool.tagType, W - 4, 2);
@@
- _sprite.drawString(label, x + 22, y);
+ _sprite.drawString(label, x, y);🧰 Tools
🪛 Clang (14.0.6)
[warning] 536-536: method 'drawTagIcon' can be made static
(readability-convert-member-functions-to-static)
[warning] 536-536: 3 adjacent parameters of 'drawTagIcon' of similar type ('int') are easily swapped by mistake
(bugprone-easily-swappable-parameters)
[note] 536-536: the first parameter in the range is 'tagType'
(clang)
[note] 536-536: the last parameter in the range is 'y'
(clang)
[warning] 536-536: parameter name 'x' is too short, expected at least 3 characters
(readability-identifier-length)
[warning] 536-536: parameter name 'y' is too short, expected at least 3 characters
(readability-identifier-length)
[warning] 538-538: variable 'color' is not initialized
(cppcoreguidelines-init-variables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/TFTManager.cpp` around lines 536 - 569, The label is being drawn with a
hardcoded x + 22 which anchors it into the old icon slot and can clip longer
labels; in drawTagIcon(uint8_t tagType, int x, int y) replace the magic 22 with
a true right-edge x coordinate (e.g. x + TAG_ICON_WIDTH - PADDING) and keep
TR_DATUM so the text is right-aligned; define or use a constant like
TAG_ICON_WIDTH (or compute the icon box width) and a small PADDING, then call
_sprite.drawString(label, x + TAG_ICON_WIDTH - PADDING, y) so labels like
"Bambu" won't be truncated (refer to drawTagIcon, _sprite.drawString, TR_DATUM,
label).
1 participant
Summary
Labels
Closes #66
Test Plan
Summary by CodeRabbit