GPIOs not checked when board profile is adjusted

#2841

src/core/console.cpp

@@ -100,8 +100,6 @@ static void setup_commands(std::shared_ptr<Commands> const & commands) {
                 EMSESP::show_sensor_values(shell);
             } else if (command == F_(mqtt)) {
                 Mqtt::show_mqtt(shell);
-            } else if (command == F_(gpio)) {
-                EMSESP::system_.show_gpio(shell);
             } else {
                 shell.printfln("Unknown show command");
             }

src/core/locale_common.h

@@ -159,7 +159,7 @@ MAKE_WORD_CUSTOM(deviceid_mandatory, "<deviceID>")
 MAKE_WORD_CUSTOM(device_type_optional, "[device]")
 MAKE_WORD_CUSTOM(invalid_log_level, "Invalid log level")
 MAKE_WORD_CUSTOM(log_level_optional, "[level]")
-MAKE_WORD_CUSTOM(show_commands, "[system | users | devices | log | ems | values | mqtt | commands | gpio]")
+MAKE_WORD_CUSTOM(show_commands, "[system | users | devices | log | ems | values | mqtt | commands]")
 MAKE_WORD_CUSTOM(name_mandatory, "<name>")
 MAKE_WORD_CUSTOM(name_optional, "[name]")
 MAKE_WORD_CUSTOM(new_password_prompt1, "Enter new password: ")

src/core/system.cpp

@@ -96,6 +96,8 @@ uint32_t                                      System::max_alloc_mem_;
 uint32_t                                      System::heap_mem_;
 std::vector<uint8_t, AllocatorPSRAM<uint8_t>> System::valid_system_gpios_;
 std::vector<uint8_t, AllocatorPSRAM<uint8_t>> System::used_gpios_;
+std::vector<uint8_t, AllocatorPSRAM<uint8_t>> System::snapshot_used_gpios_;
+std::vector<uint8_t, AllocatorPSRAM<uint8_t>> System::snapshot_valid_system_gpios_;
 
 // find the index of the language
 // 0 = EN, 1 = DE, etc...
@@ -1015,22 +1017,6 @@ void System::show_users(uuid::console::Shell & shell) {
     shell.println();
 }
 
