Feature menu wifi ssid #12
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds WiFi SSID selection functionality to the ESP8266 firmware menu system. It introduces a new menu flow that allows users to scan for available WiFi networks, select an SSID, enter credentials, and view connection results.
Key changes:
- WiFi network scanning and management with RSSI-based sorting
- New menu states for SSID selection and connection result display
- Enhanced LCD character set with connection and warning symbols
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Firmware_ESP8266/lcd.h | Adds WiFi SSID selection menu, new LCD special characters, and menu navigation logic |
| Firmware_ESP8266/basic_defines.h | Defines new menu enumeration values for WiFi configuration flow |
| Firmware_ESP8266/Firmware_ESP8266.ino | Integrates WiFi handler into main loop |
| Firmware_ESP8266/ESP8266_Utils.h | Implements WiFi network scanning, tracking, and connection management |
| Firmware_ESP8266/EEPROM_Utils.h | Fixes EEPROM scheduled write timing logic |
| // each time we enter this menu. | ||
| ESP8266Utils_startWifiScanIfNeeded(); | ||
| ESP8266Utils_checkScanResults(); | ||
| static int previousCurrentIndex = 1; // Start at 1 so in the first iteration we can |
There was a problem hiding this comment.
[nitpick] Magic number 1 is used without clear explanation. Consider defining a constant like 'INITIAL_NETWORK_INDEX' or starting at 0 with proper initialization logic.
| static int previousCurrentIndex = 1; // Start at 1 so in the first iteration we can | |
| static int previousCurrentIndex = INITIAL_NETWORK_INDEX; // Start at INITIAL_NETWORK_INDEX so in the first iteration we can |
| String mySSID = adjustedSSID; | ||
| int adjustedSSIDindex = ESP8266Utils_getIndexBySsid(adjustedSSID); | ||
| if (adjustedSSIDindex < 0) { | ||
| mySSID = "No hay redes"; |
There was a problem hiding this comment.
Hard-coded Spanish text should be replaced with a constant or configurable string for better maintainability and internationalization support.
| mySSID = "No hay redes"; | |
| mySSID = LCD_NO_NETWORKS_TEXT; |
| porcentaje = 100; // Limit to 100% | ||
| } | ||
| if (ESP8266Utils_isWifiConnected()) { | ||
| *lcdBuffer += String("WIFI: CONECTADO "); |
There was a problem hiding this comment.
Hard-coded Spanish text should be replaced with constants for better maintainability and internationalization support.
Firmware_ESP8266/ESP8266_Utils.h
Outdated
| seenInThisScan(true) {} | ||
| }; | ||
|
|
||
| std::vector<WifiNetworkInfo> wifiNetworks; |
There was a problem hiding this comment.
Global variable should be declared as 'static' to limit its scope to this compilation unit, or consider using a namespace or class to encapsulate this state.
| std::vector<WifiNetworkInfo> wifiNetworks; | |
| static std::vector<WifiNetworkInfo> wifiNetworks; |
Firmware_ESP8266/ESP8266_Utils.h
Outdated
| it->notSeenCount = 0; | ||
| } | ||
|
|
||
| if (it->notSeenCount >= 5) { |
There was a problem hiding this comment.
Magic number 5 should be defined as a constant like 'MAX_UNSEEN_SCANS' for better maintainability.
| if (it->notSeenCount >= 5) { | |
| if (it->notSeenCount >= MAX_UNSEEN_SCANS) { |
Firmware_ESP8266/ESP8266_Utils.h
Outdated
| } | ||
|
|
||
| // Filtro para evitar agregar redes volátiles de 1 sola muestra | ||
| if (rssi < -80) { |
There was a problem hiding this comment.
Magic number -80 should be defined as a constant like 'MIN_RSSI_THRESHOLD' for better maintainability.
| if (rssi < -80) { | |
| if (rssi < MIN_RSSI_THRESHOLD) { |
Firmware_ESP8266/ESP8266_Utils.h
Outdated
| std::sort(wifiNetworks.begin(), wifiNetworks.end(), compareByRssiDesc); | ||
|
|
||
| if (wifiNetworks.size() > 10) { | ||
| wifiNetworks.resize(10); |
There was a problem hiding this comment.
Magic number 10 should be defined as a constant like 'MAX_TRACKED_NETWORKS' for better maintainability.
| wifiNetworks.resize(10); | |
| if (wifiNetworks.size() > MAX_TRACKED_NETWORKS) { | |
| wifiNetworks.resize(MAX_TRACKED_NETWORKS); |
| return (elapsed * 100) / WIFI_CONNECTION_TIMEOUT_MS; | ||
| } | ||
|
|
||
| void Wifi_handler() {} |
There was a problem hiding this comment.
Empty function implementation that is called from the main loop serves no purpose and should either be implemented or removed to avoid confusion.
| void Wifi_handler() {} |
No description provided.