Fix 5168 #5181
Fix 5168 #5181
Conversation
Remove the redundant field from the info structure and report what we actually want to know.
Shorten the name, and round up -- getPsramSize() is the usable size, not the total size (the difference is allocator overhead).
WalkthroughReplace boolean PSRAM presence reporting with size-based reporting and use that size to drive runtime allocations and JS reporting; PSRAM size is emitted as Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wled00/data/index.js(1 hunks)wled00/json.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/json.cpp
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/index.js
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/index.js
🧠 Learnings (3)
📓 Common learnings
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.
📚 Learning: 2025-09-18T02:59:33.666Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4928
File: wled00/FX_fcn.cpp:1181-1181
Timestamp: 2025-09-18T02:59:33.666Z
Learning: In WLED, getFreeHeapSize() is a platform-agnostic function defined in fcn_declare.h that provides accurate free heap reporting across ESP32 and ESP8266 platforms. On ESP32, it uses heap_caps_get_free_size(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) instead of ESP.getFreeHeap() because the latter can include other memory types. On ESP8266, it uses ESP.getFreeHeap().
Applied to files:
wled00/json.cpp
📚 Learning: 2025-11-20T00:04:04.829Z
Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
Applied to files:
wled00/data/index.js
🔇 Additional comments (1)
wled00/json.cpp (1)
844-850: LGTM! Clean PSRAM reporting refactor.The changes correctly implement the PR objectives:
- Eliminates redundant
psramPresentby conditionally emitting fields only whenpsramSize > 0- Introduces
psrSzwith a proper rounding-up calculation to report nominal PSRAM size in MB- Maintains backward compatibility with the
psram(free PSRAM) field underBOARD_HAS_PSRAMThe rounding formula
1 + ((psramSize - 1) / (1024U * 1024U))correctly rounds up fractional MB values to account for allocator overhead.
wled00/json.cpp
Outdated
| if (hasPsram) { | ||
| size_t psramSize = ESP.getPsramSize(); // returns 0 if no psram | ||
| if (psramSize > 0) { | ||
| #if defined(BOARD_HAS_PSRAM) |
There was a problem hiding this comment.
can the BOARD_HAS_PSRAM not be used to comment out this whole block? AFAIK this has to be defined for PSRAM to be used, at least that is what I found at one point but don't remember what the current state of code is with all those pending PRs but I fixed that up at one point.
There was a problem hiding this comment.
OK - I've done a more thorough review and I think you're correct about this. If BOARD_HAS_PSRAM is not set, then the Arduino core shouldn't try to initialize that subsystem. I had thought that ESP-IDF might be doing so anyways, but after reviewing the Arduino core code, I don't think that's the case. The upshot is that the code will work as is -- without BOARD_HAS_PSRAM, ESP.getPsramSize() is guaranteed to return 0 -- but we could lift the conditional compilation around the whole block and save a few bytes of code there.
There is a minor issue in the current code that if you try to use a firmware compiled with BOARD_HAS_PSRAM on a board that doesn't actually have any, it'll crash at boot when it fails to allocate the JSON buffer. I also have a small patch for that if it isn't already fixed in your memory PRs. (Hoping to test those soon!)
Also this means that we cannot detect a case where PSRAM is present but unused. If BOARD_HAS_PSRAM was missing, the hardware is never initialized, and the size always returns 0. If BOARD_HAS_PSRAM is defined, we will always try to use it. So the only three cases we can report are:
- BOARD_HAS_PSRAM defined and PSRAM present
- BOARD_HAS_PSRAM defined and PSRAM not present
- BOARD_HAS_PSRAM was not defined
I will update the code to reflect this.
There was a problem hiding this comment.
thanks for confirming, I did dive down the same rabbit hole a few months back and updated some of the code after that but memory is hazy...
There was a problem hiding this comment.
If I'm understanding this correctly, are we saying that it's actually impossible to detect if there is PSRAM without the BOARD_HAS_PSRAM included in the build, so as most users are using our generic esp32dev, they will always return false?
However if we fixed the issues where things crash if you build with that flag on a device without then we could have a generic esp32dev build that handled both?
There was a problem hiding this comment.
If I'm understanding this correctly, are we saying that it's actually impossible to detect if there is PSRAM without the BOARD_HAS_PSRAM included in the build, so as most users are using our generic esp32dev, they will always return false?
That's correct.
However if we fixed the issues where things crash if you build with that flag on a device without then we could have a generic esp32dev build that handled both?
Yes, though with two caveats:
- Non-psram boards would not be able to use the PSRAM IO pins anymore (16 and 17, IIRC): the platform will always try and check for PSRAM on every boot. That's probably not safe in the general case, depending on what's connected; and pin 16 happens to be the default LED pin for the current
esp32devbuild. It would likely break a lot of setups. - The
esp32_wroverbuild with PSRAM support uses the-mfix-esp32-psram-cache-issueflag to enable a workaround for an MMU bug on the v1 chips. This comes at a nontrivial flash space cost (and likely a performance hit too, though I haven't benchmarked it). We'd have to consider how to manage that -- right now we slow all WROVERs down to make sure the firmware is always compatible, but is that an acceptable loss for every ESP32?
I did try both ways on my test setup (sending the esp32dev_wrover build to a baseline ESP32, and vice-versa) and it does boot, with c86d09c -- that's how I generated the "0mb" report during testing.
Given those caveats, though, I'm not confident that a unified build is worth the cost.
There was a problem hiding this comment.
Not necessarily action on your part, I was just surprised by your comment approving, while this comment thread was unresolved
There was a problem hiding this comment.
I was under the impression this was resolved too - what did I miss? The boolean has already been removed in 8ec94af. I was expecting any changes to Hub75 envs to be out of scope for this PR.
There was a problem hiding this comment.
Yes this code removes the need for a separate Boolean in the JSON info in favour of a undefined check on the size to dictate the value of the Boolean send to the usage server, but if it's always going to be undefined for builds that don't have PSRAM enabled in the software, then this is failing to achieve the key objective, which was to understand what % of device has PSRAM hardware that was potentially going unused due to the choice of firmware.
There was a problem hiding this comment.
I think BOARD_HAS_PSRAM is required by Arduino calls to even compile the logic to check
would first need to confirm this but if that is the case, can't check for PSRAM if it is not compiled in.
There was a problem hiding this comment.
if it's always going to be undefined for builds that don't have PSRAM enabled in the software, then this is failing to achieve the key objective, which was to understand what % of device has PSRAM hardware that was potentially going unused due to the choice of firmware.
Ah, ok - sorry I wasn't clear earlier. That is impossible. There is no way to check for PSRAM unless BOARD_HAS_PSRAM was compiled in.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wled00/wled.cpp (1)
398-404: Add debug output for the PSRAM fallback path.The PSRAM allocation path logs buffer information (line 401), but the fallback to onboard RAM (line 403) is silent. Adding a debug message in the
elsebranch would improve troubleshooting and make it clear when the system falls back to onboard RAM.Apply this diff to add debug output for the fallback:
} else { pDoc = new DynamicJsonDocument(JSON_BUFFER_SIZE); // Use onboard RAM instead as a fallback + DEBUG_PRINTF_P(PSTR("JSON buffer size: %ubytes (onboard RAM)\n"), JSON_BUFFER_SIZE); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
wled00/data/index.js(1 hunks)wled00/json.cpp(1 hunks)wled00/wled.cpp(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- wled00/data/index.js
- wled00/json.cpp
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/wled.cpp
🧠 Learnings (12)
📓 Common learnings
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Learnt from: DedeHai
Repo: wled/WLED PR: 4791
File: wled00/FX_fcn.cpp:1187-1191
Timestamp: 2025-08-29T15:38:46.208Z
Learning: In WLED's allocate_buffer() function, BFRALLOC_ENFORCE_PSRAM already includes fallback logic to DRAM if PSRAM is not available, as documented in the comment "use PSRAM if available, otherwise fall back to DRAM". The function also uses validateFreeHeap() for additional safety checks. During setup() when finalizeInit() runs, PSRAM has vast available memory making failures unlikely.
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.
📚 Learning: 2025-08-29T15:38:46.208Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4791
File: wled00/FX_fcn.cpp:1187-1191
Timestamp: 2025-08-29T15:38:46.208Z
Learning: In WLED's allocate_buffer() function, BFRALLOC_ENFORCE_PSRAM already includes fallback logic to DRAM if PSRAM is not available, as documented in the comment "use PSRAM if available, otherwise fall back to DRAM". The function also uses validateFreeHeap() for additional safety checks. During setup() when finalizeInit() runs, PSRAM has vast available memory making failures unlikely.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-11-22T20:33:14.840Z
Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.840Z
Learning: In WLED, board JSON files in the boards/ directory may reference partition files that don't exist or have slightly different names than actual files in tools/. This is intentional because platformio.ini or platformio_override.sample.ini explicitly override the partition file using the board_build.partitions setting, which takes precedence over the board JSON partition reference. This pattern exists in multiple local board definitions and should not be flagged as an issue.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-09-16T18:08:42.848Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-09-15T19:13:56.469Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4791
File: wled00/util.cpp:737-743
Timestamp: 2025-09-15T19:13:56.469Z
Learning: In WLED's util.cpp, the *_realloc_malloc functions (p_realloc_malloc and d_realloc_malloc) are intentionally designed to free the original buffer on realloc failure and allocate a new buffer, implementing a "replace buffer" semantic rather than traditional realloc behavior. This is documented in the function comments and is the intended design by the author DedeHai.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-09-18T02:59:33.666Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4928
File: wled00/FX_fcn.cpp:1181-1181
Timestamp: 2025-09-18T02:59:33.666Z
Learning: In WLED, getFreeHeapSize() is a platform-agnostic function defined in fcn_declare.h that provides accurate free heap reporting across ESP32 and ESP8266 platforms. On ESP32, it uses heap_caps_get_free_size(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) instead of ESP.getFreeHeap() because the latter can include other memory types. On ESP8266, it uses ESP.getFreeHeap().
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-10-10T18:34:06.550Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-08-29T19:51:15.511Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/open_weather_map_source.cpp:13-19
Timestamp: 2025-08-29T19:51:15.511Z
Learning: On ESP8266/ESP32 devices with limited heap, using HTTPS for API calls that return large JSON responses may not be feasible due to the combined memory requirements of TLS handshake and JSON parsing. HTTP may be necessary despite security concerns.
Applied to files:
wled00/wled.cpp
⏰ 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). (15)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp32S3_wroom2)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (esp32dev)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
- GitHub Check: wled_build / Build Environments (esp32dev_debug)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (nodemcuv2_160)
- GitHub Check: wled_build / Build Environments (esp8266_2m_160)
|
Updated logic: BOARD_HAS_PSRAM defined and PSRAM present: reports "psramSize" > 0 I've added the little patch to allow a psram build to work on hardware with no psram connected -- note that it still does not allow those pins to be re-used, though. I think that's probably reasonable given that the platform is going to try probing them for psram on every boot. |
DedeHai
left a comment
DedeHai
left a comment
There was a problem hiding this comment.
I did not run any tests, but code looks good.
3 participants
Correct the AI code from #5168.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.