Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esp32: fix esp32 wifi bug caused scan failed #13841

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

GUIDINGLI
Copy link
Contributor

@GUIDINGLI GUIDINGLI commented Oct 6, 2024

Summary

esp32: fix esp32 wifi bug caused scan failed

tools/configure.sh esp32c3-generic:wifi
wapi scan wlan0
has no return result after couple times

rootcause:
mq_timedsend() return failed because the time valid check error. the ts_nsec is bigger than 1000000000.
esp_update_time() hasn't consider of the ns > 1s after the adding calculation

This PR fix the issue which described by:
#13814

Impact

esp32

Testing

tools/configure.sh esp32c3-generic:wifi

tools/configure.sh esp32c3-generic:wifi
wapi scan wlan0
has no return result after couple times

rootcause:
mq_timedsend() return failed because the time valid check error.
the ts_nsec is bigger than 1000000000.
esp_update_time() hasn't consider of the ns > 1s after the adding
calculation

Signed-off-by: ligd <[email protected]>
@github-actions github-actions bot added Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Arch: xtensa Issues related to the Xtensa architecture Size: S The size of the change in this PR is small labels Oct 6, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 6, 2024

[Experimental Bot, please feedback here]

The PR summary you provided does not meet all of the NuttX requirements. Here's a breakdown:

What's Missing/Needs Improvement:

  • Summary:
    • Insufficient Detail: While it mentions fixing an ESP32 WiFi bug, it lacks specific details about the bug's nature and its impact.
    • Missing References: It mentions a related PR (mqueue: revert refactor of mqueue #13814) but should also include a related NuttX issue if one exists.
  • Impact:
    • Too Vague: Simply stating "esp32" is not descriptive enough. Specify the impact on:
      • Features: Is only the scan feature affected, or are other WiFi functionalities impacted?
      • Users: Will users experience any changes in behavior? If so, how?
      • Hardware: Does this fix apply to all ESP32 variants, or only specific ones?
      • Documentation: Does this fix require any documentation updates?
    • Missing Sections: The PR summary does not address the other impact categories: build process, security, compatibility, etc. Consider if any of these apply.
  • Testing:
    • Insufficient Information:
      • Build Host: Provide details about your development environment: OS, CPU architecture, compiler version.
      • Target: Specify the exact ESP32 board and configuration used for testing.
    • Missing Logs: Include actual testing logs (both before and after the change) demonstrating the issue and its resolution.

How to Improve the PR Summary:

  1. Elaborate on the Bug: Clearly describe the WiFi scan bug, its symptoms, and the root cause (e.g., "The ESP32 WiFi scan functionality consistently fails after multiple attempts due to an error in the esp_update_time() function...").
  2. Specific Impact: Provide detailed information about the impact, including:
    • Affected Features: Specify which WiFi features are impacted (e.g., scan, connection, data transfer).
    • User Experience: Explain if users will notice any changes (e.g., "This fix ensures reliable WiFi scanning, preventing intermittent connection failures for users.").
    • Hardware Scope: State which ESP32 variants are affected (e.g., "This fix applies to all ESP32 boards.").
    • Documentation: Mention if any documentation needs updates (e.g., "No documentation updates are required.").
  3. Complete Impact Assessment: Address all impact categories, even if the answer is "NO." For example:
    • "Impact on build (will build process change)? NO"
    • "Impact on security (any sort of implications)? NO"
  4. Detailed Testing Information:
    • Build Host: "Tested on Ubuntu 22.04 with an Intel Core i7 processor and GCC 11.3.0."
    • Target: "Tested on an ESP32-DevKitC board with the default configuration."
  5. Include Relevant Logs: Provide snippets of testing logs showing the WiFi scan failure before the fix and the successful scan after the fix.

By following these suggestions, you can make your PR summary much more informative and aligned with NuttX requirements.

@GUIDINGLI
Copy link
Contributor Author

@fdcavalcanti

@FelipeMdeO
Copy link
Contributor

Hello @xiaoxiang781216 , I tested using esp32c6 devkit.
This PR solve scan error. Thank you @GUIDINGLI .

@xiaoxiang781216 xiaoxiang781216 merged commit b3727f6 into apache:master Oct 7, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Arch: xtensa Issues related to the Xtensa architecture Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants