-
-
Notifications
You must be signed in to change notification settings - Fork 177
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
Fix issue with Wifi AP stopping debug in VS for IDF 5.x #2954
Conversation
Automated fixes for code style.
@AdrianSoundy there are issues with the code style on the source files. Make sure to follow the project code style. Check the details here on how it works and the tools required to help you with that. |
WalkthroughThe changes in Changes
Sequence Diagram(s)sequenceDiagram
participant UserApp
participant NF_ESP32_Wireless
participant ESP32
UserApp->>NF_ESP32_Wireless: NF_ESP32_InitaliseWifi()
NF_ESP32_Wireless->>ESP32: Initialize wifiStaNetif and wifiAPNetif
ESP32-->>NF_ESP32_Wireless: Initialization success
NF_ESP32_Wireless-->>UserApp: Return initialization status
UserApp->>NF_ESP32_Wireless: NF_ESP32_DeinitWifi()
NF_ESP32_Wireless->>ESP32: Deinitialize wifiStaNetif and wifiAPNetif
ESP32-->>NF_ESP32_Wireless: Deinitialization success
NF_ESP32_Wireless-->>UserApp: Return deinitialization status
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- targets/ESP32/_Network/NF_ESP32_Wireless.cpp (4 hunks)
Additional comments not posted (3)
targets/ESP32/_Network/NF_ESP32_Wireless.cpp (3)
20-21
: Initialization ofwifiStaNetif
andwifiAPNetif
asNULL
.This change ensures that the pointers are properly initialized to
NULL
before use, which is a good practice to avoid undefined behavior.
123-126
: Proper destruction ofesp_netif
instances.This change is crucial for preventing exceptions when
esp_netif
is accessed after a soft reboot, as described in the PR. It ensures that the network interfaces are properly cleaned up, which should help resolve the issues reported.
156-156
: Creation of default WiFi interfaces and setting flags.The creation of default WiFi station and AP interfaces is handled correctly. Additionally, setting the
ESP_NETIF_FLAG_AUTOUP
flag on the AP interface ensures that it is brought up automatically, aligning with the expected behavior.Also applies to: 165-165, 171-172
…0c339-cc4a-4b7b-bd6f-ac908c936cb9 Code style fixes for nanoframework/nf-interpreter PR#2954
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- targets/ESP32/_Network/NF_ESP32_Wireless.cpp (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- targets/ESP32/_Network/NF_ESP32_Wireless.cpp
Quite interesting the output from coderabbitai |
@AdrianSoundy the plan is to have network debugging in the future. That's why we had wifi configured and started at boot and only config changes allowed. Therefore the network interface can not be destroyed and recreated with CLR restart. Otherwise the connection would be lost. Can you please revisit this and make sure the fix doesn't involve restarting the network interface? :) |
It's always has been destroying network on debug restarts. it's just that with idf 5 it expects more to be unitialised for it to work correctly. And that's what this fixes. So this PR doesn't change that and is to fix a problem which has occurred with latest release. For network debugging we would need to automatically connect on startup which isn't case with how most people are using wifi. I feel this is a separate issue to be addressed. We could maintain network if set to auto connect and has ssid and already connected. |
@AdrianSoundy understood. Let's go for this then. Thanks! 😉 |
Description
When you start a debug session the nanoClr is warm restarted.
With latest IDF we now have to make sure the created esp_netif(esp network interfaces) are destroyed when wifi & wifi AP are closed on soft reboot otherwise it fails to create new ones on new start creating an exception when netif is accessed.
Motivation and Context
How Has This Been Tested?
Tested on mini S3 device using WifiAP sample.
Types of changes
Checklist
Summary by CodeRabbit