Add compile-time WiFi hard-off mode (WLED_FORCE_WIFI_OFF)#5394
Add compile-time WiFi hard-off mode (WLED_FORCE_WIFI_OFF)#5394Stargazer2026 wants to merge 10 commits intowled:mainfrom
Conversation
…an-behavior-with-lan Add compile-time WiFi hard-off mode (WLED_FORCE_WIFI_OFF)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdds a commented build-time feature flag Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wled00/wled.cpp (1)
514-520: Pre-guard WiFi calls on ESP32 are unnecessary when WiFi is forced off.
WiFi.setScanMethod(WIFI_ALL_CHANNEL_SCAN)(line 508) andWiFi.onEvent(WiFiEvent)(line 513) run unconditionally even in aWLED_FORCE_WIFI_OFFbuild. On ESP32 these are benign (scan method is irrelevant, the event handler simply never fires), but moving them into the#elsebranch keeps the forced-off path visibly clean and avoids any implicit WiFi-driver side-effects from configuration calls.♻️ Suggested cleanup
- `#ifndef` ESP8266 - WiFi.setScanMethod(WIFI_ALL_CHANNEL_SCAN); - WiFi.persistent(true); - `#else` - WiFi.persistent(false); - `#endif` - WiFi.onEvent(WiFiEvent); `#ifdef` WLED_FORCE_WIFI_OFF DEBUG_PRINTLN(F("WLED_FORCE_WIFI_OFF: forcing WiFi OFF.")); WiFi.mode(WIFI_OFF); `#else` + `#ifndef` ESP8266 + WiFi.setScanMethod(WIFI_ALL_CHANNEL_SCAN); + WiFi.persistent(true); + `#else` + WiFi.persistent(false); + `#endif` + WiFi.onEvent(WiFiEvent); WiFi.mode(WIFI_STA); // enable scanning findWiFi(true); // start scanning for available WiFi-s `#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.cpp` around lines 514 - 520, Move the unconditional WiFi configuration calls out of the forced-off path: when WLED_FORCE_WIFI_OFF is defined, avoid calling WiFi.setScanMethod(...) and WiFi.onEvent(WiFiEvent) so no WiFi config runs; instead, place WiFi.setScanMethod(WIFI_ALL_CHANNEL_SCAN) and WiFi.onEvent(WiFiEvent) inside the `#else` branch alongside WiFi.mode(WIFI_STA) and findWiFi(true). Update the block that currently uses WLED_FORCE_WIFI_OFF and WiFi.mode(...) so the forced-off branch only calls WiFi.mode(WIFI_OFF) and the scanning/event setup occurs only when WiFi is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/wled.cpp`:
- Around line 685-690: The WLED_FORCE_WIFI_OFF block in initConnection() exits
before updating lastReconnectAttempt, causing handleConnection() to repeatedly
call initConnection(); fix by setting lastReconnectAttempt = millis() inside the
WLED_FORCE_WIFI_OFF block (before the early return) so the reconnection timer is
updated; edit the block in initConnection() where WiFi.disconnect(true) and
WiFi.mode(WIFI_OFF) are called to assign lastReconnectAttempt (the same variable
used by handleConnection()) prior to returning.
---
Nitpick comments:
In `@wled00/wled.cpp`:
- Around line 514-520: Move the unconditional WiFi configuration calls out of
the forced-off path: when WLED_FORCE_WIFI_OFF is defined, avoid calling
WiFi.setScanMethod(...) and WiFi.onEvent(WiFiEvent) so no WiFi config runs;
instead, place WiFi.setScanMethod(WIFI_ALL_CHANNEL_SCAN) and
WiFi.onEvent(WiFiEvent) inside the `#else` branch alongside WiFi.mode(WIFI_STA)
and findWiFi(true). Update the block that currently uses WLED_FORCE_WIFI_OFF and
WiFi.mode(...) so the forced-off branch only calls WiFi.mode(WIFI_OFF) and the
scanning/event setup occurs only when WiFi is enabled.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
wled00/wled.cpp (1)
641-643: Consider adding a debug print for consistency with the other twoWLED_FORCE_WIFI_OFFblocks.
initAP()silently returns, whilesetup()(line 515) andinitConnection()(line 686) both emit aDEBUG_PRINTLNwhen forced off. This makes it harder to trace the suppression in debug logs.✨ Proposed addition
`#ifdef` WLED_FORCE_WIFI_OFF + DEBUG_PRINTLN(F("WLED_FORCE_WIFI_OFF: skipping AP init.")); return; `#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.cpp` around lines 641 - 643, The initAP() function currently returns silently when WLED_FORCE_WIFI_OFF is defined; add a DEBUG_PRINTLN call consistent with the other blocks (setup() and initConnection()) so the suppression is visible in logs. Locate the WLED_FORCE_WIFI_OFF guard inside initAP() and insert a DEBUG_PRINTLN with a clear message (e.g., "WLED_FORCE_WIFI_OFF: AP init suppressed") immediately before the return so behavior matches the other WLED_FORCE_WIFI_OFF blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@wled00/wled.cpp`:
- Around line 641-643: The initAP() function currently returns silently when
WLED_FORCE_WIFI_OFF is defined; add a DEBUG_PRINTLN call consistent with the
other blocks (setup() and initConnection()) so the suppression is visible in
logs. Locate the WLED_FORCE_WIFI_OFF guard inside initAP() and insert a
DEBUG_PRINTLN with a clear message (e.g., "WLED_FORCE_WIFI_OFF: AP init
suppressed") immediately before the return so behavior matches the other
WLED_FORCE_WIFI_OFF blocks.
netmindz
added
connectivity
|
If we add this PR, then there should be a HUGE warning that "no wifi" means "no wifi connection at all"
|
|
(joke) maybe rename the build flag to WLED_FORCE_WIFI_OFF_AND_SHOOT_YOURSELF_IN_THE_FOOT 😉 I'm not saying NO to this PR - there might be special use cases where its helpful. But I would prefer a less "hard off" approach, maybe by adding a "wifi mode" user setting next to "AP mode", and keep at least "AP mode by button" operational for emergency cases. |
|
can you please elaborate on the advantages? |
|
Thank you for reviewing this. The intention of this PR is not to change default behavior. WiFi remains enabled unless explicitly excluded via a compile time flag. The primary motivation is deterministic Ethernet only deployments. In certain installations, especially fixed architectural or PoE based setups, it is desirable to completely exclude the WiFi stack at build time rather than simply not configuring it. This approach has several technical advantages:
The decision to implement this as a compile time exclusion instead of a runtime toggle was intentional. A runtime option would require:
By contrast, this implementation:
The feature is strictly opt in for specialized builds and does not affect existing users or standard firmware releases. If needed, I am happy to provide exact firmware size comparisons or adjust naming and placement of the build flag to better match existing conventions. |
|
The only point I am getting is the "radio restricted area" thing, but that is very niche. All other points can be addressed in a different way and the firmware size reduction is not really helpful unless you pack it full of usermods. |
|
Thanks for the clarification. I understand that this is a niche use case. I am not trying to position it as something broadly needed for typical installations. My main point is that the implementation cost for the project is extremely low:
This does not introduce additional behavioral complexity in the normal code path. If the flag is not set, the firmware behaves exactly as before. Regarding flexibility: yes, a recompile is required if the network topology changes. But since this is a compile-time exclusion, it is explicitly targeting fixed-purpose deployments where that trade-off is intentional. In other words, this is not meant as a user-facing feature but as a specialization hook for constrained or regulated environments. Given the very small and well-contained delta, I would argue that the long-term maintenance overhead should be close to zero, especially compared to runtime configuration features. If you still feel this increases project surface unnecessarily, I completely understand. I just wanted to clarify that the implementation was intentionally minimal and non-invasive. |
I do understand your intend and I can see its usefulness for some cases, however my concern is this: edit: edit2: |
wled00/wled.h
Outdated
| #endif | ||
|
|
||
| // You can choose some of these features to disable: | ||
| //#define WLED_FORCE_WIFI_OFF // saves 5kb, hard-disable WiFi radio entirely (WIFI_OFF). AP/STA will not start. |
There was a problem hiding this comment.
Its better to not add the flag here. It will be too tempting for "cool lets try this" users to brick their boards.
For documentation, i'd prefer a PR in WLED-Docs and describe the new flag as an expert option.
wled00/my_config_sample.h
Outdated
| #define CLIENT_PASS "Your_Password" | ||
| */ | ||
|
|
||
| //#define WLED_FORCE_WIFI_OFF // Hard disable WiFi radio (WIFI_OFF): no STA and no AP. |
There was a problem hiding this comment.
please remove the flag here, too (see my comment on wled.h)
|
A few use case questions that came to my mind:
|
|
@Stargazer2026 please don't get me wrong - I'm not trying to corner your PR. This is just an attempt to find out what could be the consequences of having WLED without wifi option. See it as a free "safety analysis", that can be used to improve the safety and availability in non-normal cases. Actually, this kind of analysis is part of my "day time job", so don't take my questions personally :-) |
|
I can completely understand that outside of the home setting, that to have devices that might run on ethernet 99% of the time, the suddenly swap to AP mode in an office, nightclub etc just because someone rebooted the router is a valid concern, so having the option to select ethernet only makes sense. I'm less clear on the use case of when you would use WLED in a setting where you couldn't (legally?) have an active radio and if this change would be robust enough to meet that compliance 100% - it's certainly a feature I wouldn't want the team to take responsibility for ensuring every release never activates the radio at all |
I agree, especially we know that many "non-expert admins" will simply use OTA to refresh their device with the latest software. Like "I trust HomeAssistent to pick the right update for me". After that point, the WLED device will start offering its WLED-AP against all "radio off" assumptions. As a counter-measure, removing OTA and ESP-NOW could work. But if hard "radio off" requirements need to be met, it also means to put the board into a hard-to-break enclosure with intruder alert, to prevent updating WLED by UART pins. |
|
First of all, thank you all for the detailed analysis! I genuinely appreciate the “safety review” perspective. These are exactly the right questions to ask :-) Let me clarify one important point: This feature is not intended as a regulatory compliance guarantee and should not be marketed or understood as “certified radio-off firmware”. It is simply a compile-time exclusion of the WiFi stack for controlled, fixed-purpose builds where the integrator fully owns the deployment. I completely agree that default WLED behavior must remain flexible, recoverable and safe for normal users. Given the concerns raised, I would propose tightening the scope further:
Regarding responsibility: From an implementation perspective, the delta remains small, isolated and fully opt-in. Default builds and OTA users remain completely unaffected. If the concern is mainly about inexperienced users experimenting with it, I am fully in favor of making access intentionally inconvenient (for example via forced acknowledgement in code), so it does not become a “cool let’s try this” toggle. My goal here is not to expand WLED’s general feature surface, but to provide a safe specialization hook for advanced deployments without affecting mainstream users. Happy to adjust the PR accordingly. |
|
Thanks for your response :-) I think "make access inconvenient" is the way to go 👍
What if we could have both? If we lower our expectations on "radio off", we could still allow "WLEP-AP by button" for dummy user recovery. The 5Kb firmware shrink is nothing we absolutely need, so keeping the AP code active but enforcing "AP_BEHAVIOR_BUTTON_ONLY" could be an option. Not saving 5Kb flash is really a small price, compared to a few "stuck users" who need help to get out of their "no Wi-Fi" problems, especially if the "inaccessible" board is sealed in epoxy and fixed to the roof ;-) |
|
Thanks again for the constructive feedback. I’ve reworked the implementation based on the discussion. Updated BehaviorWhen
In other words, WiFi is no longer part of normal network behavior, but intentional recovery via button-triggered AP is still possible. This addresses the main concern about “stuck” devices while still preventing unintended RF activity in Ethernet-first deployments. Compile-Time SafeguardsTo make this feature safer and clearly expert-only, I added the following safeguards:
This shifts prerequisite validation to compile time, where it is most useful for this type of feature. Scope ClarificationThis is not intended to provide regulatory “radio-off compliance”. Default firmware behavior remains completely unchanged. The implementation remains small and isolated:
If the naming should better reflect the soft-off semantics instead of “FORCE_WIFI_OFF”, I’m happy to adjust that as well. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
wled00/wled.h (1)
32-32: Flag placement in the feature-disable section attracts casual experimentationThe "You can choose some of these features to disable" block is the first place new users look to customise builds. Placing
WLED_FORCE_WIFI_OFFhere without a stronger barrier (e.g. a deliberate#errorthat must be commented out, or docs-only exposure) is the concern raised in a prior review that remains open.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.h` at line 32, The WLED_FORCE_WIFI_OFF flag is placed in the beginner-facing "feature-disable" block and may be accidentally enabled by new users; move the definition out of that block and into a clearly labeled advanced/experimental section (or keep it in place but require an explicit opt-in barrier such as a deliberate `#error` that must be commented out) and add a strong explanatory comment referencing WLED_FORCE_WIFI_OFF and the emergency AP recovery requirement so accidental enabling is unlikely; update the surrounding comment to point to the momentary button 0 recovery requirement and/or documentation location for safe use.wled00/my_config_sample.h (1)
25-25: Flag in sample config risks casual misusePlacing
WLED_FORCE_WIFI_OFFinmy_config_sample.halongside everyday opt-in knobs (MAX_LEDS,MDNS_NAME) makes it easy for non-expert users to enable it without understanding the brick risk. This concern was raised in a prior review and remains unresolved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/my_config_sample.h` at line 25, The sample config exposes the dangerous compile-time flag WLED_FORCE_WIFI_OFF in my_config_sample.h near everyday knobs (e.g., MAX_LEDS, MDNS_NAME), risking accidental bricking; remove or relocate that flag from the everyday sample and instead place it in a clearly marked advanced/experimental area (or a separate file like my_config_advanced.h) with an explicit, strong warning mentioning the requirement for a configured momentary button 0 for emergency AP recovery and advising novices not to enable it; update references/comments so only advanced users intentionally enable WLED_FORCE_WIFI_OFF.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/wled.cpp`:
- Around line 682-691: The early-return condition in handleConnection() uses
WiFi.scanComplete() and blocks progress when WLED_FORCE_WIFI_OFF causes WiFi to
be off; update the guard so the scanComplete check is skipped when WiFi is
turned off (e.g., add a runtime check like WiFi.getMode() != WIFI_OFF or
!WiFi.isEnabled()) so the expression becomes something like: (wifiConfigured &&
multiWiFi.size() > 1 && WiFi.getMode() != WIFI_OFF && WiFi.scanComplete() < 0)
|| ...; this lets handleConnection() fall through to the else-if that checks
interfacesInited and call initInterfaces() when Ethernet comes up while
preserving existing multi-SSID scan behavior when WiFi is actually active (refer
to handleConnection(), WiFi.scanComplete(), multiWiFi, wifiConfigured,
initInterfaces(), interfacesInited).
---
Duplicate comments:
In `@wled00/my_config_sample.h`:
- Line 25: The sample config exposes the dangerous compile-time flag
WLED_FORCE_WIFI_OFF in my_config_sample.h near everyday knobs (e.g., MAX_LEDS,
MDNS_NAME), risking accidental bricking; remove or relocate that flag from the
everyday sample and instead place it in a clearly marked advanced/experimental
area (or a separate file like my_config_advanced.h) with an explicit, strong
warning mentioning the requirement for a configured momentary button 0 for
emergency AP recovery and advising novices not to enable it; update
references/comments so only advanced users intentionally enable
WLED_FORCE_WIFI_OFF.
In `@wled00/wled.h`:
- Line 32: The WLED_FORCE_WIFI_OFF flag is placed in the beginner-facing
"feature-disable" block and may be accidentally enabled by new users; move the
definition out of that block and into a clearly labeled advanced/experimental
section (or keep it in place but require an explicit opt-in barrier such as a
deliberate `#error` that must be commented out) and add a strong explanatory
comment referencing WLED_FORCE_WIFI_OFF and the emergency AP recovery
requirement so accidental enabling is unlikely; update the surrounding comment
to point to the momentary button 0 recovery requirement and/or documentation
location for safe use.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wled00/wled.cpp (1)
894-899:⚠️ Potential issue | 🟠 MajorGuard
findWiFi()calls inhandleConnection()whenWLED_FORCE_WIFI_OFFis active.
findWiFi()executesWiFi.scanNetworks(true)whenWiFi.scanComplete()returnsWIFI_SCAN_FAILED. At lines 896, 929, and 934 inhandleConnection(), these calls execute unconditionally, even thoughWiFi.mode(WIFI_OFF)is set at line 517 whenWLED_FORCE_WIFI_OFFis defined. WhileWiFi.scanNetworks()will fail silently on a disabled radio (NULL mode), the calls violate the design intent of keeping WiFi disabled. OtherfindWiFi()calls innetwork.cpp(lines 390, 433) are already properly guarded with#ifndef WLED_FORCE_WIFI_OFF; the same guard should be applied at these three locations.🔧 Suggested guard to prevent scan attempts when WiFi is forced off
- selectedWiFi = findWiFi(); // find strongest WiFi +#ifndef WLED_FORCE_WIFI_OFF + selectedWiFi = findWiFi(); // find strongest WiFi +#endif initConnection(); @@ - if (scanDone && multiWiFi.size() > 1) { +#ifndef WLED_FORCE_WIFI_OFF + if (scanDone && multiWiFi.size() > 1) { DEBUG_PRINTLN(F("WiFi scan initiated on disconnect.")); findWiFi(true); // reinit scan scanDone = false; return; // try to connect in next iteration } +#endif @@ - selectedWiFi = findWiFi(); +#ifndef WLED_FORCE_WIFI_OFF + selectedWiFi = findWiFi(); +#endif initConnection();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.cpp` around lines 894 - 899, In handleConnection(), the unconditional calls to findWiFi() (used to pick selectedWiFi) violate the WLED_FORCE_WIFI_OFF intent; wrap each invocation of findWiFi() in an `#ifndef` WLED_FORCE_WIFI_OFF / `#endif` guard so the scan is skipped when WiFi is forcibly disabled (same pattern used in network.cpp). Specifically, protect the findWiFi() calls inside handleConnection() (the initial connect/forced reconnect path and the reconnect attempts) with that preprocessor guard so WiFi.scanNetworks() is not invoked when WiFi.mode(WIFI_OFF) is in effect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@wled00/wled.cpp`:
- Around line 894-899: In handleConnection(), the unconditional calls to
findWiFi() (used to pick selectedWiFi) violate the WLED_FORCE_WIFI_OFF intent;
wrap each invocation of findWiFi() in an `#ifndef` WLED_FORCE_WIFI_OFF / `#endif`
guard so the scan is skipped when WiFi is forcibly disabled (same pattern used
in network.cpp). Specifically, protect the findWiFi() calls inside
handleConnection() (the initial connect/forced reconnect path and the reconnect
attempts) with that preprocessor guard so WiFi.scanNetworks() is not invoked
when WiFi.mode(WIFI_OFF) is in effect.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
wled00/wled.cpp (1)
929-934: Avoid misleading scan log when scans are compiled out.Line 930 logs “scan initiated” even though
findWiFi(true)is omitted underWLED_FORCE_WIFI_OFF. Consider making the log conditional or updating the message.💡 Optional tweak
if (scanDone && multiWiFi.size() > 1) { - DEBUG_PRINTLN(F("WiFi scan initiated on disconnect.")); -#ifndef WLED_FORCE_WIFI_OFF - findWiFi(true); // reinit scan -#endif +#ifndef WLED_FORCE_WIFI_OFF + DEBUG_PRINTLN(F("WiFi scan initiated on disconnect.")); + findWiFi(true); // reinit scan +#else + DEBUG_PRINTLN(F("WiFi scan suppressed (WLED_FORCE_WIFI_OFF).")); +#endif scanDone = false; return; // try to connect in next iteration }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.cpp` around lines 929 - 934, The log message "WiFi scan initiated on disconnect." can be misleading when scans are compiled out under WLED_FORCE_WIFI_OFF; update the condition around the DEBUG_PRINTLN or change the message so it only appears when findWiFi(true) will actually be called. Concretely, move or wrap the DEBUG_PRINTLN(F("WiFi scan initiated on disconnect.")) so it is executed only when WLED_FORCE_WIFI_OFF is not defined (alongside the call to findWiFi(true)), or alter the text to say "WiFi scan skipped (scanning disabled) on disconnect" when WLED_FORCE_WIFI_OFF is defined; adjust the block that uses scanDone, multiWiFi, and findWiFi accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@wled00/wled.cpp`:
- Around line 929-934: The log message "WiFi scan initiated on disconnect." can
be misleading when scans are compiled out under WLED_FORCE_WIFI_OFF; update the
condition around the DEBUG_PRINTLN or change the message so it only appears when
findWiFi(true) will actually be called. Concretely, move or wrap the
DEBUG_PRINTLN(F("WiFi scan initiated on disconnect.")) so it is executed only
when WLED_FORCE_WIFI_OFF is not defined (alongside the call to findWiFi(true)),
or alter the text to say "WiFi scan skipped (scanning disabled) on disconnect"
when WLED_FORCE_WIFI_OFF is defined; adjust the block that uses scanDone,
multiWiFi, and findWiFi accordingly.
| #error "WLED_FORCE_WIFI_OFF requires WLED_USE_ETHERNET." | ||
| #endif | ||
| #if (BTNPIN < 0) | ||
| #warning "WLED_FORCE_WIFI_OFF: BTNPIN is disabled. Emergency AP long-press on button 0 will not be available unless configured in cfg.json." |
There was a problem hiding this comment.
both button defines should be mendatory IMHO
There was a problem hiding this comment.
Thanks for the feedback. How do you feel about a change as outlined below?
It moves the default definitions of BTNPIN and BTNTYPE below the checks for the WLED_FORCE_WIFI_OFF use case. This results in a hard enforcement that the button is explicitly configured by the user in the intended way. It also changes from warning to error in case it's missing.
diff --git a/wled00/wled.h b/wled00/wled.h
index 207848d2671fda94e14b671d2d61fb30c7e178e8..7e29a4b231df4fe9baeb652fa4ee85c38a432b0f 100644
--- a/wled00/wled.h
+++ b/wled00/wled.h
@@ -263,69 +263,75 @@ using PSRAMDynamicJsonDocument = BasicJsonDocument<PSRAM_Allocator>;
#define _INIT(x)
#define _INIT_N(x)
#define _INIT_PROGMEM(x)
#else
#define WLED_GLOBAL
#define _INIT(x) = x
//needed to ignore commas in array definitions
#define UNPACK( ... ) __VA_ARGS__
#define _INIT_N(x) UNPACK x
#define _INIT_PROGMEM(x) PROGMEM = x
#endif
#define STRINGIFY(X) #X
#define TOSTRING(X) STRINGIFY(X)
#define WLED_CODENAME "Niji"
// AP and OTA default passwords (for maximum security change them!)
WLED_GLOBAL char apPass[65] _INIT(WLED_AP_PASS);
#ifdef WLED_OTA_PASS
WLED_GLOBAL char otaPass[33] _INIT(WLED_OTA_PASS);
#else
WLED_GLOBAL char otaPass[33] _INIT(DEFAULT_OTA_PASS);
#endif
-// Hardware and pin config
-#ifndef BTNPIN
- #define BTNPIN 0
-#endif
-#ifndef BTNTYPE
- #define BTNTYPE BTN_TYPE_PUSH
-#endif
-
#ifdef WLED_FORCE_WIFI_OFF
#ifndef WLED_USE_ETHERNET
#error "WLED_FORCE_WIFI_OFF requires WLED_USE_ETHERNET."
#endif
+ #ifndef BTNPIN
+ #error "WLED_FORCE_WIFI_OFF requires BTNPIN to be explicitly defined."
+ #endif
+ #ifndef BTNTYPE
+ #error "WLED_FORCE_WIFI_OFF requires BTNTYPE to be explicitly defined."
+ #endif
#if (BTNPIN < 0)
- #warning "WLED_FORCE_WIFI_OFF: BTNPIN is disabled. Emergency AP long-press on button 0 will not be available unless configured in cfg.json."
+ #error "WLED_FORCE_WIFI_OFF requires BTNPIN >= 0 so emergency AP long-press can be triggered on button 0."
#endif
#if !((BTNTYPE == BTN_TYPE_PUSH) || (BTNTYPE == BTN_TYPE_PUSH_ACT_HIGH) || (BTNTYPE == BTN_TYPE_TOUCH))
- #warning "WLED_FORCE_WIFI_OFF: BTNTYPE is not momentary by default. Emergency AP long-press on button 0 requires a momentary button type."
+ #error "WLED_FORCE_WIFI_OFF requires a momentary BTNTYPE (BTN_TYPE_PUSH / BTN_TYPE_PUSH_ACT_HIGH / BTN_TYPE_TOUCH)."
#endif
#endif
+
+// Hardware and pin config
+#ifndef BTNPIN
+ #define BTNPIN 0
+#endif
+#ifndef BTNTYPE
+ #define BTNTYPE BTN_TYPE_PUSH
+#endif
#ifndef RLYPIN
WLED_GLOBAL int8_t rlyPin _INIT(-1);
#else
WLED_GLOBAL int8_t rlyPin _INIT(RLYPIN);
#endif
//Relay mode (1 = active high, 0 = active low, flipped in cfg.json)
#ifndef RLYMDE
WLED_GLOBAL bool rlyMde _INIT(true);
#else
WLED_GLOBAL bool rlyMde _INIT(RLYMDE);
#endif
//Use open drain (floating pin) when relay should be off
#ifndef RLYODRAIN
WLED_GLOBAL bool rlyOpenDrain _INIT(false);
#else
WLED_GLOBAL bool rlyOpenDrain _INIT(RLYODRAIN);
#endif
#ifndef IRPIN
#define IRPIN -1
#endif
#ifndef IRTYPE
#define IRTYPE 0
#endif
#if defined(CONFIG_IDF_TARGET_ESP32S3) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S2) || (defined(RX) && defined(TX))
|
I refactored the implementation now to use less #ifdef blocks. Please have a look at commit d14643b and let me know if you prefer this implementation or the former implementation, with more explicit (inline) #ifdef blocks. |
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/network.cpp`:
- Around line 13-17: The function restartWiFiScanIfEnabled() is returning early
for any negative WiFi.scanComplete() value, which blocks recovery for
WIFI_SCAN_FAILED; change the check so only an ongoing scan (WIFI_SCAN_RUNNING)
returns early and allow WIFI_SCAN_FAILED to fall through to findWiFi(true).
Concretely, in restartWiFiScanIfEnabled() read int status = WiFi.scanComplete();
if (status == WIFI_SCAN_RUNNING) return false; (keep the rest of the guards:
interfacesInited, requireMultiWiFi) so failed scans can trigger findWiFi(true);
this aligns behavior with findWiFi(), wifiScanRunning(), and handleConnection().
|
how do the two versions compare regarding code size? |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
They're almost identical: | Code Base / Build Target | WIFI ON | WIFI OFF |
|--------------------------|---------|----------|
| Before Refactoring | 1301777 | 1297105 |
| After Refactoring | 1301789 | 1297077 | |
4 participants
Motivation
Description
WLED_FORCE_WIFI_OFFtowled.hand document it inmy_config_sample.hso users can enable it frommy_config.hor build flags.wled.cpp, change startup networking to callWiFi.mode(WIFI_OFF)whenWLED_FORCE_WIFI_OFFis defined instead of starting STA scanning andfindWiFi(true).initAP()to immediately return whenWLED_FORCE_WIFI_OFFis defined so no softAP is created.initConnection()early-exit whenWLED_FORCE_WIFI_OFFis defined after callingWiFi.disconnect(true)and forcingWiFi.mode(WIFI_OFF), and add debug prints to indicate the forced-off state.Summary by CodeRabbit
New Features
Documentation