Wifi stay disconnected after manual disconnect #3033
Conversation
Stay disconnected after manual disconnect
WalkthroughThe pull request modifies two functions 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/_Network/NF_ESP32_Wireless.cpp (1)
284-285: Consider improving connection state management.While the current solution using a global flag works, consider these future improvements:
- Encapsulate connection state management in a dedicated class/struct
- Use an enum for connection states instead of boolean flags
- Add state transition validation
This would make the code more maintainable and robust.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
targets/ESP32/_Network/NF_ESP32_Wireless.cpp(1 hunks)
🔇 Additional comments (1)
targets/ESP32/_Network/NF_ESP32_Wireless.cpp (1)
284-285: LGTM! Setting connection flag prevents auto-reconnection.
The addition of NF_ESP32_IsToConnect = false before disconnecting is the correct approach to maintain the disconnected state and prevent automatic reconnection. This change directly addresses the PR objective.
Let's verify the flag usage across the codebase:
✅ Verification successful
Setting NF_ESP32_IsToConnect = false is correctly integrated with the WiFi connection management
The verification shows that NF_ESP32_IsToConnect is consistently used across the codebase:
- It's declared as a global flag in
NF_ESP32_Network.h - Used in
targetHAL_Network.cppto control WiFi connection attempts - In
NF_ESP32_Wireless.cpp, it's properly managed:- Set to
truewhen opening a connection - Set to
falsebefore disconnecting - Reset to
falsein error conditions
- Set to
The change aligns with the existing connection management logic and correctly prevents auto-reconnection after manual disconnect.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all places where NF_ESP32_IsToConnect is used to ensure consistent behavior
rg -A 3 "NF_ESP32_IsToConnect"
Length of output: 2617
josesimoes
added
the
Platform: ESP32
|
Any idea why its getting a build error with 2024-11-06T06:24:29.5803921Z CMake Error at D:/a/1/s/esp-idf/tools/cmake/build.cmake:592 (message): |
|
@AdrianSoundy I do... just spent the last hour trying to figure out this... 🙄 |
Description
If the Wifi connection was disconnected using WifiNetworkHelper.Disconnect() the connection was disconnected and then reconnected automatically as if it was a network error.
This change causes the Wifi to stay disconected.
Motivation and Context
Reported on discord.
https://discord.com/channels/478725473862549535/483428854493478913/1301627595619700776
How Has This Been Tested?
Tested using Wifi test program and monitoring events, access point.
Note : There is also a problem with WifiNetworkHelper which doesn't allow you to connect again after the disconnect.
Types of changes
Checklist
Summary by CodeRabbit
New Features
Bug Fixes