Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,23 @@ void setup()
#endif
#endif

#if defined(SENSECAP_INDICATOR)
// The ST7701 panel shares SCK/MOSI/MISO (41/48/47) with the SX1262, and its host is SPI2_HOST,
// which on the S3 is the same peripheral as the Arduino `SPI` object (FSPI == SPI2).
// LovyanGFX bit-bangs the ST7701 init sequence on those pins, and because this variant builds
// with USE_ARDUINO_HAL_GPIO it does so via Arduino pinMode()/digitalWrite(). pinMode() calls
// perimanSetPinBus(.., ESP32_BUS_TYPE_GPIO, ..), whose deinit callback (spiDetachBus_SCK) ends
// up in spiStopBus() and gates the SPI2 clock. RadioLib then spins forever in spiTransferByte()
// waiting on cmd.update, which never clears on a stopped peripheral, and the watchdog fires.
//
// Restart the bus here, after the panel is up and before the radio is touched. Note that
// SPIClass::begin() early-returns when _spi is already non-NULL, so end() first is required.
Comment on lines +1115 to +1124

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Shorten the multi-paragraph explanatory comment.

As per coding guidelines, keep code comments minimal—one or two lines maximum—and do not add multi-paragraph explanatory blocks. Consider moving this detailed background explanation to the pull request description or a separate documentation file.

♻️ Proposed fix to simplify the comment
-    // The ST7701 panel shares SCK/MOSI/MISO (41/48/47) with the SX1262, and its host is SPI2_HOST,
-    // which on the S3 is the same peripheral as the Arduino `SPI` object (FSPI == SPI2).
-    // LovyanGFX bit-bangs the ST7701 init sequence on those pins, and because this variant builds
-    // with USE_ARDUINO_HAL_GPIO it does so via Arduino pinMode()/digitalWrite(). pinMode() calls
-    // perimanSetPinBus(.., ESP32_BUS_TYPE_GPIO, ..), whose deinit callback (spiDetachBus_SCK) ends
-    // up in spiStopBus() and gates the SPI2 clock. RadioLib then spins forever in spiTransferByte()
-    // waiting on cmd.update, which never clears on a stopped peripheral, and the watchdog fires.
-    //
-    // Restart the bus here, after the panel is up and before the radio is touched. Note that
-    // SPIClass::begin() early-returns when _spi is already non-NULL, so end() first is required.
+    // Restart SPI to recover the bus after LovyanGFX bit-bangs the shared LoRa pins.
+    // SPI.end() is required first because begin() early-returns if already initialized.
📝 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
// The ST7701 panel shares SCK/MOSI/MISO (41/48/47) with the SX1262, and its host is SPI2_HOST,
// which on the S3 is the same peripheral as the Arduino `SPI` object (FSPI == SPI2).
// LovyanGFX bit-bangs the ST7701 init sequence on those pins, and because this variant builds
// with USE_ARDUINO_HAL_GPIO it does so via Arduino pinMode()/digitalWrite(). pinMode() calls
// perimanSetPinBus(.., ESP32_BUS_TYPE_GPIO, ..), whose deinit callback (spiDetachBus_SCK) ends
// up in spiStopBus() and gates the SPI2 clock. RadioLib then spins forever in spiTransferByte()
// waiting on cmd.update, which never clears on a stopped peripheral, and the watchdog fires.
//
// Restart the bus here, after the panel is up and before the radio is touched. Note that
// SPIClass::begin() early-returns when _spi is already non-NULL, so end() first is required.
// Restart SPI to recover the bus after LovyanGFX bit-bangs the shared LoRa pins.
// SPI.end() is required first because begin() early-returns if already initialized.
🤖 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/main.cpp` around lines 1115 - 1124, Replace the multi-paragraph comment
above the SPI restart with a concise one- or two-line summary stating that the
panel initialization can stop the shared SPI2 bus and that the bus must be
restarted before using the radio. Keep the existing implementation and ordering
unchanged.

Source: Coding guidelines

SPI.end();
SPI.begin(LORA_SCK, LORA_MISO, LORA_MOSI, -1); // CS is an IO-expander pin, driven by RadioLib
SPI.setFrequency(4000000);
LOG_DEBUG("SPI2 restarted after ST7701 init (SCK=%d, MISO=%d, MOSI=%d)", LORA_SCK, LORA_MISO, LORA_MOSI);
#endif

auto rIf = initLoRa();

lateInitVariant(); // Do board specific init (see extra_variants/README.md for documentation)
Expand Down
Loading