Feature menu wifi ssid #13
Conversation
…e ble and display wifi/weater indication icon in screen
…ControlPanel into feature_menu_wifi_ssid
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a WiFi configuration menu feature that allows users to manage WiFi settings through the LCD interface. The changes restructure the WiFi configuration workflow by adding enable/disable toggles and reorganize the weather data handling system.
- Adds new menu options for WiFi enable/disable and OTA enable/disable settings
- Refactors weather data handling from global variables to persistent storage
- Reorganizes configuration structs into a pending configuration system for better state management
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| persistentVars.h | Adds weatherData_t struct and functions for persistent weather storage |
| lcd.h | Major refactor adding PendingConfig structs and new WiFi menu handlers |
| basic_defines.h | Updates default settings and adds new menu enum values |
| README.md | Reformats documentation with improved line wrapping |
| Firmware_ESP8266.ino | Adds WiFi handler parameter and settings change logging |
| ESP8266_Utils.h | Restructures weather handling and WiFi connection logic |
| EEPROM_Utils.h | Updates WifiSettings struct to use boolean flags instead of strings |
| if ((now - buttonHoldingTimeMs) >= LCD_HOLDING_TIME_CONFIRM_MS) { | ||
| if (previousButtonHolding == BUTTON_STATUS_DOWN) { | ||
| buzzer_sound_accept(); | ||
| ESP8266Utils_connectToWifi( |
There was a problem hiding this comment.
[nitpick] The WiFi connection attempt is moved inside the button holding timeout check. This means WiFi connection only starts after the user holds the button for the required time, which could confuse users who expect immediate feedback.
| if (adjustedPassword.length() > 11) { | ||
| String truncatedPassword = pendingConfig.wifi.password; | ||
| if (truncatedPassword.length() > 11) { | ||
| truncatedPassword = truncatedPassword.substring(truncatedPassword.length() - 11); |
There was a problem hiding this comment.
The variable 'truncatedPassword' is assigned but the condition check uses 'truncatedPassword.length()' instead of 'pendingConfig.wifi.password.length()' on line 849, which could lead to incorrect truncation logic.
| truncatedPassword = truncatedPassword.substring(truncatedPassword.length() - 11); | |
| if (pendingConfig.wifi.password.length() > 11) { | |
| truncatedPassword = pendingConfig.wifi.password.substring(pendingConfig.wifi.password.length() - 11); |
| void Wifi_handler() {} | ||
| bool ESP8266Utils_getWifiConnectAttemptTmo(void) { | ||
| if (lastConnectUpdateMs == 0) { | ||
| return true; // No hay intento previo, se considera un timeout |
There was a problem hiding this comment.
Mixed language comments: 'No hay intento previo, se considera un timeout' is in Spanish while most other comments are in English. Consider maintaining consistency in comment language.
| return true; // No hay intento previo, se considera un timeout | |
| return true; // No previous attempt, considered a timeout |
|
|
||
| bool ESP8266Utils_getWifiWeatherAttemptTmo(void) { | ||
| if (lastWeatherUpdateMs == 0) { | ||
| return true; // No hay intento previo, se considera un timeout |
There was a problem hiding this comment.
Mixed language comments: 'No hay intento previo, se considera un timeout' is in Spanish while most other comments are in English. Consider maintaining consistency in comment language.
| return true; // No hay intento previo, se considera un timeout | |
| return true; // No previous attempt, considered a timeout |
No description provided.