fix: TFT code review fixes — null guards, queue, showText4, struct cleanup

[sjordan0228]

Summary

Fixes from dual-model deep code review (Superpowers + Codex) of the TFT implementation.

• Null queue guards — all 12 queue operations now check for null before access. Prevents Guru Meditation crash if queue allocation fails.
• Queue depth 4 → 8 — increased buffer for burst traffic during state changes. Drops now logged to serial.
• showText4 shows all lines — was discarding lines 1-2 (e.g., "Spool Scanned"). Now combines context into "Spool: Unknown Tag" / "Use app to setup".
• Remove TFTSpoolData — duplicate of DisplaySpoolData with an unused name[48] field. Eliminated 10 lines of field-by-field copying, saves 48 bytes per queued message.
• startTask() checks result — xTaskCreatePinnedToCore return value now checked. Logs failure instead of claiming success.

Review Findings Disposition

See CODE-REVIEW.md (gitignored) for full review tracker with all 15 findings and dispositions.

Test Plan

• Clean compile on esp32s3zero and esp32dev (verified)
• TFT displays boot/ready/spool screens correctly
• Generic tag scan shows "Spool: Unknown Tag" (not just "Unknown Tag")
• Low spool breathing animation still works
• OTA progress display still works

Summary by CodeRabbit

• Bug Fixes

  • Added validation and error logging for display operations, now reports when updates fail due to queue saturation
  • Improved error handling during task initialization with clearer status reporting

• Improvements

  • Increased display message queue capacity for enhanced update stability
  • Enhanced text message formatting with improved whitespace and character handling

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes enhance TFTManager robustness by increasing message queue capacity, adding task creation error handling, implementing defensive queue null-checks with diagnostic logging, consolidating data types from TFTSpoolData to DisplaySpoolData, and improving text formatting with string cleaning logic.

Changes

Cohort / File(s)	Summary
Configuration .gitignore	Added CODE-REVIEW.md to version control ignore list.
TFTManager Robustness & Queue Management src/TFTManager.cpp, src/TFTManager.h	Increased FreeRTOS message queue capacity from 4 to 8 items. Added return value capture and error logging for xTaskCreatePinnedToCore. Implemented defensive null-checks on _messageQueue and added queue-full diagnostic logging across show* and showText* methods. Enhanced showText4 with leading/trailing asterisk and space stripping from context parameter.
Type System Unification src/TFTManager.h, src/TFTManager.cpp	Removed locally-defined TFTSpoolData struct. Replaced all spool-related parameters and message field types with DisplaySpoolData, including showSpoolScanned, renderSpoolScanned, and TFTMessage.spool field.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description includes a detailed summary and changes list, but the required 'How to Test' section with verification steps and the 'Checklist' section are mostly empty or incomplete.	Complete the 'How to Test' section with explicit verification steps and fill in all checklist items (currently marked unchecked) with confirmation of test results.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: null guards, queue improvements, showText4 fixes, and struct cleanup, matching the changeset content.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/tft-code-review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 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/TFTManager.cpp`:
- Around line 698-699: The while loop that advances the pointer "clean"
(initialized from "line1") is written without braces, which can hinder
readability and lead to future bugs; update the loop that checks (*clean == '*'
|| *clean == ' ') to use explicit braces around its body so the intent is clear
and future additions won't be accidentally excluded—locate the initialization
"const char* clean = line1;" and the subsequent while(...) and add braces around
the statement that advances the pointer.

🪄 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: 624298b7-1a18-406b-a872-17bc6c386788

📥 Commits

Reviewing files that changed from the base of the PR and between ab79ade and 1db63bd.

📒 Files selected for processing (3)

• .gitignore
• src/TFTManager.cpp
• src/TFTManager.h

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider adding braces for clarity.

The while loop on line 699 is a single statement without braces. While syntactically correct, braces improve readability and prevent accidental bugs during future modifications.

Optional style improvement

-        while (*clean == '*' || *clean == ' ') clean++;
+        while (*clean == '*' || *clean == ' ') {
+            clean++;
+        }

📝 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.

Suggested change

	const char* clean = line1;
	while (*clean == '*' || *clean == ' ') clean++;
	const char* clean = line1;
	while (*clean == '*' || *clean == ' ') {
	clean++;
	}

🧰 Tools

🪛 Clang (14.0.6)

[warning] 699-699: 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/TFTManager.cpp` around lines 698 - 699, The while loop that advances the
pointer "clean" (initialized from "line1") is written without braces, which can
hinder readability and lead to future bugs; update the loop that checks (*clean
== '*' || *clean == ' ') to use explicit braces around its body so the intent is
clear and future additions won't be accidentally excluded—locate the
initialization "const char* clean = line1;" and the subsequent while(...) and
add braces around the statement that advances the pointer.