added Enable/Disable WiFi functionality
- Added ability to disable WiFi
- enabling or disabling of WiFi takes effect after restart
- AP can be temporarily started with a Button0 long press
Summary by CodeRabbit
-
New Features
- Added the ability to fully enable or disable WiFi via a new control flag.
- Improved handling for toggling Access Point (AP) and Station (STA) modes through updated logic in the WiFi settings.
-
Bug Fixes
- Centralized and streamlined the shutdown process for the WiFi Access Point, improving reliability when disabling AP mode.
Walkthrough
Adds a centralized WLED::shutdownAP() to stop the DNS server and disconnect SoftAP, introduces a wifiEnabled flag to WiFiOptions, and updates connection and JSON handling to use the new shutdown and to gate behavior when WiFi is disabled. (≤50 words)
Changes
| Cohort / File(s) | Change Summary |
|---|---|
Core connection logicwled00/wled.cpp |
Added WLED::shutdownAP() to centralize AP shutdown. Replaced direct DNS/SoftAP stop calls with shutdownAP() in initConnection() and handleConnection(). Added early return when wifiEnabled is false and prevented AP startup for AP_BEHAVIOR_BUTTON_ONLY. |
Public API / configwled00/wled.h |
Added bool wifiEnabled : 1 to WiFiOptions, extended constructor signature to accept the flag, updated global wifiOpt initializers and added #define wifiEnabled wifiOpt.wifiEnabled. Declared void shutdownAP();. Added a public wifiEnabled global in non-WLED_SAVE_RAM builds. |
JSON state handlingwled00/json.cpp |
Replaced inline AP shutdown code with WLED::instance().shutdownAP() in deserializeState(). Added handling for "on" inside the "wifi" object to toggle STA mode, update wifiEnabled and forceReconnect, and disconnect STA or shutdown AP accordingly. Removed commented-out restart handling. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- wled/WLED#4558 — Modifies initConnection and AP/WiFi shutdown/disconnect timing; likely overlaps with AP shutdown changes.
- wled/WLED#4748 — Alters connection handling in
wled00/wled.cppand introduces or useswifiEnabled-style guards; closely related to this diff.
Suggested reviewers
- blazoncek
[!TIP]
🔌 Remote MCP (Model Context Protocol) integration is now available!
Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.
✨ Finishing Touches
- [ ] 📝 Generate Docstrings
🧪 Generate unit tests
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-
I pushed a fix in commit <commit_id>, please review it. -
Open a follow-up GitHub issue for this discussion.
-
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. - PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:-
@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase. -
@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
-
Support
Need help? Create a ticket on our support page for assistance with any issues or questions.
CodeRabbit Commands (Invoked using PR/Issue comments)
Type @coderabbitai help to get the list of available commands.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Status, Documentation and Community
- Visit our Status Page to check the current availability of CodeRabbit.
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
One thing that I forgot to note...
I do not have an Ethernet board to test the USER_ETHERNET features. I think that I covered that area correctly, however, if someone with a board could test that Ethernet still works correct, I would greatly appreciate it.
I'd also be interested in others' thoughts regarding the "turn WiFi off" comments from code rabbit.
I'm afraid I'll have to say no to this PR.
First, if you disable WiFi, how do you enable it again? Even AP mode on button press is disabled so there will be no way to enable it again. Or did I miss such behaviour?
the AP still starts with a button0 longpress (see 1st line in initAP().
Second, even though ESP-NOW uses WiFi circuitry it is not a WiFi protocol and should not be disabled. There is an option for enabling/disabling ESP-NOW in the settings.
I did not realize that ESP-NOW was not a WiFi protocol. I'll remove those code changes. That was my mistake.
Thirdly, there are unnecessary changes. Let me make the changes requested by you and @DedeHai and I'll pester you some more with another review. If there are additional unnecessary changes that are not noted so far, please let me know and I'll clean those up too.
ESPNow and wifi hardware are tightly entangled, it is not possible to disable wifi and have ESPNow enabled, at least in arduino framework afaik. Also the ability to disable wifi should be a usermod or at least a setting that first needs to be activated , hidden by default, with a big warning. Users tend to go nuts with random settings and I see much potential for bricked setups
ESPNow and wifi hardware are tightly entangled, it is not possible to disable wifi and have ESPNow enabled, at least in arduino framework afaik. so I should ignore coderabbit's suggestion about turning off the hardware. :)
Also the ability to disable wifi should be a usermod or at least a setting that first needs to be activated , hidden by default, with a big warning. Users tend to go nuts with random settings and I see much potential for bricked setups
The setting initializes to Active, can be enabled via b0 longpress, ETH (if installed), the BLE usermod (once I finish it) Not sure what suggestion or preference you have with respect to making the option hidden.
OK, @blazoncek said in an earlier PR of mine that he felt that disable WiFi should be integrated, not a usermod. So, I'm not sure which direction to go there... I already ripped out the usermod code.
I've made the code changes noted above. But, I still have to test the rebuild. I'll do that tonight and refresh the PR.
My point is that it should not be possible to accidentally disable wifi and brick a setup
OK, @blazoncek said in an earlier PR of mine that he felt that disable WiFi should be integrated, not a usermod. So, I'm not sure which direction to go there... I already ripped out the usermod code.
Yes, I said that. @DedeHai the approach to include possibility to enable WiFi as a core feature is IMO acceptable and preferred.
Also it is possible to disconnect STA and not enable AP and still use ESP-NOW. There is no need for WiFi stack to be enabled (apart from being initialised) to use ESP-NOW.
However, @rhkean not every device has a button and it may be impossible to recover form accidentally setting WiFi to be disabled. Also, reporting that WiFi is disabled in JSON may be unnecessary as nobody will be able to get that info (as web server will be inaccessible).
I would suggest to remove configuration items and only use JSON API to enable/disable WiFi. It can be scheduled as a boot preset to allow disabling WiFi on device start and may also allow to use IR (or other remote) to enable WiFi back on.
My own experiments (while doing ESP-NOW code) showed that if you do:
WiFi.disconnect();
WiFi.softAPdisconnect();
WiFi will be disabled (it will not connect to configured SSID and it will not open AP). However it will still allow ESP-NOW to function if desired.
There is one catch, though. WLED will periodically check if it is connected or not and will try to reconnect if WiFi is configured. You need to skip this check in handleConnection().
I am not against making it a core feature if done right, was just thinking out loud. What I do want to avoid is many complaints about bricked setups
I've made the changes noted above and improved the WiFi settings page by moving the disable setting to the bottom of the page and included a warning with required confirmation checkbox.
Did not test yet but from reading the code the double-check to disable it seems acceptable. Does ESPNow work like this? also, wifi-sleep is no longer respected. Not sure wifi-sleep should be generally disabled when enabling ESPNow as it does not work well with wifi-sleep enabled.
Did not test yet but from reading the code the double-check to disable it seems acceptable. Does ESPNow work like this? also, wifi-sleep is no longer respected. Not sure wifi-sleep should be generally disabled when enabling ESPNow as it does not work well with wifi-sleep enabled.
Can you point me to where the wifi-sleep is not respected? I can't find any of my modifications that impacted that. thanks
Can you point me to where the wifi-sleep is not respected? I can't find any of my modifications that impacted that. thanks
WiFi.setSleep() is not executed.
Can you point me to where the wifi-sleep is not respected? I can't find any of my modifications that impacted that. thanks
WiFi.setSleep()is not executed.
AH! good catch! thank you. fixed.
fully agree with @blazoncek, if there were a JSON command to disable wifi instead, that would be a much more elegant solution.
wow! Great feedback. Thank you so much.
My use case is BLE. My intentions were to implement disabling WiFi, then implement BLE, then eventually upgrade the BLE module with COEXISTENCE so the WiFi did not have to be disabled.
I'll make the changes you noted above, remove the config save and settings page changes, and add a JSON API call
If you are trying to implement BLE (I assume you also developed BLE enabled mobile app), be prepared to have ESP with at least 8MB of flash as otherwise your image will be too big (except if you'll fiddle with partitions which I do not recommend).
As for enabling and disabling WiFi (to prolong battery life for example) it would be sufficient (IMO) just to have JSON API (which can be triggered via various means like saved preset, HTTP, MQTT, IR, Wiz remote and UDP). It is true that you may only be able to disable WiFi via network methods, but playlists or IR or reboot will be able to enable it back on (or BLE command).
The important trick is how to properly handle disabled WiFi in handleConnection(). I was preoccupied with ESP-NOW to implement that but I may have a second look when time permits. If you are curious please see my fork where WiFi handling is almost entirely rewritten.
If you are trying to implement BLE (I assume you also developed BLE enabled mobile app),
yes, next step after getting BLE working from WLED, I plan to modify the Android App to include BLE support (after that, I hope to also add AndroidAuto functionality). The biggest bummer that I discovered this weekend was the the WROOM chips don't support ExtendedAdvertising.... sigh
be prepared to have ESP with at least 8MB of flash as otherwise your image will be too big (except if you'll fiddle with partitions which I do not recommend).
actually, using the IDF4 env that's included in the platformio.ini and the NimBLE-Arduino package (I need the newer g++ compiler in the IDF4 toolchain for NimBLE 2.x ... datatypes that aren't recognized by the g++ compiler that is included with the arduino toolchain), I have a build that is only about 1.35MB... don't even have to disable OTA. (granted at this point of only sent a few words back and forth, so.... we'll see. LOL ). That leaves me with about 200k to work with... fingers-crossed that it'll be enough
As for enabling and disabling WiFi (to prolong battery life for example) it would be sufficient (IMO) just to have JSON API (which can be triggered via various means like saved preset, HTTP, MQTT, IR, Wiz remote and UDP). It is true that you may only be able to disable WiFi via network methods, but playlists or IR or reboot will be able to enable it back on (or BLE command).
I haven't done much with the JSON API to date, so I didn't think of that option when I started modifying the code. That's the route I'll take this week. I'm assuming much of the valid parts of my PR would stay, and I only need to the triggering method to JSON API instead of Settings pages.
my real goal would be to get COEXISTENCE working so the wifi and ble work at the same time, but... I'll focus on getting the communication working first... LOL
The important trick is how to properly handle disabled WiFi in
handleConnection(). I was preoccupied with ESP-NOW to implement that but I may have a second look when time permits. If you are curious please see my fork where WiFi handling is almost entirely rewritten.
Thanks for the pointer. I'll take a look.
@blazoncek,
OK.... made all of the changes mentioned above. I've tested by putting a wifiEnabled = false; at the beginning of the setup() in wled.cpp. the code successfully disables WiFi and still allows a button0 long press to start the AP even with a WiFi configuration.
I'll add the JSON API call next.
For JSON API I'm thinking something in the lines of:
if (!wifi[F("on")].isNull()) {
bool sta = getBoolVal(wifi[F("on")], wifiEnabled);
if (Network.isConnected() && !sta) WiFi.disconnect();
if (sta) forceReconnect = true;
wifiEnabled = sta;
}
And simple logic in handleConnection() like:
if (wifiConfigured && !wifiEnabled) return;
This will allow firing up AP on boot if WiFi is not configured (assuming AP was not disabled).
The logic behind the above is that you WiFi.disconnect() from configured SSID via JSON API. This (disconnect(false)) will leave WiFi circuitry enabled and operational (for ESP-NOW for example) while no longer trying to connect to SSID. If you'd want to completely shut down WiFi, you can call WiFi.disconnect(true) instead.
When enabling WiFi back on using JSON (via IR or button or some other means), forcing a reconnect will do all that's necessary.
now that's an elegant solution... I'll code it up and start testing after work!
(I've got to figure out how to use SerialJSON for now, bc I don't have an IR remote lying about) :/ (at least until I get further along with the BLE stuff)
Create two presets: {"wifi":{"on":false}} and {"wifi":{"on":true}} or a single {"wifi":{"on":"t"}} and assign them/it to buttons.
Added JSON API added wifi["on"] and wifi["pwrOff"]
- button0 longpress will start the AP
- {"wifi":{"on":true}} will connect to configured wifi
- {"wifi":{"on":false}} will disconnect wifi leaving circuitry on
- {"wifi":{"on":false},"pwrOff":false} will disconnect wifi leaving circuitry on
- {"wifi":{"on":false},"pwrOff":true} will disconnect wifi and turn off the circuitry
Added JSON API added wifi["on"] and wifi["pwrOff"]
- button0 longpress will start the AP
- {"wifi":{"on":true}} will connect to configured wifi
- {"wifi":{"on":false}} will disconnect wifi leaving circuitry on
- {"wifi":{"on":false},"pwrOff":false} will disconnect wifi leaving circuitry on
- {"wifi":{"on":false},"pwrOff":true} will disconnect wifi and turn off the circuitry
I'm trying to track down the cause, but I've found, what I think is, an issue.
if I create a preset {"wifi":{"on":false,"pwrOff":true}} and set it to "Apply on boot", it works perfectly if wifiConfigured = false
if, however, wifiConfigured = true, then it connects to the WiFi, then gets disabled, but does not release the IP... watching the debug output, it continues to report the leased IP instead of 0.0.0.0. It also, seems to be taking more than 1 execution of the toggle preset to get it to reconnect to the network. (the multiple toggling to get it to re-enable doesn't happen if I don't select pwrOff, but the IP issue does)
(side note, it appears that saving a preset with "apply at boot" causes that preset to execute immediately. is that a bug?)
I'm trying to track down the cause, but I've found, what I think is, an issue. if I create a preset
{"wifi":{"on":false,"pwrOff":true}}and set it to "Apply on boot", it works perfectly ifwifiConfigured = falseif, however,
wifiConfigured = true, then it connects to the WiFi, then gets disabled, but does not release the IP... watching the debug output, it continues to report the leased IP instead of 0.0.0.0. It also, seems to be taking more than 1 execution of the toggle preset to get it to reconnect to the network. (the multiple toggling to get it to re-enable doesn't happen if I don't select pwrOff, but the IP issue does)
figured it out...
WiFi init is started before the bootPresets are applied, but before the WiFi connects...
so a proper WiFi.disconnect() is not getting called.
Does it need to release IP? It will get a new one on reconnect if needed.
If that interferes with your processing use WiFi.status() and check if it is either WL_IDLE_STATUS or WL_DISCONNECTED (or !WL_CONNECTED).
As for boot preset: It is executed in async fashion (as are all presets).
Does it need to release IP? It will get a new one on reconnect if needed. If that interferes with your processing use
WiFi.status()and check if it is either WL_IDLE_STATUS or WL_DISCONNECTED (or !WL_CONNECTED).
I, personally, don't care as long as the radio is off. It feels like bad practice, though... I'm not sure. I'll spit out some more debug statements and do some more testing after work. I'll pull in my BLE code and verify it doesn't crash. LOL
As for boot preset: It is executed in async fashion (as are all presets).
yeah, I figured. as is the WiFi connect call, I'm seeing. Hence the timing of it all. I'll try to get it to behave with a little more consistency.
Async behaviour is intended. It should be implemented everywhere.
I verified that the connection status is reporting as connect in the situation discussed earlier... I've updated the code to handle it.