Adjust memory for ESP32_C6 #3037
Conversation
WalkthroughThe changes introduce a new conditional compilation directive for the ESP32C6 target in the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
targets/ESP32/_nanoCLR/Memory.cpp (1)
Line range hint
1-300: Architecture maintains robust memory management across all targets.The implementation elegantly integrates the ESP32C6 memory requirements while preserving the existing robust memory management strategy. The code continues to:
- Handle both SPIRAM and internal RAM scenarios
- Account for additional memory needs (BLE, Thread)
- Provide comprehensive logging
- Maintain proper error handling
Consider adding a comment explaining the rationale behind the 90KB requirement for ESP32C6 to help future maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
targets/ESP32/_nanoCLR/Memory.cpp(1 hunks)
🧰 Additional context used
📓 Learnings (1)
targets/ESP32/_nanoCLR/Memory.cpp (1)
Learnt from: AdrianSoundy
PR: nanoframework/nf-interpreter#2940
File: targets/ESP32/_nanoCLR/Memory.cpp:140-140
Timestamp: 2024-10-08T15:52:09.445Z
Learning: In the `targets/ESP32/_nanoCLR/Memory.cpp` file, the `largestFreeBlock` value is already logged in the message: `ESP_LOGI(TAG, "Managed heap allocated: %d (from max:%d)", managedHeapSize, largestFreeBlock);`.
🔇 Additional comments (1)
targets/ESP32/_nanoCLR/Memory.cpp (1)
26-27: LGTM! Memory requirements align with provided statistics.
The 90KB minimum free memory requirement for ESP32C6 is well-justified by the memory statistics provided in the PR description. After this change, the system shows:
- Improved managed memory: 82,488 bytes (up from 73,416 bytes)
- Available free memory: 90,512 bytes (aligns with the 90KB requirement)
Let's verify no other memory requirement changes were missed:
✅ Verification successful
Let me check the complete context of these memory definitions to ensure they are properly guarded by the correct target conditionals.
Memory definitions are correctly structured and properly guarded
The memory requirements are well-organized with proper conditional compilation guards:
- ESP32H2: 20KB
- ESP32C6: 90KB (new addition)
- Others: 100KB
The implementation follows the established pattern and the value of 90KB for ESP32C6 sits logically between the existing values, matching the memory statistics provided in the PR description.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other MINIMUM_FREE_MEMORY_FOR_IDF definitions
rg "MINIMUM_FREE_MEMORY_FOR_IDF" --type cpp
Length of output: 390
Script:
#!/bin/bash
# Get the complete context of the memory definitions with surrounding conditionals
rg "MINIMUM_FREE_MEMORY_FOR_IDF" -B2 -A2 --type cpp
Length of output: 1084
2 participants
Description
Adjust memory for ESP32_C6 after IDF 2.2.3 upgrade
Motivation and Context
Reported discord
](https://discord.com/channels/478725473862549535/1304472705302532096/1304472708578410596)
How Has This Been Tested?
locally
Memory before upgrade
01/01/1970 00:12:38- Memory All (startup) Managed:79896 Native total:299588/Free:100712/Largest:67584
After IDF 5.2.3 - Version 1.12.1.1
01/01/1970 00:08:07- Memory All (startup) Managed:73416 Native total:293140/Free:100648/Largest:67584
After this change
01/01/1970 00:02:15- Memory All (startup) Managed:82488 Native total:292164/Free:90512/Largest:58368
Configured for 10K more for managed, let's hope it doesn't cause any other issues.
So there is just a bit more managed heap then before upgrade.
Types of changes
Checklist
Summary by CodeRabbit
New Features
Bug Fixes