Skip to content

Conversation

@DedeHai
Copy link
Collaborator

@DedeHai DedeHai commented Nov 26, 2025

  • removed extra flash section in board definition that was causing the FS to be erased upon upload
  • renamed the file so it will never use the default board definition in the future

@softhack007 should we also remove the

      "ldscript": "esp32s3_out.ld",
      "partitions": "partitions-8MB-tinyuf2.csv"

from the files build section? Reasoning: if a user's env does not specify partitions, it will still compile but with wrong partitioning.

Summary by CodeRabbit

  • Chores
    • Renamed the board profile to indicate WLED support.
    • Updated partition configuration for the board.
  • Bug Fixes
    • Removed an extra flash image specification to prevent unintended filesystem erase during uploads.

✏️ Tip: You can customize this high-level summary in your review settings.

@DedeHai DedeHai requested a review from softhack007 November 26, 2025 05:57
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 26, 2025

Walkthrough

Updated the Adafruit MatrixPortal ESP32-S3 board JSON to change partitions, rename the board to a WLED-specific variant, and remove the upload.arduino.flash_extra_images entry; updated the PlatformIO sample to use the new board ID.

Changes

Cohort / File(s) Summary
Board JSON update
boards/adafruit_matrixportal_esp32s3_wled.json
Changed partitions from partitions-8MB-tinyuf2.csv to default_8MB.csv; renamed board to "Adafruit MatrixPortal ESP32-S3 for WLED"; removed upload.arduino.flash_extra_images (tinyuf2.bin at 0x410000).
PlatformIO sample update
platformio_override.sample.ini
Switched env adafruit_matrixportal_esp32s3 board value to adafruit_matrixportal_esp32s3_wled with comment noting it avoids FS erase on upload; other env settings unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Scope: small, consistent configuration edits across two files.
  • Review focus:
    • Verify partition filename change is compatible with build/platform expectations.
    • Confirm removal of flash_extra_images aligns with intended upload behavior and tooling.
    • Ensure PlatformIO env board ID matches the updated JSON and any CI/build scripts.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main change: removing an extra flash section from the Adafruit MatrixPortal ESP32-S3 board definition to fix filesystem erasure on upload.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 507f799 and fecbff9.

📒 Files selected for processing (1)
  • boards/adafruit_matrixportal_esp32s3_wled.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • boards/adafruit_matrixportal_esp32s3_wled.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: wled_build / Build Environments (usermods)
  • GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
  • GitHub Check: wled_build / Build Environments (esp32c3dev)
  • GitHub Check: wled_build / Build Environments (esp32dev)
  • GitHub Check: wled_build / Build Environments (lolin_s2_mini)
  • GitHub Check: wled_build / Build Environments (esp32S3_wroom2)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
  • GitHub Check: wled_build / Build Environments (esp32_wrover)

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

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

@softhack007
Copy link
Member

softhack007 commented Nov 26, 2025

@DedeHai good idea, I was also thinking maybe the "extra image" might be harmful because there is no space reserved for this in the non-UF2 partitions.csv.

should we also remove the
"ldscript": "esp32s3_out.ld",
"partitions": "partitions-8MB-tinyuf2.csv"
from the files build section?

Maybe we just change it to the "normal 8MB arduino-esp32 partition" file - I would better not remove this part completely. All board manifests have a partitions entry, and usually they are too small or they use FAT32 so wled will not work any way when users forget to set build.partitions.

even the "default_partitions" set in [env] (or was is another section? Not sure atm) will be better than the default used in most board.json manifest files.

Copy link
Member

@softhack007 softhack007 left a comment

Choose a reason for hiding this comment

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

just change the "partitions", everything else looks good for me.

plus minor name adjustment (adding "for WLED")
@softhack007 softhack007 self-requested a review November 30, 2025 13:57
Copy link
Member

@softhack007 softhack007 left a comment

Choose a reason for hiding this comment

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

@DedeHai all done, ready to merge.

@DedeHai
Copy link
Collaborator Author

DedeHai commented Nov 30, 2025

perfect, thanks. I have been busy working on something else, I will ping you in a moment.

@DedeHai DedeHai merged commit b5f13e4 into wled:main Nov 30, 2025
23 checks passed
@DedeHai DedeHai deleted the S3portal_boarddef_fix branch November 30, 2025 14:38
softhack007 added a commit to MoonModules/WLED-MM that referenced this pull request Nov 30, 2025
…WLED partitions (wled#5113)

* remove extra flash section, rename board file
* matrixportal: change partitions to standard 8MB

plus minor name adjustment (adding "for WLED")

Co-authored-by: Frank <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants