You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "fdcavalcanti (via GitHub)" <gi...@apache.org> on 2025/02/10 16:45:08 UTC
[PR] arch/xtensa: unify WiFi source code for ESP32|S2|S3 (1/2) [nuttx]
fdcavalcanti opened a new pull request, #15804:
URL: https://github.com/apache/nuttx/pull/15804
## Summary
This PR modifies ESP32 and ESP32S3 to use WiFi source code from the common layer, which is already used by ESP32S2.
Overall there's a massive reduction in source code since WiFi support is now centered around the common layer for the Xtensa devices.
## Impact
KConfig prefixes related to radio functionalities have been modified. ESP32_* and ESP32S3_* are replaced with ESPRESSIF_* (CONFIG_ESP32_WIFI -> CONFIG_ESPRESSIF_WIFI). This is applied through arch/ and also board/ (different PR) and may impact users if they are running custom defconfigs.
BLE symbols are altered as well, since they share some common wireless implementations. However, source code for BLE has not been merged into the common layer.
## Testing
Reduces the amount of source code required for WiFi support.
No difference in performance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/xtensa: unify WiFi source code for ESP32|S2|S3 (1/2) [nuttx]
Posted by "fdcavalcanti (via GitHub)" <gi...@apache.org>.
fdcavalcanti closed pull request #15804: arch/xtensa: unify WiFi source code for ESP32|S2|S3 (1/2)
URL: https://github.com/apache/nuttx/pull/15804
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/xtensa: unify WiFi source code for ESP32|S2|S3 (1/2) [nuttx]
Posted by "fdcavalcanti (via GitHub)" <gi...@apache.org>.
fdcavalcanti commented on PR #15804:
URL: https://github.com/apache/nuttx/pull/15804#issuecomment-2648642764
This is complemented by #15805.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/xtensa: unify WiFi source code for ESP32|S2|S3 (1/2) [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #15804:
URL: https://github.com/apache/nuttx/pull/15804#issuecomment-2648642485
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
Yes, this PR appears to mostly meet the NuttX requirements, but some information is missing or could be improved.
**Strengths:**
* The summary clearly explains the "why," "what," and "how" of the change. The mention of code reduction is helpful.
* The impact section identifies key changes and potential user impact due to KConfig prefix modifications. The impact on code size is also noted.
* The testing section confirms basic functionality.
**Weaknesses and Missing Information:**
* **Missing Issue References:** The summary should link to related NuttX or NuttX Apps issues if any exist.
* **Incomplete Impact Assessment:** Several impact fields are not addressed:
* **Build:** While Kconfig changes are mentioned, it's unclear if the build *process* itself changes (e.g., new dependencies, build steps).
* **Hardware:** While ESP32 and ESP32S3 are mentioned, specifying the exact chips and boards tested is crucial. Just saying "ESP32" is not sufficient. Mention specific development boards.
* **Documentation:** Does this change require documentation updates? If so, were they provided in this PR or a separate one?
* **Security:** Even if there's no security impact, explicitly stating "NO" is important.
* **Compatibility:** Address backward and forward compatibility. Will existing applications still work?
* **Insufficient Testing Details:**
* **Build Host(s):** Provide details about the development environment used (OS, CPU architecture, compiler version).
* **Target(s):** Be much more specific. E.g., instead of "arch(sim,RISC-V,ARM..)," specify "Xtensa LX6 (ESP32), Xtensa LX7 (ESP32S3)." List the exact boards and configurations used for testing.
* **Testing Logs:** The placeholder logs need to be replaced with actual logs demonstrating the functionality before and after the change. Show that WiFi works as expected. Quantify the code size reduction.
**Recommendation:**
Before merging, the PR should be updated to address the missing information and provide more specific details about testing and impact. This will ensure the PR meets the NuttX standards and helps reviewers thoroughly assess the changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org