You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "eren-terzioglu (via GitHub)" <gi...@apache.org> on 2024/11/01 08:01:03 UTC
[PR] Improve nxdiag example for Espressif devices [nuttx-apps]
eren-terzioglu opened a new pull request, #2808:
URL: https://github.com/apache/nuttx-apps/pull/2808
## Summary
- Improve nxdiag example for Espressif devices
## Impact
All ESP32 devices
## Testing
Tested with `esp32c3-generic:nsh` config with these options enabled:
```
CONFIG_SYSTEM_NXDIAG=y
CONFIG_SYSTEM_NXDIAG_ESPRESSIF=y
CONFIG_SYSTEM_NXDIAG_ESPRESSIF_CHIP=y
```
During build process make sure that chip is connected. Output should be like this:
```
nsh> nxdiag --all
Nxdiag Report:
NuttX RTOS info:
Hostname:
Release: 10.4.0
Build: 69e813ce78 Oct 31 2024 10:43:05
Arch: risc-v
Defconfig: esp32c3-generic:nsh-dirty
Host system OS:
...
Espressif specific information:
Bootloader version:
esp32: v5.1-dev-3972-g1559b6309f
esp32s2: v5.1-dev-3972-g1559b6309f
esp32s3: v5.1-dev-3972-g1559b6309f
esp32c2: v5.1-dev-3972-g1559b6309f
esp32c3: v5.1-dev-3972-g1559b6309f
esp32c6: v5.1-dev-3972-g1559b6309f
esp32h2: v5.1-dev-3972-g1559b6309f
Toolchain version:
clang: Not found
gcc: gcc (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0
xtensa-esp32-elf-gcc: xtensa-esp32-elf-gcc (crosstool-NG esp-12.2.0_20230208) 12.2.0
xtensa-esp32s2-elf-gcc: xtensa-esp32s2-elf-gcc (crosstool-NG esp-12.2.0_20230208) 12.2.0
xtensa-esp32s3-elf-gcc: xtensa-esp32s3-elf-gcc (crosstool-NG esp-12.2.0_20230208) 12.2.0
riscv32-esp-elf-gcc: Not found
riscv64-unknown-elf-gcc: Not found
Esptool version: 4.8.dev4
HAL version: sync/release_v5.1.c-nuttx-20230814-1573-gca869dd97e
CHIP ID: ESP32-C3 has no Chip ID. Reading MAC instead.
Flash ID:
Manufacturer: 20
Device: 4016
Security information:
Flags: 0x00000000 (0b0)
Key Purposes: (0, 0, 0, 0, 0, 0, 12)
Chip ID: 5
API Version: 3
Secure Boot: Disabled
Flash Encryption: Disabled
SPI Boot Crypt Count (SPI_BOOT_CRYPT_CNT): 0x0
Flash status: 0x0000
MAC address: ...
nsh>
```
--
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] Improve nxdiag example for Espressif devices [nuttx-apps]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #2808:
URL: https://github.com/apache/nuttx-apps/pull/2808
--
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] Improve nxdiag example for Espressif devices [nuttx-apps]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #2808:
URL: https://github.com/apache/nuttx-apps/pull/2808#issuecomment-2451593624
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
__Fill In The Commit Message:__ This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.
Yes, this PR appears to meet the NuttX requirements, although some sections could be more explicit.
Here's a breakdown and suggestions for improvement:
**Strengths:**
* **Clear Summary:** The summary clearly states the purpose of the change: improving the `nxdiag` example for Espressif devices.
* **Specific Impact:** The impact section identifies the affected devices (all ESP32 devices).
* **Detailed Testing:** The testing section provides specific configuration options, expected output, and a log snippet. This demonstrates functionality and allows for easy verification.
**Areas for Improvement:**
* **Summary (What functional part & How it works):** While stating the *why* (improvement), the summary lacks details on *what* part of `nxdiag` is changed and *how* the improvement is achieved. For example, "The `nxdiag` example is improved by adding the ability to display Espressif-specific information such as bootloader version, toolchain version, flash ID, and security information."
* **Impact (More Explicit "NO" answers):** While mentioning "All ESP32 devices" implies some impact, explicitly state "NO" for categories without impact. For example: "Impact on user: NO", "Impact on build: NO (assuming the required Espressif tools are already installed)", "Impact on documentation: YES (the documentation should be updated to reflect the new `nxdiag` capabilities for Espressif devices)". This removes ambiguity.
* **Impact (Security and Compatibility):** Even if there's no impact, briefly justify the "NO". For example, "Impact on security: NO (This change only reads existing information and does not introduce new vulnerabilities)." and "Impact on compatibility: NO (This change only adds functionality and does not affect existing `nxdiag` features)."
* **Testing (Before Change Logs):** Include logs from *before* the change to demonstrate the improvement. This shows what was missing and how the PR addresses it.
* **Testing (Build Host):** Specify the build host details as requested in the template.
* **Testing (Multiple Targets):** Ideally, test across multiple ESP32 variants (e.g., ESP32, ESP32-S2, ESP32-C3) to ensure broader compatibility.
**Example of an improved Summary and Impact:**
```
## Summary
This change enhances the `nxdiag` example to provide more detailed diagnostic information for Espressif devices. It adds support for displaying Espressif-specific details, including bootloader version, toolchain information, flash and chip IDs, security settings, and MAC address. This is achieved by integrating Espressif's APIs and tools into the `nxdiag` framework. Relates to [NuttX Issue #XXX] (if applicable).
## Impact
* Is new feature added? YES
* Is existing feature changed? YES (The `nxdiag` command now outputs additional information for Espressif devices)
* Impact on user: NO (Users can simply run the `nxdiag` command to access the new information.)
* Impact on build: NO (assuming the required Espressif tools are already installed)
* Impact on hardware: YES (This change specifically targets Espressif ESP32 devices)
* Impact on documentation: YES (The documentation should be updated to describe the new `nxdiag` output for Espressif devices.)
* Impact on security: NO (This change only reads existing information and does not introduce new vulnerabilities).
* Impact on compatibility: NO (This change only adds functionality and does not affect existing `nxdiag` features).
* Anything else to consider: N/A
```
By addressing these points, the PR will be even stronger and easier for reviewers to assess.
--
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] Improve nxdiag example for Espressif devices [nuttx-apps]
Posted by "eren-terzioglu (via GitHub)" <gi...@apache.org>.
eren-terzioglu closed pull request #2808: Improve nxdiag example for Espressif devices
URL: https://github.com/apache/nuttx-apps/pull/2808
--
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