diff --git a/esphome/components/improv_serial/__init__.py b/esphome/components/improv_serial/__init__.py index 11e9f1ea62..a34e2ab793 100644 --- a/esphome/components/improv_serial/__init__.py +++ b/esphome/components/improv_serial/__init__.py @@ -16,7 +16,7 @@ from esphome.types import ConfigType AUTO_LOAD = ["improv_base"] CODEOWNERS = ["@esphome/core"] -DEPENDENCIES = ["logger", "wifi"] +DEPENDENCIES = ["logger", "network"] improv_serial_ns = cg.esphome_ns.namespace("improv_serial") diff --git a/esphome/components/improv_serial/improv_serial_component.cpp b/esphome/components/improv_serial/improv_serial_component.cpp index 0fb18e9b0d..ffa7b79d9b 100644 --- a/esphome/components/improv_serial/improv_serial_component.cpp +++ b/esphome/components/improv_serial/improv_serial_component.cpp @@ -1,5 +1,5 @@ #include "improv_serial_component.h" -#ifdef USE_WIFI +#ifdef USE_IMPROV_SERIAL #include "esphome/core/application.h" #include "esphome/core/defines.h" #include "esphome/core/hal.h" @@ -7,7 +7,10 @@ #include "esphome/core/version.h" #include "esphome/components/logger/logger.h" +#include "esphome/components/network/util.h" +#ifdef USE_WIFI #include "esphome/components/wifi/scan_list.h" +#endif #include @@ -26,13 +29,17 @@ void ImprovSerialComponent::setup() { this->hw_serial_ = logger::global_logger->get_hw_serial(); #endif - if (wifi::global_wifi_component->has_sta()) { + // The Improv state machine tracks Wi-Fi provisioning only. General device + // connectivity (e.g. Ethernet) is reported separately via GET_NETWORK_STATE. +#ifdef USE_WIFI + if (wifi::global_wifi_component != nullptr && wifi::global_wifi_component->has_sta()) { this->state_ = improv::STATE_PROVISIONED; - } else if (!wifi::global_wifi_component->is_disabled()) { + } else if (wifi::global_wifi_component != nullptr && !wifi::global_wifi_component->is_disabled()) { // Respect Wi-Fi's disabled state; forcing a scan while disabled throws // the wifi component into an invalid state from which it cannot recover. wifi::global_wifi_component->start_scanning(); } +#endif } void ImprovSerialComponent::loop() { @@ -55,8 +62,14 @@ void ImprovSerialComponent::loop() { } } - if (this->state_ == improv::STATE_PROVISIONING) { - if (wifi::global_wifi_component->is_connected()) { +#ifdef USE_WIFI + if (this->state_ == improv::STATE_PROVISIONING && wifi::global_wifi_component != nullptr && + wifi::global_wifi_component->is_connected()) { + // Being connected is not enough: re-provisioning a device that is already online leaves the + // prior network up until it drops, so check that the joined network is the requested one + // before reporting success. Same test as the wifi.connect action. + char ssid_buf[wifi::SSID_BUFFER_SIZE]; + if (strcmp(wifi::global_wifi_component->wifi_ssid_to(ssid_buf), this->connecting_sta_.get_ssid().c_str()) == 0) { wifi::global_wifi_component->save_wifi_sta(this->connecting_sta_.get_ssid(), this->connecting_sta_.get_password()); this->connecting_sta_ = {}; @@ -66,6 +79,7 @@ void ImprovSerialComponent::loop() { this->send_settings_response_(improv::WIFI_SETTINGS); } } +#endif } void ImprovSerialComponent::dump_config() { ESP_LOGCONFIG(TAG, "Improv Serial:"); } @@ -143,15 +157,17 @@ void ImprovSerialComponent::write_data_(const uint8_t *data, const size_t size) #endif } -void ImprovSerialComponent::send_settings_response_(improv::Command command) { - std::array buf; - improv::RpcResponseBuilder builder(buf, command); -#ifdef USE_IMPROV_SERIAL_NEXT_URL - this->add_next_url_(builder, MAX_NEXT_URL_LEN); -#endif #ifdef USE_WEBSERVER - for (auto &ip : wifi::global_wifi_component->wifi_sta_ip_addresses()) { - if (ip.is_ip4()) { +void ImprovSerialComponent::add_webserver_urls_(improv::RpcResponseBuilder &builder, [[maybe_unused]] bool wifi_first) { + // The webserver listens on every interface, so advertise each one that has a usable IPv4. + // network::get_ip_addresses() can't be used here: it returns only the highest-priority + // interface's addresses, which are all-unset (0.0.0.0) when e.g. Ethernet has no link while + // the device is online via Wi-Fi, and 0.0.0.0 must not become the advertised URL. OpenThread + // is omitted: it only ever has IPv6 addresses, which cannot form an IPv4 http:// URL. + const auto append_urls = [&builder](const network::IPAddresses &addresses) { + for (const auto &ip : addresses) { + if (!ip.is_ip4() || !ip.is_set()) + continue; char ip_buf[network::IP_ADDRESS_BUFFER_SIZE]; ip.str_to(ip_buf); // "http://" (7) + IP (40) + ":" (1) + port (5) + null (1) = 54 @@ -162,9 +178,43 @@ void ImprovSerialComponent::send_settings_response_(improv::Command command) { if (!builder.add_string(webserver_url, len)) { ESP_LOGW(TAG, "Response full; URL dropped"); } - break; } - } + }; +#ifdef USE_WIFI + // Clients redirect to the first URL, so the interface the client just configured has to lead: + // another interface's address can be on a subnet that client cannot reach. + const auto append_wifi_urls = [&append_urls]() { + if (wifi::global_wifi_component != nullptr) + append_urls(wifi::global_wifi_component->get_ip_addresses()); + }; + if (wifi_first) + append_wifi_urls(); +#endif +#ifdef USE_ETHERNET + if (ethernet::global_eth_component != nullptr) + append_urls(ethernet::global_eth_component->get_ip_addresses()); +#endif +#ifdef USE_MODEM + if (modem::global_modem_component != nullptr) + append_urls(modem::global_modem_component->get_ip_addresses()); +#endif +#ifdef USE_WIFI + if (!wifi_first) + append_wifi_urls(); +#endif +} +#endif // USE_WEBSERVER + +void ImprovSerialComponent::send_settings_response_(improv::Command command) { + std::array buf; + improv::RpcResponseBuilder builder(buf, command); +#ifdef USE_IMPROV_SERIAL_NEXT_URL + this->add_next_url_(builder, MAX_NEXT_URL_LEN); +#endif +#ifdef USE_WEBSERVER + // This response only ever answers Wi-Fi provisioning, so lead with the Wi-Fi URL as it did + // before other interfaces were reported. + this->add_webserver_urls_(builder, /*wifi_first=*/true); #endif this->send_response_(builder.finish(false)); } @@ -231,7 +281,8 @@ bool ImprovSerialComponent::parse_improv_serial_byte_(uint8_t byte) { bool ImprovSerialComponent::parse_improv_payload_(improv::ImprovCommand &command) { switch (command.command) { case improv::WIFI_SETTINGS: { - if (wifi::global_wifi_component->is_disabled()) { +#ifdef USE_WIFI + if (wifi::global_wifi_component == nullptr || wifi::global_wifi_component->is_disabled()) { // Wi-Fi is disabled, so we can't provision. Respond immediately // instead of letting the client wait out its provisioning timeout. ESP_LOGW(TAG, "Wi-Fi is disabled; cannot provision"); @@ -243,21 +294,32 @@ bool ImprovSerialComponent::parse_improv_payload_(improv::ImprovCommand &command sta.set_password(command.password.c_str()); this->connecting_sta_ = sta; + // Sampled before start_connecting(): the old connection drops asynchronously after it. + const bool switching = wifi::global_wifi_component->is_connected(); wifi::global_wifi_component->set_sta(sta); wifi::global_wifi_component->start_connecting(sta); this->set_state_(improv::STATE_PROVISIONING); ESP_LOGD(TAG, "Received settings: SSID=%s, password=" LOG_SECRET("%s"), command.ssid.c_str(), command.password.c_str()); - this->set_timeout("wifi-connect-timeout", 30000, [this]() { this->on_wifi_connect_timeout_(); }); + this->set_timeout("wifi-connect-timeout", switching ? WIFI_SWITCH_TIMEOUT_MS : WIFI_CONNECT_TIMEOUT_MS, + [this]() { this->on_wifi_connect_timeout_(); }); +#else + // No Wi-Fi support compiled in; there is nothing to provision. + ESP_LOGW(TAG, "Wi-Fi not supported; cannot provision"); + this->set_error_(improv::ERROR_UNABLE_TO_CONNECT); +#endif return true; } - case improv::GET_CURRENT_STATE: - if (wifi::global_wifi_component->is_disabled()) { - // Wi-Fi is disabled; report the Improv "stopped" state so a client can tell - // the user that provisioning is unavailable. Reported transiently without - // disturbing our internal provisioning state machine, so a later `wifi.enable` - // still reports the correct state. + case improv::GET_CURRENT_STATE: { + // This state machine tracks Wi-Fi provisioning only. When Wi-Fi is disabled or not + // compiled in, provisioning is unavailable -> report STOPPED so the client doesn't + // offer a Wi-Fi form. General connectivity (e.g. Ethernet) is reported separately + // via GET_NETWORK_STATE. +#ifdef USE_WIFI + if (wifi::global_wifi_component == nullptr || wifi::global_wifi_component->is_disabled()) { + // Reported transiently without disturbing our internal provisioning state machine, + // so a later `wifi.enable` still reports the correct state. this->send_current_state_(improv::STATE_STOPPED); return true; } @@ -265,14 +327,20 @@ bool ImprovSerialComponent::parse_improv_payload_(improv::ImprovCommand &command if (this->state_ == improv::STATE_PROVISIONED) { this->send_settings_response_(improv::GET_CURRENT_STATE); } +#else + this->send_current_state_(improv::STATE_STOPPED); +#endif return true; + } case improv::GET_DEVICE_INFO: { this->send_version_info_(); return true; } case improv::GET_WIFI_NETWORKS: { - const auto &results = wifi::global_wifi_component->get_scan_result(); + // Declared out here because the terminating empty response is sent with or without Wi-Fi std::array buf; +#ifdef USE_WIFI + const auto &results = wifi::global_wifi_component->get_scan_result(); for (const auto &scan : results) { bool with_auth = false; if (!wifi::should_show_scan_entry(results, scan, with_auth)) @@ -289,11 +357,52 @@ bool ImprovSerialComponent::parse_improv_payload_(improv::ImprovCommand &command builder.add_string(YESNO(with_auth)); this->send_response_(builder.finish(false)); } +#endif // USE_WIFI // Send empty response to signify the end of the list. improv::RpcResponseBuilder builder(buf, improv::GET_WIFI_NETWORKS); this->send_response_(builder.finish(false)); return true; } + case improv::GET_NETWORK_STATE: { + // Reports general device connectivity and which network interfaces are present, decoupled + // from the Wi-Fi-only provisioning state machine. data[0] is a decimal flags byte; + // when online, the reachable device URL(s) follow. + uint8_t flags = 0; + if (network::is_connected()) + flags |= improv::NETWORK_IS_ONLINE; +#ifdef USE_WIFI + flags |= improv::NETWORK_SUPPORTS_WIFI; +#endif +#ifdef USE_ETHERNET + flags |= improv::NETWORK_SUPPORTS_ETHERNET; +#endif +#ifdef USE_OPENTHREAD + flags |= improv::NETWORK_SUPPORTS_THREAD; +#endif +#ifdef USE_MODEM + flags |= improv::NETWORK_SUPPORTS_MODEM; +#endif + std::array buf; + improv::RpcResponseBuilder builder(buf, improv::GET_NETWORK_STATE); + // Every flag bit fits int8_t's positive range, so int8_to_str renders the byte + static_assert(improv::NETWORK_SUPPORTS_MODEM <= 0x7F, "network flags no longer fit int8_to_str"); + char flags_buf[4]; // uint8_t: max "255" + null + char *flags_end = int8_to_str(flags_buf, static_cast(flags)); + builder.add_string(flags_buf, flags_end - flags_buf); +#ifdef USE_WEBSERVER + // Not tied to one interface, so follow the configured priority the way + // network::get_ip_addresses() does: a wifi-first network priority list leads with Wi-Fi. + if (flags & improv::NETWORK_IS_ONLINE) { +#if defined(USE_NETWORK_PRIMARY_INTERFACE_WIFI) && defined(USE_WIFI) + this->add_webserver_urls_(builder, /*wifi_first=*/true); +#else + this->add_webserver_urls_(builder, /*wifi_first=*/false); +#endif + } +#endif + this->send_response_(builder.finish(false)); + return true; + } default: { ESP_LOGW(TAG, "Unknown payload"); this->set_error_(improv::ERROR_UNKNOWN_RPC); @@ -331,12 +440,14 @@ void ImprovSerialComponent::send_response_(std::span response) { this->write_data_(response.data(), response.size()); } +#ifdef USE_WIFI void ImprovSerialComponent::on_wifi_connect_timeout_() { this->set_error_(improv::ERROR_UNABLE_TO_CONNECT); this->set_state_(improv::STATE_AUTHORIZED); ESP_LOGW(TAG, "Timed out while connecting to Wi-Fi network"); wifi::global_wifi_component->clear_sta(); } +#endif ImprovSerialComponent *global_improv_serial_component = // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) diff --git a/esphome/components/improv_serial/improv_serial_component.h b/esphome/components/improv_serial/improv_serial_component.h index 692873bbb6..68cdd75214 100644 --- a/esphome/components/improv_serial/improv_serial_component.h +++ b/esphome/components/improv_serial/improv_serial_component.h @@ -2,15 +2,19 @@ #include "esphome/components/improv_base/improv_base.h" #include "esphome/components/logger/logger.h" -#include "esphome/components/wifi/wifi_component.h" +#include "esphome/components/network/util.h" #include "esphome/core/component.h" #include "esphome/core/defines.h" #include "esphome/core/helpers.h" -#ifdef USE_WIFI +#ifdef USE_IMPROV_SERIAL #include #include #include +#ifdef USE_WIFI +#include "esphome/components/wifi/wifi_component.h" +#endif + #ifdef USE_IMPROV_SERIAL_UART #include "esphome/components/uart/uart_component.h" #elif defined(USE_ESP32) @@ -48,13 +52,22 @@ enum ImprovSerialType : uint8_t { static const uint16_t IMPROV_SERIAL_TIMEOUT = 100; static const uint8_t IMPROV_SERIAL_VERSION = 1; +#ifdef USE_WIFI +// Wi-Fi connect failure timers: a fresh provision reports at 30 s (stock behavior), while +// switching networks on an already-connected device (disconnect + reconnect) can legitimately +// take longer; 90 s matches esp32_improv's default wifi_timeout. +static const uint32_t WIFI_CONNECT_TIMEOUT_MS = 30000; +static const uint32_t WIFI_SWITCH_TIMEOUT_MS = 90000; +#endif + // The serial frame length field is one byte static constexpr size_t MAX_SERIAL_RESPONSE = 255; // command + data length + trailing byte static constexpr size_t RPC_RESPONSE_OVERHEAD = 3; static constexpr size_t MAX_SERIAL_PAYLOAD = MAX_SERIAL_RESPONSE - RPC_RESPONSE_OVERHEAD; #ifdef USE_WEBSERVER -// length byte + "http://" + IPv4 + ":" + port +// length byte + "http://" + IPv4 + ":" + port. Reserves the first URL only; a device with +// several interfaces online adds the rest best-effort and warns if one no longer fits. static constexpr size_t WEBSERVER_URL_RESERVE = 1 + 7 + 15 + 1 + 5; #else static constexpr size_t WEBSERVER_URL_RESERVE = 0; @@ -84,8 +97,15 @@ class ImprovSerialComponent final : public Component, public improv_base::Improv void send_current_state_(improv::State state); void set_error_(improv::Error error); void send_response_(std::span response); +#ifdef USE_WIFI void on_wifi_connect_timeout_(); +#endif +#ifdef USE_WEBSERVER + /// Append one web server URL per interface that has a usable IPv4. With wifi_first the Wi-Fi + /// URL leads, for responses to Wi-Fi provisioning; otherwise interfaces go in priority order. + void add_webserver_urls_(improv::RpcResponseBuilder &builder, [[maybe_unused]] bool wifi_first); +#endif void send_settings_response_(improv::Command command); void send_version_info_(); @@ -167,7 +187,9 @@ class ImprovSerialComponent final : public Component, public improv_base::Improv std::vector rx_buffer_; uint32_t last_read_byte_{0}; +#ifdef USE_WIFI wifi::WiFiAP connecting_sta_; +#endif improv::State state_{improv::STATE_AUTHORIZED}; }; diff --git a/tests/components/improv_serial/common-ethernet.yaml b/tests/components/improv_serial/common-ethernet.yaml new file mode 100644 index 0000000000..c1d8190c13 --- /dev/null +++ b/tests/components/improv_serial/common-ethernet.yaml @@ -0,0 +1,17 @@ +ethernet: + type: W5500 + clk_pin: 19 + mosi_pin: 21 + miso_pin: 17 + cs_pin: 18 + interrupt_pin: 36 + reset_pin: 12 + clock_speed: 10Mhz + +logger: + hardware_uart: UART0 + +# Exercises the per-interface webserver URL collection at compile time +web_server: + +improv_serial: diff --git a/tests/components/improv_serial/test-ethernet.esp32-idf.yaml b/tests/components/improv_serial/test-ethernet.esp32-idf.yaml new file mode 100644 index 0000000000..2dd3a1551e --- /dev/null +++ b/tests/components/improv_serial/test-ethernet.esp32-idf.yaml @@ -0,0 +1,2 @@ +packages: + improv_serial: !include common-ethernet.yaml diff --git a/tests/integration/fixtures/external_components/wifi/wifi_component.cpp b/tests/integration/fixtures/external_components/wifi/wifi_component.cpp index d1e19a1a0a..b29b1a2bd1 100644 --- a/tests/integration/fixtures/external_components/wifi/wifi_component.cpp +++ b/tests/integration/fixtures/external_components/wifi/wifi_component.cpp @@ -29,6 +29,8 @@ void WiFiComponent::set_sta(const WiFiAP &ap) { ESP_LOGI(TAG, "set_sta ssid=%s", void WiFiComponent::start_connecting(const WiFiAP &ap) { ESP_LOGI(TAG, "start_connecting ssid=%s", ap.get_ssid().c_str()); + // Connecting succeeds immediately, so the requested network is the connected one + this->connected_ssid_ = ap.get_ssid().c_str(); } void WiFiComponent::clear_sta() { ESP_LOGI(TAG, "clear_sta"); } diff --git a/tests/integration/fixtures/external_components/wifi/wifi_component.h b/tests/integration/fixtures/external_components/wifi/wifi_component.h index a68f811ebd..6fe6e84e9d 100644 --- a/tests/integration/fixtures/external_components/wifi/wifi_component.h +++ b/tests/integration/fixtures/external_components/wifi/wifi_component.h @@ -13,11 +13,15 @@ #include "esphome/core/component.h" #include "esphome/core/string_ref.h" +#include +#include #include #include namespace esphome::wifi { +static constexpr size_t SSID_BUFFER_SIZE = 33; + class WiFiAP { public: void set_ssid(const char *ssid) { this->ssid_ = ssid; } @@ -58,6 +62,12 @@ class WiFiComponent : public Component { bool is_disabled() const { return false; } // Always connected so network::is_connected() keeps the API server accepting clients bool is_connected() const { return true; } + // Reports the network start_connecting() was last asked for, so a consumer checking that it + // joined the network it requested (rather than an earlier one) sees the connect succeed + const char *wifi_ssid_to(std::span buffer) { + snprintf(buffer.data(), buffer.size(), "%s", this->connected_ssid_.c_str()); + return buffer.data(); + } void start_scanning(); const std::vector &get_scan_result() const { return this->scan_result_; } void set_sta(const WiFiAP &ap); @@ -70,6 +80,7 @@ class WiFiComponent : public Component { protected: std::vector scan_result_; + std::string connected_ssid_; }; extern WiFiComponent *global_wifi_component; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)