-// print GPIO available and used pins to console
-void System::show_gpio(uuid::console::Shell & shell) {
-    shell.printfln("GPIO:");
-    shell.printf(" In use (%d):", used_gpios_.size());
-    for (const auto & gpio : used_gpios_) {
-        shell.printf(" %d", gpio);
-    }
-    shell.println();
-    auto available = available_gpios();
-    shell.printf(" Available (%d):", available.size());
-    for (const auto & gpio : available) {
-        shell.printf(" %d", gpio);
-    }
-    shell.println();
-}
-
 // shell command 'show system'
 void System::show_system(uuid::console::Shell & shell) {
     refreshHeapMem(); // refresh free heap and max alloc heap
@@ -1068,8 +1054,20 @@ void System::show_system(uuid::console::Shell & shell) {
     } else {
         shell.printfln(" PSRAM: not available");
     }
-
+    // GPIOs
+    shell.printf(" GPIO in use (%d):", used_gpios_.size());
+    for (const auto & gpio : used_gpios_) {
+        shell.printf(" %d", gpio);
+    }
     shell.println();
+    auto available = available_gpios();
+    shell.printf(" GPIO available (%d):", available.size());
+    for (const auto & gpio : available) {
+        shell.printf(" %d", gpio);
+    }
+    shell.println();
+    shell.println();
+
     shell.println("Network:");
 
     switch (WiFi.status()) {
@@ -2241,10 +2239,9 @@ bool System::load_board_profile(std::vector<int8_t> & data, const std::string &
 
 // format command - factory reset, removing all config files
 bool System::command_format(const char * value, const int8_t id) {
-    LOG_INFO("Formatting FS, removing all config files");
 #ifndef EMSESP_STANDALONE
     if (LittleFS.format()) {
-        LOG_INFO("FS formatted successfully");
+        LOG_INFO("Filesystem formatted successfully. All config files removed.");
     } else {
         LOG_ERROR("Format failed");
     }
@@ -2688,4 +2685,30 @@ std::vector<uint8_t> System::available_gpios() {
     return gpios;
 }
 
+// make a snapshot of the current GPIOs
+void System::make_snapshot_gpios() {
+    snapshot_used_gpios_.clear();
+    for (const auto & gpio : used_gpios_) {
+        snapshot_used_gpios_.push_back(gpio);
+    }
+
+    snapshot_valid_system_gpios_.clear();
+    for (const auto & gpio : valid_system_gpios_) {
+        snapshot_valid_system_gpios_.push_back(gpio);
+    }
+}
+
+// restore the GPIOs from the snapshot
+void System::restore_snapshot_gpios() {
+    used_gpios_.clear();
+    for (const auto & gpio : snapshot_used_gpios_) {
+        used_gpios_.push_back(gpio);
+    }
+
+    valid_system_gpios_.clear();
+    for (const auto & gpio : snapshot_valid_system_gpios_) {
+        valid_system_gpios_.push_back(gpio);
+    }
+}
+
 } // namespace emsesp

src/core/system.h

@@ -145,9 +145,12 @@ class System {
     static void extractSettings(const char * filename, const char * section, JsonObject output);
     static bool saveSettings(const char * filename, const char * section, JsonObject input);
 
+    // GPIOs
     static bool                 add_gpio(uint8_t pin, const char * source_name);
     static std::vector<uint8_t> available_gpios();
     static bool                 load_board_profile(std::vector<int8_t> & data, const std::string & board_profile);
+    static void                 make_snapshot_gpios();
+    static void                 restore_snapshot_gpios();
 
     static bool readCommand(const char * data);
 
@@ -301,7 +304,6 @@ class System {
 
     void show_system(uuid::console::Shell & shell);
     void show_users(uuid::console::Shell & shell);
-    void show_gpio(uuid::console::Shell & shell);
 
     void wifi_reconnect();
 
@@ -404,6 +406,8 @@ class System {
 
     static std::vector<uint8_t, AllocatorPSRAM<uint8_t>> valid_system_gpios_;          // list of valid GPIOs for the ESP32 board that can be used
     static std::vector<uint8_t, AllocatorPSRAM<uint8_t>> used_gpios_;                  // list of GPIOs used by the application
+    static std::vector<uint8_t, AllocatorPSRAM<uint8_t>> snapshot_used_gpios_;         // snapshot of the used GPIOs
+    static std::vector<uint8_t, AllocatorPSRAM<uint8_t>> snapshot_valid_system_gpios_; // snapshot of the valid GPIOs
 
     int8_t wifi_quality(int8_t dBm);
 

src/web/WebSettingsService.cpp

@@ -91,6 +91,9 @@ StateUpdateResult WebSettings::update(JsonObject root, WebSettings & settings) {
     // make a copy of the settings to compare to later
     const WebSettings original_settings(settings);
 
+    // make a snapshot of the current GPIOs
+    EMSESP::system_.make_snapshot_gpios();
+
     reset_flags();
 
     settings.version       = root["version"] | EMSESP_DEFAULT_VERSION; // save the version, we use it later in System::check_upgrade()
@@ -147,8 +150,6 @@ StateUpdateResult WebSettings::update(JsonObject root, WebSettings & settings) {
     EMSESP::system_.remove_gpio(1, true);
     EMSESP::system_.remove_gpio(3, true);
 #endif
-    // if any of the GPIOs have changed and re-validate them
-    bool have_valid_gpios = true;
 
     // free old gpios from used list to allow remapping
     EMSESP::system_.remove_gpio(original_settings.led_gpio);
@@ -157,27 +158,32 @@ StateUpdateResult WebSettings::update(JsonObject root, WebSettings & settings) {
     EMSESP::system_.remove_gpio(original_settings.rx_gpio);
     EMSESP::system_.remove_gpio(original_settings.tx_gpio);
 
-    // now add new gpio assignment, start with rx/tx
+    // if any of the GPIOs have changed and re-validate them
+    bool have_valid_gpios = true;
+
+    // Helper lambda for optional GPIOs (can be 0 to disable)
+    auto add_optional_gpio = [&have_valid_gpios](uint8_t & gpio, const char * name) {
+        if (gpio != 0 && !EMSESP::system_.add_gpio(gpio, name)) {
+            gpio             = 0; // 0 means disabled
+            have_valid_gpios = false;
+        }
+    };
+
+    // add new gpio assignment
     check_flag(original_settings.rx_gpio, settings.rx_gpio, ChangeFlags::UART);
-    have_valid_gpios = have_valid_gpios && EMSESP::system_.add_gpio(settings.rx_gpio, "UART Rx");
+    have_valid_gpios &= EMSESP::system_.add_gpio(settings.rx_gpio, "UART Rx");
 
     check_flag(original_settings.tx_gpio, settings.tx_gpio, ChangeFlags::UART);
-    have_valid_gpios = have_valid_gpios && EMSESP::system_.add_gpio(settings.tx_gpio, "UART Tx");
+    have_valid_gpios &= EMSESP::system_.add_gpio(settings.tx_gpio, "UART Tx");
 
     check_flag(original_settings.led_gpio, settings.led_gpio, ChangeFlags::LED);
-    if (settings.led_gpio != 0 && !EMSESP::system_.add_gpio(settings.led_gpio, "LED")) {
+    add_optional_gpio(settings.led_gpio, "LED");
-        settings.led_gpio = 0; // 0 means disabled
-        have_valid_gpios  = false;
-    }
 
     check_flag(original_settings.dallas_gpio, settings.dallas_gpio, ChangeFlags::TEMPERATURE_SENSOR);
-    if (settings.dallas_gpio != 0 && !EMSESP::system_.add_gpio(settings.dallas_gpio, "Dallas")) {
+    add_optional_gpio(settings.dallas_gpio, "Dallas");
-        settings.dallas_gpio = 0; // 0 means disabled
-        have_valid_gpios     = false;
-    }
 
     check_flag(original_settings.pbutton_gpio, settings.pbutton_gpio, ChangeFlags::BUTTON);
-    have_valid_gpios = have_valid_gpios && EMSESP::system_.add_gpio(settings.pbutton_gpio, "Button");
+    have_valid_gpios &= EMSESP::system_.add_gpio(settings.pbutton_gpio, "Button");
 
     // check if the LED type, eth_phy_addr or eth_clock_mode have changed
     check_flag(original_settings.led_type, settings.led_type, ChangeFlags::LED);
@@ -303,19 +309,23 @@ StateUpdateResult WebSettings::update(JsonObject root, WebSettings & settings) {
         settings.weblog_buffer = root["weblog_buffer"] | 25; // limit to 25 messages if no psram
     }
 
+    // save the settings if changed from the webUI
+    // if we encountered an invalid GPIO, rollback changes and don't save settings, and report the error to WebUI
+    if (!have_valid_gpios) {
+        // replace settings with original settings
+        settings = original_settings; // the original settings are still valid
+        // restore the GPIOs from the snapshot
+        EMSESP::system_.restore_snapshot_gpios();
+
+        // report the error to WebUI
+        EMSESP::system_.systemStatus(SYSTEM_STATUS::SYSTEM_STATUS_INVALID_GPIO);
+        return StateUpdateResult::ERROR; // don't save the settings if the GPIOs are invalid
+    }
+
+
     // save the setting internally, for reference later
     EMSESP::system_.store_settings(settings);
 
-    // save the settings if changed from the webUI
-    // if we encountered an invalid GPIO, don't save settings and report the error
-    if (!have_valid_gpios) {
-#if defined(EMSESP_DEBUG)
-        EMSESP::logger().debug("Warning: one or more GPIOs are invalid");
-#endif
-        EMSESP::system_.systemStatus(SYSTEM_STATUS::SYSTEM_STATUS_INVALID_GPIO);
-        return StateUpdateResult::CHANGED; // save the settings anyway, without restart
-    }
-
     if (has_flags(WebSettings::ChangeFlags::RESTART)) {
         return StateUpdateResult::CHANGED_RESTART;
     }
@@ -503,7 +513,6 @@ void WebSettings::set_board_profile(WebSettings & settings) {
     settings.led_type       = data[9]; // LED Type
 }
 
-
 // returns true if the value was changed
 bool WebSettings::check_flag(int prev_v, int new_v, uint8_t flag) {
     if (prev_v != new_v) {