Add heltec_rcc6 board#11041
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Heltec RCC6 ESP32-C6 board support, including build metadata, pin and feature definitions, hardware identification, Arduino_GFX NV3001B display handling, and runtime screen detection. ChangesHeltec RCC6 support
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant PlatformIO
participant TFTDisplay
participant Arduino_NV3001B
participant NodeDB
participant NV3001B
PlatformIO->>TFTDisplay: Build with HELTEC_RCC6
TFTDisplay->>Arduino_NV3001B: Initialize over software SPI
TFTDisplay->>Arduino_NV3001B: Render and control display
NodeDB->>NV3001B: Read display identifier
NV3001B-->>NodeDB: Return identifier
NodeDB->>NodeDB: Set screen availability
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/graphics/TFTDisplay.cpp (1)
1626-1635: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReturn failure when the NV3001B cannot initialize.
A failed
begin()is only logged; execution continues through rotation, drawing, and ultimately returnstrue, falsely reporting a connected display.Proposed fix
- if (beginStatus) - LOG_DEBUG("TFT Success!"); - else + if (!beginStatus) { LOG_ERROR("TFT Fail!"); + return false; + } + LOG_DEBUG("TFT Success!");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graphics/TFTDisplay.cpp` around lines 1626 - 1635, Update the TFT initialization flow around beginStatus so a failed tft->begin() immediately returns false from the enclosing display initialization function after logging the failure. Preserve the existing success log and subsequent rotation/drawing logic only for successful initialization, ensuring the function cannot report true when the display fails to initialize.
🧹 Nitpick comments (1)
src/mesh/NodeDB.cpp (1)
254-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
get_nv3001b_idtogetNv3001bId.As per coding guidelines, “Use
camelCasefor functions and members.”Proposed rename
-uint32_t get_nv3001b_id(uint8_t cs, uint8_t sck, uint8_t mosi, uint8_t dc, uint8_t rst, uint8_t en, uint8_t bl) +uint32_t getNv3001bId(uint8_t cs, uint8_t sck, uint8_t mosi, uint8_t dc, uint8_t rst, uint8_t en, uint8_t bl) ... - uint32_t nv3001b_id = get_nv3001b_id(TFT_CS, TFT_SCL, TFT_SDA, TFT_RS, TFT_RST, TFT_EN, TFT_BL); + uint32_t nv3001b_id = getNv3001bId(TFT_CS, TFT_SCL, TFT_SDA, TFT_RS, TFT_RST, TFT_EN, TFT_BL);Also applies to: 1140-1140
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mesh/NodeDB.cpp` at line 254, Rename the function get_nv3001b_id to getNv3001bId and update every declaration, definition, and call site, including the additional occurrence, while preserving its signature and behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mesh/NodeDB.cpp`:
- Around line 1139-1144: Update the HELTEC_RCC6 display initialization flow so
the false value assigned to hasScreen after the NV3001B ID check prevents
TFTDisplay::connect() from starting when no panel is detected. Thread hasScreen
into the TFT startup path or gate the module connection, while preserving normal
initialization when the expected display is present.
---
Outside diff comments:
In `@src/graphics/TFTDisplay.cpp`:
- Around line 1626-1635: Update the TFT initialization flow around beginStatus
so a failed tft->begin() immediately returns false from the enclosing display
initialization function after logging the failure. Preserve the existing success
log and subsequent rotation/drawing logic only for successful initialization,
ensuring the function cannot report true when the display fails to initialize.
---
Nitpick comments:
In `@src/mesh/NodeDB.cpp`:
- Line 254: Rename the function get_nv3001b_id to getNv3001bId and update every
declaration, definition, and call site, including the additional occurrence,
while preserving its signature and behavior.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 0debf068-32e4-4aca-8aab-623f5e419b07
📒 Files selected for processing (7)
boards/heltec-rcc6.jsonsrc/graphics/TFTDisplay.cppsrc/mesh/NodeDB.cppsrc/platform/esp32/architecture.hvariants/esp32c6/heltec_rcc6/pins_arduino.hvariants/esp32c6/heltec_rcc6/platformio.inivariants/esp32c6/heltec_rcc6/variant.h
| #ifdef HELTEC_RCC6 | ||
| uint32_t nv3001b_id = get_nv3001b_id(TFT_CS, TFT_SCL, TFT_SDA, TFT_RS, TFT_RST, TFT_EN, TFT_BL); | ||
| if (nv3001b_id != NV3001B_EXPECTED_ID) { | ||
| hasScreen = false; | ||
| } | ||
| #endif // HELTEC_RCC6 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Propagate the absent-display state into TFT startup. hasScreen only affects Bluetooth pairing mode here; RCC6 still builds with HAS_SCREEN, so TFTDisplay::connect() can still initialize a panel that was just detected as absent and powered off. Thread that state into display startup or gate the module connection.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mesh/NodeDB.cpp` around lines 1139 - 1144, Update the HELTEC_RCC6 display
initialization flow so the false value assigned to hasScreen after the NV3001B
ID check prevents TFTDisplay::connect() from starting when no panel is detected.
Thread hasScreen into the TFT startup path or gate the module connection, while
preserving normal initialization when the expected display is present.
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
1 participant
Summary by CodeRabbit