From a9aea047e68ede9cd531ac63b5c0ac27d2c1c7e3 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sat, 3 Oct 2026 23:57:24 -0500 Subject: [PATCH 01/21] feat(desktop): Kconfig-gated CANopen/DS402, I2C scanner and Network apps for the desktop example Three hardware apps for the desktop example, each behind an option in the "Desktop Example Configuration" menu (all on by default so the esp32s3 CI build compiles every one of them): - CANopen / DS402 (DESKTOP_EXAMPLE_ENABLE_CANOPEN): espp::CanopenClient + espp::Ds402Drive on either the can_bridge example's simulated DS402 node (CANOPEN_BUS_SIMULATED, the default) or the TWAI peripheral (CANOPEN_BUS_TWAI + TX/RX GPIO + bit rate). Node id + Apply, NMT buttons, NMT / drive state / statusword labels, mode select, enable / disable / quick stop / fault reset, target-velocity slider, position / velocity, an SDO read / write row and (simulated) an Inject fault button. Every bus transaction runs on the app's own task; handlers only queue commands. - I2C scanner (DESKTOP_EXAMPLE_ENABLE_I2C + port / SDA / SCL / clock): probe 0x01..0x7F on a short task into a table; register read / write row; a failed bus init shows a hint. - Network (DESKTOP_EXAMPLE_ENABLE_WIFI, depends on SOC_WIFI_SUPPORTED; DESKTOP_EXAMPLE_ENABLE_ETHERNET, default n, depends on SOC_EMAC_SUPPORTED): Wi-Fi station status / scan (own task) / AP list / password / connect / disconnect / forget with NVS credentials; RMII Ethernet link labels. The hardware components are always part of the build (REQUIRES cannot depend on Kconfig); the simulated bus headers come from components/canopen/can_bridge_example/main (promoting them into the canopen component is a follow-up). READMEs updated with the app list and a Kconfig table. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- components/desktop/README.md | 4 +- components/desktop/example/CMakeLists.txt | 8 +- components/desktop/example/README.md | 55 +++ .../desktop/example/main/CMakeLists.txt | 11 +- .../desktop/example/main/Kconfig.projbuild | 94 ++++ .../desktop/example/main/apps/canopen_app.hpp | 451 ++++++++++++++++++ .../example/main/apps/i2c_scanner_app.hpp | 198 ++++++++ .../desktop/example/main/apps/network_app.hpp | 312 ++++++++++++ .../desktop/example/main/desktop_example.cpp | 23 +- 9 files changed, 1151 insertions(+), 5 deletions(-) create mode 100644 components/desktop/example/main/apps/canopen_app.hpp create mode 100644 components/desktop/example/main/apps/i2c_scanner_app.hpp create mode 100644 components/desktop/example/main/apps/network_app.hpp diff --git a/components/desktop/README.md b/components/desktop/README.md index 345b53eb86..443355b7da 100644 --- a/components/desktop/README.md +++ b/components/desktop/README.md @@ -193,4 +193,6 @@ and covered by the host test. Mutually exclusive with The [example](./example) serves the desktop with the Counter, About, System Monitor, Task Manager, Log Viewer, Files + Editor and Settings apps on the native USB port of an ESP32-S3 (WebUSB + Web Serial), next to the standard -System / Monitor / OTA / CoreDump services. +System / Monitor / OTA / CoreDump services, plus three Kconfig-gated hardware +apps: CANopen / DS402 (a simulated node or the TWAI peripheral), an I2C +scanner and Network (Wi-Fi station, Ethernet). diff --git a/components/desktop/example/CMakeLists.txt b/components/desktop/example/CMakeLists.txt index ae118d34b1..8b23022922 100644 --- a/components/desktop/example/CMakeLists.txt +++ b/components/desktop/example/CMakeLists.txt @@ -21,11 +21,15 @@ include($ENV{IDF_PATH}/tools/cmake/project.cmake) set(EXTRA_COMPONENT_DIRS "../../../components/base_component" + "../../../components/canopen" + "../../../components/cli" "../../../components/coredump" "../../../components/desktop" "../../../components/dispatcher" + "../../../components/ethernet" "../../../components/file_system" "../../../components/format" + "../../../components/i2c" "../../../components/logger" "../../../components/monitor" "../../../components/nvs" @@ -33,7 +37,9 @@ set(EXTRA_COMPONENT_DIRS "../../../components/stream_frame" "../../../components/system" "../../../components/task" + "../../../components/twai" "../../../components/usb_device" + "../../../components/wifi" ) if(DEFINED ENV{IDF_COMPONENT_MANAGER} AND "$ENV{IDF_COMPONENT_MANAGER}" STREQUAL "0") @@ -46,7 +52,7 @@ endif() set( COMPONENTS - "main esptool_py base_component desktop dispatcher file_system format logger monitor nvs stream_frame system task usb_device esp_tinyusb esp_psram coredump ota espcoredump littlefs" + "main esptool_py base_component desktop dispatcher file_system format logger monitor nvs stream_frame system task usb_device esp_tinyusb esp_psram coredump ota espcoredump littlefs canopen twai i2c wifi ethernet cli" CACHE STRING "List of components to include" ) diff --git a/components/desktop/example/README.md b/components/desktop/example/README.md index b0325503d6..81f81f9679 100644 --- a/components/desktop/example/README.md +++ b/components/desktop/example/README.md @@ -30,6 +30,31 @@ Apps (`main/apps/*.hpp`, one `register__app()` each): console; the capture itself is the compile-time `CONFIG_DESKTOP_EXAMPLE_LOG_CAPTURE`), all kept in NVS and restored at boot. +Hardware apps, each behind a Kconfig option (all on by default, so the CI +build compiles every one of them; see [Configuration](#configuration)): + +- **CANopen / DS402** — a CiA 301 NMT master + SDO client + (`espp::CanopenClient`) and a CiA 402 drive panel (`espp::Ds402Drive`): + node id, NMT Start / Stop / Pre-operational / Reset, NMT + drive state and + statusword, mode of operation, Enable / Disable / Quick stop / Fault reset, + a target-velocity slider, position / velocity, and a raw SDO read / write + row. The bus is either the in-firmware **simulated DS402 node** (the CAN + bridge example's `SimulatedCanBus`, no hardware needed, with an "Inject + fault" button) or the **TWAI peripheral** wired to a CAN transceiver. Every + bus transaction runs on the app's own task (SDO calls block); the window + only queues commands and the task updates the widgets. +- **I2C scanner** — probe every 7-bit address on the configured bus + (`espp::I2c`) from a short task and list what answers; read / write a + device register from the window. A bus that fails to initialize shows a + hint instead. +- **Network** — the Wi-Fi station (`espp::WifiSta`): status / SSID / IP / + RSSI / MAC, Scan (on its own task; a scan disconnects first), the AP list, + password field and Connect / Disconnect / Forget with the credentials kept + in NVS (`desktop` namespace, `wifi_ssid` / `wifi_pass`); and, on SoCs with an + EMAC, the RMII Ethernet link (`espp::Ethernet`): link / IP / MAC / speed. + The interfaces come up on the first launch and stay up when the window is + closed. + ## How to use example ### Hardware Required @@ -39,6 +64,36 @@ console / logs go to UART0 (see `sdkconfig.defaults`); with `CONFIG_DESKTOP_EXAMPLE_LOG_CAPTURE` (default on) they are also captured for the Log Viewer. +The hardware apps need nothing extra by default: the CANopen app talks to a +simulated node, the I2C scanner just reports an empty bus and the Network app +scans for Wi-Fi. For a real CAN bus select the TWAI peripheral and wire a +transceiver (SN65HVD230 or similar) to the configured TX / RX GPIOs; for the +Ethernet group an ESP32-Ethernet-Kit-style RMII PHY (the pins are in +`main/apps/network_app.hpp`). + +### Configuration + +`idf.py menuconfig` -> *Desktop Example Configuration*: + +| Option | Default | Meaning | +|---|---|---| +| `DESKTOP_EXAMPLE_LOG_CAPTURE` (+ `_BYTES`) | y (16384) | Tee the console into a ring for the Log Viewer | +| `DESKTOP_EXAMPLE_ENABLE_CANOPEN` | y | Register the CANopen / DS402 app | +| `DESKTOP_EXAMPLE_CANOPEN_BUS` | `SIMULATED` | `SIMULATED` (in-firmware DS402 node) or `TWAI` (the peripheral) | +| `DESKTOP_EXAMPLE_CANOPEN_NODE_ID` | 1 | Server node id (1..127; also the simulated node's id) | +| `DESKTOP_EXAMPLE_CAN_TX_GPIO` / `_RX_GPIO` / `_BAUDRATE` | 17 / 16 / 500000 | TWAI wiring and bit rate (TWAI bus only) | +| `DESKTOP_EXAMPLE_ENABLE_I2C` | y | Register the I2C scanner app | +| `DESKTOP_EXAMPLE_I2C_PORT` / `_SDA_GPIO` / `_SCL_GPIO` / `_FREQ_HZ` | 0 / 8 / 9 / 400000 | The I2C bus it scans | +| `DESKTOP_EXAMPLE_ENABLE_WIFI` | y | The Network app's Wi-Fi station group (`SOC_WIFI_SUPPORTED`) | +| `DESKTOP_EXAMPLE_ENABLE_ETHERNET` | n | The Network app's RMII Ethernet group (`SOC_EMAC_SUPPORTED`: ESP32 / -P4) | + +The hardware components (`canopen`, `twai`, `i2c`, `wifi`, `ethernet`, `cli`) +are always part of the build (`REQUIRES` cannot depend on Kconfig); the +options only decide which apps are registered. The simulated CAN bus / +DS402 node headers are included from the CAN bridge example +(`components/canopen/can_bridge_example/main`); promoting them into the +`canopen` component is a follow-up. + ### Build and Flash ``` diff --git a/components/desktop/example/main/CMakeLists.txt b/components/desktop/example/main/CMakeLists.txt index 3d6d6ccd8a..cfaed2613b 100644 --- a/components/desktop/example/main/CMakeLists.txt +++ b/components/desktop/example/main/CMakeLists.txt @@ -1,4 +1,11 @@ idf_component_register( SRC_DIRS "." - INCLUDE_DIRS "." "apps" - REQUIRES desktop system monitor file_system nvs dispatcher task usb_device esp_tinyusb coredump ota espcoredump) + # The CANopen app's simulated bus / DS402 node live in the can_bridge example + # for now (simulated_can_bus.hpp, simulated_ds402_node.hpp); promoting them + # into the canopen component is a follow-up. + INCLUDE_DIRS "." "apps" "../../../canopen/can_bridge_example/main" + # Every hardware component is required unconditionally (REQUIRES cannot + # depend on Kconfig); the apps themselves are gated by + # CONFIG_DESKTOP_EXAMPLE_ENABLE_* in desktop_example.cpp. + REQUIRES desktop system monitor file_system nvs dispatcher task usb_device esp_tinyusb coredump ota espcoredump + canopen twai i2c wifi ethernet) diff --git a/components/desktop/example/main/Kconfig.projbuild b/components/desktop/example/main/Kconfig.projbuild index 267be8630b..ef95d573b2 100644 --- a/components/desktop/example/main/Kconfig.projbuild +++ b/components/desktop/example/main/Kconfig.projbuild @@ -18,4 +18,98 @@ menu "Desktop Example Configuration" How many of the newest console bytes are kept for the Log Viewer (and shown when it opens). + config DESKTOP_EXAMPLE_ENABLE_CANOPEN + bool "CANopen / DS402 app" + default y + help + Register the CANopen / DS402 app: an NMT master + SDO client + (espp::CanopenClient) and a CiA 402 drive panel (espp::Ds402Drive) + on the bus selected below. + + choice DESKTOP_EXAMPLE_CANOPEN_BUS + prompt "CAN bus for the CANopen app" + depends on DESKTOP_EXAMPLE_ENABLE_CANOPEN + default DESKTOP_EXAMPLE_CANOPEN_BUS_SIMULATED + help + Where the CANopen app's frames go: an in-firmware simulated DS402 + node (no hardware needed; the CI default) or the TWAI peripheral + wired to a CAN transceiver. + + config DESKTOP_EXAMPLE_CANOPEN_BUS_SIMULATED + bool "Simulated DS402 node (no CAN hardware)" + config DESKTOP_EXAMPLE_CANOPEN_BUS_TWAI + bool "TWAI peripheral (CAN transceiver on the GPIOs below)" + endchoice + + config DESKTOP_EXAMPLE_CANOPEN_NODE_ID + int "CANopen node id" + depends on DESKTOP_EXAMPLE_ENABLE_CANOPEN + default 1 + range 1 127 + help + The server node id the app talks to at launch (changeable in the + window). With the simulated bus this is also the simulated node's id. + + config DESKTOP_EXAMPLE_CAN_TX_GPIO + int "TWAI TX GPIO" + depends on DESKTOP_EXAMPLE_CANOPEN_BUS_TWAI + default 17 + + config DESKTOP_EXAMPLE_CAN_RX_GPIO + int "TWAI RX GPIO" + depends on DESKTOP_EXAMPLE_CANOPEN_BUS_TWAI + default 16 + + config DESKTOP_EXAMPLE_CAN_BAUDRATE + int "CAN bit rate (bit/s)" + depends on DESKTOP_EXAMPLE_CANOPEN_BUS_TWAI + default 500000 + range 10000 1000000 + + config DESKTOP_EXAMPLE_ENABLE_I2C + bool "I2C scanner app" + default y + help + Register the I2C scanner app: probe every 7-bit address on the + bus below, and read / write device registers. + + config DESKTOP_EXAMPLE_I2C_PORT + int "I2C port" + depends on DESKTOP_EXAMPLE_ENABLE_I2C + default 0 + range 0 1 + + config DESKTOP_EXAMPLE_I2C_SDA_GPIO + int "I2C SDA GPIO" + depends on DESKTOP_EXAMPLE_ENABLE_I2C + default 8 + + config DESKTOP_EXAMPLE_I2C_SCL_GPIO + int "I2C SCL GPIO" + depends on DESKTOP_EXAMPLE_ENABLE_I2C + default 9 + + config DESKTOP_EXAMPLE_I2C_FREQ_HZ + int "I2C clock (Hz)" + depends on DESKTOP_EXAMPLE_ENABLE_I2C + default 400000 + range 1000 1000000 + + config DESKTOP_EXAMPLE_ENABLE_WIFI + bool "Network app: Wi-Fi station" + depends on SOC_WIFI_SUPPORTED + default y + help + Register the Network app's Wi-Fi group: status, scan, connect / + disconnect with the credentials kept in NVS (espp::WifiSta). + + config DESKTOP_EXAMPLE_ENABLE_ETHERNET + bool "Network app: Ethernet (RMII)" + depends on SOC_EMAC_SUPPORTED + default n + help + Register the Network app's Ethernet group: link / IP / MAC / speed + of the internal EMAC over RMII (espp::Ethernet, ESP32-Ethernet-Kit + pins in network_app.hpp). + endmenu diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp new file mode 100644 index 0000000000..bf61121d4f --- /dev/null +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -0,0 +1,451 @@ +#pragma once + +// CANopen / DS402: an NMT master + SDO client (espp::CanopenClient) and a CiA +// 402 drive panel (espp::Ds402Drive) on the bus Kconfig selects -- the +// in-firmware simulated DS402 node (the can_bridge example's SimulatedCanBus) +// or the TWAI peripheral. SDO calls block (the bus delivers the responses from +// its own receive task), so every bus transaction runs on the app's drive +// task: the widget handlers only queue commands, and the drive task polls the +// statusword / position / velocity and updates the widgets from there (any +// task may call the mutators). + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "sdkconfig.h" + +#include "canopen_client.hpp" +#include "desktop.hpp" +#include "ds402.hpp" +#include "task.hpp" +#if CONFIG_DESKTOP_EXAMPLE_CANOPEN_BUS_SIMULATED +#include "simulated_can_bus.hpp" +#else +#include "twai.hpp" +#endif + +namespace desktop_example { + +#if CONFIG_DESKTOP_EXAMPLE_CANOPEN_BUS_SIMULATED +using CanBus = can_bridge::SimulatedCanBus; +#else +using CanBus = espp::Twai; +#endif + +/// One CANopen window's bus, client, drive and drive task. Rebuilt in place +/// (on the drive task) when the node id changes; shut down from the window's +/// on_close, which runs on the desktop task with no lock held. +struct CanopenSession { + using D = espp::Desktop; + using CanFrame = espp::CanopenClient::CanFrame; + using NmtState = espp::CanopenClient::NmtState; + + explicit CanopenSession(D &desktop) + : desktop(desktop) {} + ~CanopenSession() { shutdown(); } + + D &desktop; + D::Widget nmt_label, bus_label, state_label, status_label, mode_label, pos_label, vel_label, + sdo_result; + uint8_t node_id{1}; + // destroyed in reverse order: task (stopped first), then the bus (its + // receive task feeds the client), then the drive (references the client) + std::unique_ptr client; + std::unique_ptr drive; + std::unique_ptr bus; + std::mutex mutex; + std::condition_variable cv; + std::deque> queue; + bool stopping{false}; + std::unique_ptr task; + + static const char *nmt_name(NmtState s) { + switch (s) { + case NmtState::BootUp: + return "boot-up"; + case NmtState::Stopped: + return "stopped"; + case NmtState::Operational: + return "operational"; + case NmtState::PreOperational: + return "pre-operational"; + default: + return "unknown"; + } + } + + /// (Re)create the bus, client and drive for `id`. Called at launch (before + /// the drive task runs) and from the drive task afterwards. + bool open(uint8_t id, std::error_code &ec) { + bus.reset(); // first: nothing feeds the client while it is replaced + drive.reset(); + client.reset(); + node_id = id; + client = std::make_unique(espp::CanopenClient::Config{ + .node_id = id, + .send = + [this](const CanFrame &f) { + if (!bus) + return false; + CanBus::Message m{ + .id = f.id, .extended = f.extended, .rtr = f.rtr, .dlc = f.dlc, .data = f.data}; + std::error_code tx_ec; + return bus->transmit(m, tx_ec); + }, + .on_heartbeat = + [this](uint8_t node, NmtState s) { // bus task + if (node == node_id) + nmt_label.set_text("NMT state: {}", nmt_name(s)); + }, + }); + drive = std::make_unique(*client); + CanBus::Config cfg; +#if CONFIG_DESKTOP_EXAMPLE_CANOPEN_BUS_SIMULATED + cfg.node_id = id; +#else + cfg.tx_gpio = CONFIG_DESKTOP_EXAMPLE_CAN_TX_GPIO; + cfg.rx_gpio = CONFIG_DESKTOP_EXAMPLE_CAN_RX_GPIO; + cfg.baudrate = CONFIG_DESKTOP_EXAMPLE_CAN_BAUDRATE; +#endif + cfg.on_receive = [this](const CanBus::Message &m) { // bus receive task, never SDO here + if (client) + client->process_frame( + {.id = m.id, .extended = m.extended, .rtr = m.rtr, .dlc = m.dlc, .data = m.data}); + }; + bus = std::make_unique(cfg); + if (!bus->initialize(ec)) { + bus.reset(); + return false; + } + nmt_label.set_text("NMT state: (nothing heard from node {} yet)", id); + return true; + } + + /// Queue a bus transaction for the drive task. + void run(std::function fn) { + { + std::lock_guard lock(mutex); + queue.push_back(std::move(fn)); + } + cv.notify_all(); + } + + /// `what` failed with `ec`: a toast (level error). + void fail(std::string_view what, const std::error_code &ec) { + std::string text = fmt::format("{}: {}", what, ec.message()); + if (client && client->last_abort_code()) + text += fmt::format(" (SDO abort 0x{:08X}: {})", client->last_abort_code(), + espp::CanopenClient::abort_code_to_string(client->last_abort_code())); + desktop.notify({.title = "CANopen", .text = text, .level = D::NotifyLevel::Error}); + } + + void start() { + task = std::make_unique( + espp::Task::Config{.callback = [this]() { return step(); }, + .task_config = {.name = "canopen_ui", .stack_size_bytes = 6 * 1024}}); + task->start(); + } + + void shutdown() { + { + std::lock_guard lock(mutex); + stopping = true; + queue.clear(); // queued commands hold a reference to this session + } + cv.notify_all(); + task.reset(); // joins: the step returns within one poll period + bus.reset(); + drive.reset(); + client.reset(); + } + + /// One drive-task iteration: a queued command, or (every 250 ms) a poll. + bool step() { + using namespace std::chrono_literals; + std::function fn; + { + std::unique_lock lock(mutex); + cv.wait_for(lock, 250ms, [this] { return stopping || !queue.empty(); }); + if (stopping) + return true; + if (!queue.empty()) { + fn = std::move(queue.front()); + queue.pop_front(); + } + } + if (fn) + fn(); + else + poll(); + return false; + } + + void poll() { + if (!bus || !drive) + return; + std::error_code ec; + const uint16_t sw = drive->get_statusword(ec); + if (ec) { + state_label.set_text("State: no response from node {} ({})", node_id, ec.message()); + return; + } + state_label.set_text("State: {}", + espp::Ds402Drive::to_string(espp::Ds402Drive::state_from_statusword(sw))); + status_label.set_text("Statusword: 0x{:04X}", sw); + const int8_t mode = drive->get_mode_display(ec); + if (!ec) + mode_label.set_text("Mode (0x6061): {}", mode); + const int32_t pos = drive->get_position_actual(ec); + if (!ec) + pos_label.set_text("Position: {}", pos); + const int32_t vel = drive->get_velocity_actual(ec); + if (!ec) + vel_label.set_text("Velocity: {}", vel); + } +}; + +} // namespace desktop_example + +inline void register_canopen_app(espp::Desktop &desktop) { + desktop.register_app({ + .name = "CANopen", + .icon = "\xF0\x9F\x94\x8C", // electric plug + .description = "CiA 301 NMT / SDO master and a CiA 402 drive panel", + .launch = + [](espp::Desktop &d, espp::Desktop::AppId app) { + using D = espp::Desktop; + using Mode = espp::Ds402Drive::OperatingMode; + auto st = std::make_shared(d); + // on_close owns the blocking teardown (it runs on the desktop task + // with no lock held, after the widgets -- and their handler copies + // of `st` -- are gone) + auto win = d.create_window( + {.title = "CANopen / DS402", .app = app, .w = 560, .h = 0, .on_close = [st]() { + st->shutdown(); + }}); + auto parse = [](const std::string &s, int base) { + return static_cast(std::strtoul(s.c_str(), nullptr, base)); + }; + + // ---- bus / node ---- + auto top = win.row(); + win.label("Node id", top.id()); + auto node_box = win.textbox(fmt::format("{}", CONFIG_DESKTOP_EXAMPLE_CANOPEN_NODE_ID), + nullptr, top.id(), "1-127"); + node_box.set_size(60, 0); + win.button( + "Apply", + [=]() mutable { + const uint32_t id = parse(node_box.text(), 10); + if (id < 1 || id > 127) { + st->desktop.notify({.title = "CANopen", + .text = "node id must be 1..127", + .level = D::NotifyLevel::Error}); + return; + } + st->run([=]() { + std::error_code ec; + if (!st->open(static_cast(id), ec)) + st->fail("bus init", ec); + else + st->desktop.notify({.title = "CANopen", + .text = fmt::format("talking to node {}", id), + .level = D::NotifyLevel::Ok}); + }); + }, + top.id()); +#if CONFIG_DESKTOP_EXAMPLE_CANOPEN_BUS_SIMULATED + st->bus_label = win.label("simulated DS402 node", top.id()); +#else + st->bus_label = win.label(fmt::format("TWAI tx {} rx {} @ {} bit/s", + CONFIG_DESKTOP_EXAMPLE_CAN_TX_GPIO, + CONFIG_DESKTOP_EXAMPLE_CAN_RX_GPIO, + CONFIG_DESKTOP_EXAMPLE_CAN_BAUDRATE), + top.id()); +#endif + + // ---- NMT ---- + auto nmt = win.group("NMT"); + auto nmt_row = win.row(nmt.id()); + using NmtFn = bool (espp::CanopenClient::*)(std::error_code &); + auto nmt_button = [=](std::string_view text, NmtFn fn) mutable { + win.button( + text, + [=]() { + st->run([=]() { + std::error_code ec; + if (!st->client || !((*st->client).*fn)(ec)) + st->fail(fmt::format("NMT {}", text), ec); + }); + }, + nmt_row.id()); + }; + nmt_button("Start", &espp::CanopenClient::nmt_start); + nmt_button("Stop", &espp::CanopenClient::nmt_stop); + nmt_button("Pre-operational", &espp::CanopenClient::nmt_pre_operational); + nmt_button("Reset node", &espp::CanopenClient::nmt_reset_node); + st->nmt_label = win.label("NMT state: -", nmt.id(), D::kLabelMonospace); + + // ---- drive ---- + auto drv = win.group("Drive (CiA 402)"); + st->state_label = win.label("State: -", drv.id(), D::kLabelBold); + auto info = win.row(drv.id()); + st->status_label = win.label("Statusword: -", info.id(), D::kLabelMonospace); + st->mode_label = win.label("Mode (0x6061): -", info.id(), D::kLabelMonospace); + auto mode_row = win.row(drv.id()); + win.label("Mode", mode_row.id()); + const std::vector modes = {Mode::ProfilePosition, Mode::ProfileVelocity, + Mode::ProfileTorque, Mode::Homing}; + win.select( + {"Profile position", "Profile velocity", "Profile torque", "Homing"}, 1, + [=](int32_t i) { + if (i < 0 || static_cast(i) >= modes.size()) + return; + const Mode m = modes[static_cast(i)]; + st->run([=]() { + std::error_code ec; + if (!st->drive || !st->drive->set_mode(m, ec)) + st->fail("set mode", ec); + }); + }, + mode_row.id()); + auto ctl = win.row(drv.id()); + using DriveFn = bool (espp::Ds402Drive::*)(std::error_code &); + auto drive_button = [=](std::string_view text, DriveFn fn, uint16_t flags = 0) mutable { + win.button( + text, + [=]() { + st->run([=]() { + std::error_code ec; + if (!st->drive || !((*st->drive).*fn)(ec)) + st->fail(text, ec); + else + st->desktop.notify({.title = "CANopen", + .text = fmt::format("{}: ok", text), + .level = D::NotifyLevel::Ok, + .timeout = std::chrono::milliseconds(1500)}); + }); + }, + ctl.id(), flags); + }; + drive_button("Enable", &espp::Ds402Drive::enable_operation, D::kButtonPrimary); + drive_button("Disable", &espp::Ds402Drive::disable); + drive_button("Quick stop", &espp::Ds402Drive::quick_stop, D::kButtonDanger); + drive_button("Fault reset", &espp::Ds402Drive::fault_reset); +#if CONFIG_DESKTOP_EXAMPLE_CANOPEN_BUS_SIMULATED + win.button( + "Inject fault", + [=]() { + st->run([=]() { // manufacturer object 0x2000 of the simulated node + std::error_code ec; + if (!st->client || !st->client->write_u8(0x2000, 0, 1, ec)) + st->fail("inject fault", ec); + }); + }, + ctl.id()); +#endif + + // ---- motion ---- + auto motion = win.group("Motion"); + auto vel_row = win.row(motion.id()); + win.label("Target velocity", vel_row.id()); + auto vel_value = win.label("0", vel_row.id(), D::kLabelMonospace); + auto slider = win.slider( + 0, -1000, 1000, [=](int32_t v) mutable { vel_value.set_text("{}", v); }, + vel_row.id(), 10); + win.button( + "Apply", + [=]() { + const int32_t v = slider.value(); + st->run([=]() { + std::error_code ec; + if (!st->drive || !st->drive->set_target_velocity(v, ec)) + st->fail("target velocity", ec); + }); + }, + vel_row.id(), D::kButtonPrimary); + auto actual = win.row(motion.id()); + st->pos_label = win.label("Position: -", actual.id(), D::kLabelMonospace); + st->vel_label = win.label("Velocity: -", actual.id(), D::kLabelMonospace); + + // ---- SDO ---- + auto sdo = win.group("SDO"); + auto sdo_row = win.row(sdo.id()); + auto idx_box = win.textbox("6041", nullptr, sdo_row.id(), "index (hex)"); + idx_box.set_size(70, 0); + auto sub_box = win.textbox("0", nullptr, sdo_row.id(), "sub (hex)"); + sub_box.set_size(50, 0); + auto width = win.select({"u8", "u16", "u32"}, 1, nullptr, sdo_row.id()); + auto val_box = win.textbox("0", nullptr, sdo_row.id(), "value (0x.. or decimal)"); + val_box.set_size(110, 0); + st->sdo_result = win.label("", sdo.id(), D::kLabelMonospace | D::kLabelWrap); + win.button( + "Read", + [=]() { + const uint16_t index = static_cast(parse(idx_box.text(), 16)); + const uint8_t sub = static_cast(parse(sub_box.text(), 16)); + st->run([=]() { + std::error_code ec; + std::array buf{}; + const size_t n = st->client ? st->client->sdo_upload(index, sub, buf, ec) : 0; + if (!n) { + st->sdo_result.set_text("read 0x{:04X}:{:02X} failed: {}", index, sub, + ec.message()); + st->fail("SDO read", ec); + return; + } + const uint32_t v = espp::detail::canopen::get_le(buf.data(), n); + st->sdo_result.set_text("0x{:04X}:{:02X} = 0x{:0{}X} ({}) [{} bytes]", index, + sub, v, n * 2, v, n); + }); + }, + sdo_row.id()); + win.button( + "Write", + [=]() { + const uint16_t index = static_cast(parse(idx_box.text(), 16)); + const uint8_t sub = static_cast(parse(sub_box.text(), 16)); + const uint32_t value = parse(val_box.text(), 0); + const int32_t w = width.selected(); + st->run([=]() { + std::error_code ec; + bool ok = false; + if (st->client) { + if (w == 0) + ok = st->client->write_u8(index, sub, static_cast(value), ec); + else if (w == 1) + ok = st->client->write_u16(index, sub, static_cast(value), ec); + else + ok = st->client->write_u32(index, sub, value, ec); + } + if (ok) + st->sdo_result.set_text("0x{:04X}:{:02X} <- 0x{:X} ok", index, sub, value); + else { + st->sdo_result.set_text("write 0x{:04X}:{:02X} failed: {}", index, sub, + ec.message()); + st->fail("SDO write", ec); + } + }); + }, + sdo_row.id()); + + // bring the bus up (quick), then start polling + std::error_code ec; + if (!st->open(CONFIG_DESKTOP_EXAMPLE_CANOPEN_NODE_ID, ec)) { + st->state_label.set_text("State: bus init failed ({}); fix the wiring / Kconfig " + "and press Apply", + ec.message()); + st->fail("bus init", ec); + } + st->start(); + }, + }); +} diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp new file mode 100644 index 0000000000..d601532aca --- /dev/null +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -0,0 +1,198 @@ +#pragma once + +// I2C scanner: probe every 7-bit address on the Kconfig bus (espp::I2c) from +// a short task and list what answers; read / write device registers from the +// window. A bus that fails to initialize shows a hint instead of the tools. + +#include +#include +#include +#include +#include +#include + +#include "sdkconfig.h" + +#include "desktop.hpp" +#include "i2c.hpp" +#include "task.hpp" + +namespace desktop_example { + +struct I2cSession { + std::unique_ptr bus; + std::mutex mutex; + std::vector found; // addresses in table order (scan task writes, handlers read) + std::unique_ptr scan; // one-shot; joined before the bus goes away + + /// Parse "0x3C", "3c" or "60" (hex unless prefixed) into a byte list of + /// space / comma separated values. + static std::vector parse_bytes(const std::string &text) { + std::vector out; + const char *p = text.c_str(); + while (*p) { + while (*p == ' ' || *p == ',') + ++p; + if (!*p) + break; + char *end = nullptr; + const unsigned long v = std::strtoul(p, &end, 16); + if (end == p) + break; + out.push_back(static_cast(v & 0xFF)); + p = end; + } + return out; + } + static uint8_t parse_hex(const std::string &text) { + return static_cast(std::strtoul(text.c_str(), nullptr, 16) & 0x7F); + } + static std::string hex_dump(const std::vector &bytes) { + std::string s; + for (const auto b : bytes) + s += fmt::format("{:02X} ", b); + return s; + } + static const char *note(uint8_t addr) { + if (addr < 0x08 || addr > 0x77) + return "reserved range"; + return ""; + } +}; + +} // namespace desktop_example + +inline void register_i2c_scanner_app(espp::Desktop &desktop) { + desktop.register_app({ + .name = "I2C", + .icon = "\xF0\x9F\x94\x8D", // magnifying glass + .description = fmt::format( + "Scan and poke the I2C bus (port {}, SDA {}, SCL {})", CONFIG_DESKTOP_EXAMPLE_I2C_PORT, + CONFIG_DESKTOP_EXAMPLE_I2C_SDA_GPIO, CONFIG_DESKTOP_EXAMPLE_I2C_SCL_GPIO), + .launch = + [](espp::Desktop &d, espp::Desktop::AppId app) { + using D = espp::Desktop; + using S = desktop_example::I2cSession; + auto st = std::make_shared(); + auto win = d.create_window( + {.title = "I2C scanner", .app = app, .w = 520, .h = 420, .on_close = [st]() { + st->scan.reset(); // joins a scan in flight + st->bus.reset(); + }}); + win.label(fmt::format("Port {}, SDA GPIO {}, SCL GPIO {}, {} kHz", + CONFIG_DESKTOP_EXAMPLE_I2C_PORT, + CONFIG_DESKTOP_EXAMPLE_I2C_SDA_GPIO, + CONFIG_DESKTOP_EXAMPLE_I2C_SCL_GPIO, + CONFIG_DESKTOP_EXAMPLE_I2C_FREQ_HZ / 1000), + 0, D::kLabelMonospace); + st->bus = std::make_unique(espp::I2c::Config{ + .port = static_cast(CONFIG_DESKTOP_EXAMPLE_I2C_PORT), + .sda_io_num = static_cast(CONFIG_DESKTOP_EXAMPLE_I2C_SDA_GPIO), + .scl_io_num = static_cast(CONFIG_DESKTOP_EXAMPLE_I2C_SCL_GPIO), + .sda_pullup_en = GPIO_PULLUP_ENABLE, + .scl_pullup_en = GPIO_PULLUP_ENABLE, + .clk_speed = CONFIG_DESKTOP_EXAMPLE_I2C_FREQ_HZ, + .auto_init = false, + }); + std::error_code ec; + st->bus->init(ec); + if (ec) { + win.label(fmt::format("The I2C bus failed to initialize: {}. Check the GPIOs in " + "menuconfig (Desktop Example Configuration).", + ec.message()), + 0, D::kLabelWrap); + st->bus.reset(); + return; + } + + // ---- scan ---- + auto bar = win.row(); + auto status = win.label("Press Scan to probe addresses 0x01..0x7F.", bar.id()); + auto table = win.table({"Address", "Decimal", "Note"}, {}, nullptr); + auto scan_btn = win.button("Scan", nullptr, bar.id(), D::kButtonPrimary); + scan_btn.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click) + return; + if (st->scan && st->scan->is_running()) + return; + st->scan.reset(); + scan_btn.set_enabled(false); + status.set_text("Scanning\xE2\x80\xA6"); + // the bus outlives the task: on_close joins the scan first + auto *bus = st->bus.get(); + auto *session = st.get(); + st->scan = std::make_unique(espp::Task::Config{ + .callback = + [=]() mutable { + std::vector rows; + std::vector found; + for (uint8_t addr = 0x01; addr <= 0x7F; ++addr) { + if (!bus->probe_device(addr)) + continue; + found.push_back(addr); + rows.push_back( + fmt::format("0x{:02X}\t{}\t{}", addr, addr, S::note(addr))); + } + { + std::lock_guard lock(session->mutex); + session->found = found; + } + table.set_items(rows); + status.set_text("{} device{} found", rows.size(), + rows.size() == 1 ? "" : "s"); + scan_btn.set_enabled(true); + return true; // one shot + }, + .task_config = {.name = "i2c_scan", .stack_size_bytes = 4 * 1024}}); + st->scan->start(); + }); + + // ---- register access ---- + auto tools = win.group("Register access"); + auto fields = win.row(tools.id()); + auto addr_box = win.textbox("", nullptr, fields.id(), "addr (hex)"); + addr_box.set_size(80, 0); + auto reg_box = win.textbox("0", nullptr, fields.id(), "register (hex)"); + reg_box.set_size(90, 0); + auto len_box = win.textbox("1", nullptr, fields.id(), "length"); + len_box.set_size(60, 0); + auto val_box = win.textbox("", nullptr, fields.id(), "bytes to write (hex)"); + auto actions = win.row(tools.id()); + auto result = win.label("", tools.id(), D::kLabelMonospace | D::kLabelWrap); + table.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Select || e.value < 0) + return; + std::lock_guard lock(st->mutex); + if (static_cast(e.value) < st->found.size()) + addr_box.set_text("{:02X}", st->found[static_cast(e.value)]); + }); + win.button( + "Read", + [=]() mutable { + const uint8_t addr = S::parse_hex(addr_box.text()); + const uint8_t reg = S::parse_hex(reg_box.text()); + const size_t len = + std::clamp(std::strtoul(len_box.text().c_str(), nullptr, 10), 1, 64); + std::vector data(len); + if (st->bus->read_at_register(addr, reg, data.data(), data.size())) + result.set_text("0x{:02X} reg 0x{:02X}: {}", addr, reg, S::hex_dump(data)); + else + result.set_text("0x{:02X} reg 0x{:02X}: read failed (no ACK?)", addr, reg); + }, + actions.id(), D::kButtonPrimary); + win.button( + "Write", + [=]() mutable { + const uint8_t addr = S::parse_hex(addr_box.text()); + std::vector data{S::parse_hex(reg_box.text())}; + const auto bytes = S::parse_bytes(val_box.text()); + data.insert(data.end(), bytes.begin(), bytes.end()); + if (st->bus->write(addr, data.data(), data.size())) + result.set_text("0x{:02X} <- {}ok", addr, S::hex_dump(data)); + else + result.set_text("0x{:02X} <- {}failed (no ACK?)", addr, S::hex_dump(data)); + }, + actions.id()); + }, + }); +} diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp new file mode 100644 index 0000000000..d51aadfc8f --- /dev/null +++ b/components/desktop/example/main/apps/network_app.hpp @@ -0,0 +1,312 @@ +#pragma once + +// Network: the Wi-Fi station (espp::WifiSta -- status, scan, connect / +// disconnect with the credentials kept in NVS) and, under its own Kconfig +// gate, the RMII Ethernet link (espp::Ethernet). The interfaces are brought +// up on the first launch and kept across windows (closing the window does not +// disconnect); the window's 1 s timer renders the state the driver callbacks +// record. A Wi-Fi scan blocks (and disconnects first), so it runs on its own +// task. + +#include +#include +#include +#include +#include +#include +#include + +#include "sdkconfig.h" + +#include "desktop.hpp" +#include "nvs_handle_espp.hpp" +#include "task.hpp" +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI +#include "wifi_sta.hpp" +#endif +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET +#include "esp_netif.h" +#include "ethernet.hpp" +#endif + +namespace desktop_example { + +/// The interfaces, shared by every Network window (created on first launch). +struct NetworkState { + std::mutex mutex; + std::string wifi_status{"idle"}; + std::string wifi_ip; + std::string wifi_mac; + std::vector scan_ssids; // in AP list order + std::atomic scanning{false}; +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI + std::unique_ptr wifi; + std::unique_ptr scan_task; // after wifi: joined before it goes away +#endif +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET + std::unique_ptr eth; +#endif + + void set_status(std::string_view status, std::string_view ip = "") { + std::lock_guard lock(mutex); + wifi_status = std::string(status); + wifi_ip = std::string(ip); + } + +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI + static const char *auth_name(wifi_auth_mode_t mode) { + switch (mode) { + case WIFI_AUTH_OPEN: + return "open"; + case WIFI_AUTH_WEP: + return "WEP"; + case WIFI_AUTH_WPA_PSK: + return "WPA"; + case WIFI_AUTH_WPA2_PSK: + return "WPA2"; + case WIFI_AUTH_WPA_WPA2_PSK: + return "WPA/WPA2"; + case WIFI_AUTH_WPA3_PSK: + return "WPA3"; + case WIFI_AUTH_WPA2_WPA3_PSK: + return "WPA2/WPA3"; + default: + return "other"; + } + } + + /// The station config for `ssid` / `password`: the callbacks record the + /// state this struct holds (they run on the event-loop task). + espp::WifiSta::Config wifi_config(std::string ssid, std::string password) { + return {.ssid = std::move(ssid), + .password = std::move(password), + .num_connect_retries = 3, + .auto_connect = true, + .on_connected = [this]() { set_status("connected, waiting for an IP"); }, + .on_disconnected = [this]() { set_status("disconnected"); }, + .on_got_ip = + [this](ip_event_got_ip_t *e) { + set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); + }, + .log_level = espp::Logger::Verbosity::INFO}; + } + + /// Bring the station up once, with the saved credentials (none = idle). + void ensure_wifi() { + if (wifi) + return; + std::error_code ec; + espp::NvsHandle nvs("desktop", ec); + std::string ssid, pass; + if (!ec) { + nvs.get("wifi_ssid", ssid, std::string(""), ec); + nvs.get("wifi_pass", pass, std::string(""), ec); + } + auto cfg = wifi_config(ssid, pass); + cfg.auto_connect = !ssid.empty(); + set_status(ssid.empty() ? "idle (no saved network)" : "connecting"); + wifi = std::make_unique(cfg); + wifi_mac = wifi->get_mac(); + } +#endif + +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET + /// Bring the RMII link up once (ESP32-Ethernet-Kit pins: IP101 PHY, 50 MHz + /// reference clock in on GPIO0; adjust for your board). + void ensure_ethernet() { + if (eth) + return; + eth = std::make_unique( + espp::Ethernet::Config{.interface = espp::Ethernet::RmiiConfig{.mdc_gpio = 23, + .mdio_gpio = 18, + .phy_addr = 1, + .phy_reset_gpio = 5, + .clock_ext_in = true, + .clock_gpio = 0}, + .hostname = "espp-desktop", + .log_level = espp::Logger::Verbosity::INFO}); + std::error_code ec; + eth->initialize(ec); + } +#endif +}; + +} // namespace desktop_example + +inline void register_network_app(espp::Desktop &desktop) { + auto net = std::make_shared(); + desktop.register_app({ + .name = "Network", + .icon = "\xF0\x9F\x93\xA1", // satellite antenna + .description = "Wi-Fi station and Ethernet status", + .launch = + [net](espp::Desktop &d, espp::Desktop::AppId app) { + using namespace std::chrono_literals; + using D = espp::Desktop; + auto win = d.create_window({.title = "Network", .app = app, .w = 520, .h = 0}); + std::vector> refreshers; + +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI + net->ensure_wifi(); + auto wifi = win.group("Wi-Fi station"); + auto status = win.label("Status: -", wifi.id(), D::kLabelBold); + auto ssid_label = win.label("SSID: -", wifi.id(), D::kLabelMonospace); + auto ip_label = win.label("IP: -", wifi.id(), D::kLabelMonospace); + auto rssi_label = win.label("RSSI: -", wifi.id(), D::kLabelMonospace); + win.label(fmt::format("MAC: {}", net->wifi_mac), wifi.id(), D::kLabelMonospace); + auto scan_bar = win.row(wifi.id()); + auto scan_btn = win.button("Scan", nullptr, scan_bar.id()); + auto scan_label = win.label("", scan_bar.id()); + auto ap_list = win.list({}, nullptr, wifi.id()); + ap_list.set_size(0, 140); + auto pass_row = win.row(wifi.id()); + win.label("Password", pass_row.id()); + auto pass_box = win.textbox("", nullptr, pass_row.id(), + "leave empty for an open network", D::kTextBoxPassword); + auto actions = win.row(wifi.id()); + + refreshers.push_back([=]() mutable { + std::string st, ip; + { + std::lock_guard lock(net->mutex); + st = net->wifi_status; + ip = net->wifi_ip; + } + const bool connected = net->wifi->is_connected(); + status.set_text("Status: {}", st); + ssid_label.set_text("SSID: {}", connected ? net->wifi->get_ssid() : "-"); + ip_label.set_text("IP: {}", ip.empty() ? "-" : ip); + if (connected) + rssi_label.set_text("RSSI: {} dBm", net->wifi->get_rssi()); + else + rssi_label.set_text("RSSI: -"); + }); + + scan_btn.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click || net->scanning) + return; + net->scanning = true; + net->scan_task.reset(); // the previous (finished) scan + scan_btn.set_enabled(false); + scan_label.set_text("scanning\xE2\x80\xA6 (disconnects first)"); + auto *state = net.get(); // outlives the task (joined by its destructor) + net->scan_task = std::make_unique(espp::Task::Config{ + .callback = + [=]() mutable { + const auto aps = state->wifi->scan(20); + std::vector rows, ssids; + for (const auto &ap : aps) { + const std::string ssid(reinterpret_cast(ap.ssid)); + ssids.push_back(ssid); + rows.push_back(fmt::format( + "{} ({} dBm, {}, ch {})", ssid.empty() ? "" : ssid, + static_cast(ap.rssi), state->auth_name(ap.authmode), + static_cast(ap.primary))); + } + { + std::lock_guard lock(state->mutex); + state->scan_ssids = ssids; + } + ap_list.set_items(rows); + scan_label.set_text("{} networks", rows.size()); + scan_btn.set_enabled(true); + state->scanning = false; + return true; // one shot + }, + .task_config = {.name = "wifi_scan", .stack_size_bytes = 6 * 1024}}); + net->scan_task->start(); + }); + + win.button( + "Connect", + [=, &d]() mutable { + std::string ssid; + { + std::lock_guard lock(net->mutex); + const int32_t i = ap_list.selected(); + if (i >= 0 && static_cast(i) < net->scan_ssids.size()) + ssid = net->scan_ssids[static_cast(i)]; + } + if (ssid.empty()) { + d.notify({.title = "Wi-Fi", + .text = "scan, then select a network", + .level = D::NotifyLevel::Warn}); + return; + } + const std::string pass = pass_box.text(); + std::error_code ec; + espp::NvsHandle nvs("desktop", ec); + if (!ec) { + nvs.set("wifi_ssid", ssid, ec); + nvs.set("wifi_pass", pass, ec); + } + net->set_status(fmt::format("connecting to {}", ssid)); + const bool was_connected = net->wifi->is_connected(); + // reconfigure() reconnects when it was connected; else connect() + bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass)); + if (ok && !was_connected) + ok = net->wifi->connect(); + if (!ok) { + net->set_status("connect failed"); + d.notify({.title = "Wi-Fi", + .text = fmt::format("could not start connecting to {}", ssid), + .level = D::NotifyLevel::Error}); + } + }, + actions.id(), D::kButtonPrimary); + win.button( + "Disconnect", + [=]() mutable { + net->wifi->disconnect(); + net->set_status("disconnected"); + }, + actions.id()); + win.button( + "Forget", + [=, &d]() mutable { + std::error_code ec; + espp::NvsHandle nvs("desktop", ec); + if (!ec) { + nvs.erase("wifi_ssid", ec); + nvs.erase("wifi_pass", ec); + } + net->wifi->disconnect(); + net->set_status("idle (no saved network)"); + d.notify({.title = "Wi-Fi", .text = "saved network forgotten"}); + }, + actions.id(), D::kButtonDanger); +#endif + +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET + net->ensure_ethernet(); + auto eth = win.group("Ethernet (RMII)"); + auto link = win.label("Link: -", eth.id(), D::kLabelBold); + auto eth_ip = win.label("IP: -", eth.id(), D::kLabelMonospace); + win.label(fmt::format("MAC: {}", net->eth->get_mac_address()), eth.id(), + D::kLabelMonospace); + auto speed = win.label("Speed: -", eth.id(), D::kLabelMonospace); + refreshers.push_back([=]() mutable { + const bool up = net->eth->link_up(); + link.set_text("Link: {}", !net->eth->is_initialized() ? "not initialized" + : up ? "up" + : "down"); + eth_ip.set_text("IP: {}", + net->eth->is_connected() ? net->eth->get_ip_address() : "-"); + if (const auto sd = net->eth->link_speed_duplex(); sd) + speed.set_text("Speed: {} Mbit/s {}-duplex", sd->first, + sd->second ? "full" : "half"); + else + speed.set_text("Speed: -"); + }); +#endif + + // (the app is only registered when at least one group is enabled) + auto refresh = [refreshers]() { + for (const auto &fn : refreshers) + fn(); + }; + refresh(); + win.add_timer(1s, refresh); + }, + }); +} diff --git a/components/desktop/example/main/desktop_example.cpp b/components/desktop/example/main/desktop_example.cpp index 6c7a6f2556..e2bf755ab0 100644 --- a/components/desktop/example/main/desktop_example.cpp +++ b/components/desktop/example/main/desktop_example.cpp @@ -29,6 +29,16 @@ #include "settings_app.hpp" #include "system_monitor_app.hpp" #include "task_manager_app.hpp" +// hardware apps, each behind its Kconfig option (Desktop Example Configuration) +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN +#include "canopen_app.hpp" +#endif +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_I2C +#include "i2c_scanner_app.hpp" +#endif +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI || CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET +#include "network_app.hpp" +#endif using namespace std::chrono_literals; @@ -45,7 +55,9 @@ using namespace std::chrono_literals; // - espp::OtaService (module 0, `espp.ota`): firmware update // - espp::CoreDumpService (module 4, `espp.coredump`): last-crash report // Apps: Counter, About, System Monitor, Task Manager, Log Viewer, Files (+ -// Editor), Settings (main/apps/*.hpp; Counter is the API reference). +// Editor), Settings (main/apps/*.hpp; Counter is the API reference), plus the +// Kconfig-gated hardware apps CANopen / DS402 (simulated node or TWAI), I2C +// scanner and Network (Wi-Fi station, Ethernet). extern "C" void app_main(void) { //! [console_capture] @@ -104,6 +116,15 @@ extern "C" void app_main(void) { register_task_manager_app(desktop); register_log_viewer_app(desktop); register_files_app(desktop); +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN + register_canopen_app(desktop); +#endif +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_I2C + register_i2c_scanner_app(desktop); +#endif +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI || CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET + register_network_app(desktop); +#endif register_settings_app(desktop, "espp Desktop"); desktop_example::apply_saved_settings(desktop); From e22d2a7aa01e9434fadc6fe6bf3db40e27ef3bd0 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 00:13:09 -0500 Subject: [PATCH 02/21] fix(desktop): address the Copilot review of the hardware apps - Network: reject SSIDs > 32 / passwords > 63 bytes before saving or reconfiguring (the driver fields are 32 / 64 bytes and the text box is unbounded); keep the driver's station config in RAM (Wifi::set_storage(WIFI_STORAGE_RAM)) so Forget really drops the credentials: it now erases the NVS keys, clears the driver config and reconfigures WifiSta with empty credentials and auto-connect off; every station action (Scan / Connect / Disconnect / Forget) is disabled while a scan drives the station from the scan task and refused with a toast if clicked anyway, re-synced by the 1 s timer; the last scan's rows are kept in the shared state so a new window shows them. - CANopen: the mode Select starts with no selection; 0x6060 is only written on a change (the label shows the drive's 0x6061). - I2C: parse the register / value fields as full 8-bit values (only the device address is masked to 7 bits). - Example README: describe the actual defaults (Ethernet off, EMAC SoCs only). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- components/desktop/example/README.md | 7 +- .../desktop/example/main/apps/canopen_app.hpp | 5 +- .../example/main/apps/i2c_scanner_app.hpp | 15 +- .../desktop/example/main/apps/network_app.hpp | 186 ++++++++++++------ 4 files changed, 143 insertions(+), 70 deletions(-) diff --git a/components/desktop/example/README.md b/components/desktop/example/README.md index 81f81f9679..a3e0c81cba 100644 --- a/components/desktop/example/README.md +++ b/components/desktop/example/README.md @@ -30,8 +30,11 @@ Apps (`main/apps/*.hpp`, one `register__app()` each): console; the capture itself is the compile-time `CONFIG_DESKTOP_EXAMPLE_LOG_CAPTURE`), all kept in NVS and restored at boot. -Hardware apps, each behind a Kconfig option (all on by default, so the CI -build compiles every one of them; see [Configuration](#configuration)): +Hardware apps, each behind a Kconfig option (see +[Configuration](#configuration)). CANopen (on the simulated node), the I2C +scanner and the Wi-Fi group are on by default, so the CI build compiles them; +the Ethernet group defaults off and is only selectable on SoCs with an EMAC +(ESP32 / ESP32-P4, not the S3): - **CANopen / DS402** — a CiA 301 NMT master + SDO client (`espp::CanopenClient`) and a CiA 402 drive panel (`espp::Ds402Drive`): diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index bf61121d4f..f39d23d47b 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -305,7 +305,10 @@ inline void register_canopen_app(espp::Desktop &desktop) { const std::vector modes = {Mode::ProfilePosition, Mode::ProfileVelocity, Mode::ProfileTorque, Mode::Homing}; win.select( - {"Profile position", "Profile velocity", "Profile torque", "Homing"}, 1, + // no selection until the user picks one: 0x6060 is only + // written on a change (the label shows the drive's 0x6061) + {"Profile position", "Profile velocity", "Profile torque", "Homing"}, + D::kNoSelection, [=](int32_t i) { if (i < 0 || static_cast(i) >= modes.size()) return; diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp index d601532aca..8efb314e64 100644 --- a/components/desktop/example/main/apps/i2c_scanner_app.hpp +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -44,9 +44,14 @@ struct I2cSession { } return out; } - static uint8_t parse_hex(const std::string &text) { + /// A 7-bit device address ("3C" / "0x3C"). + static uint8_t parse_addr(const std::string &text) { return static_cast(std::strtoul(text.c_str(), nullptr, 16) & 0x7F); } + /// A full 8-bit register value ("80" / "0xFF"). + static uint8_t parse_byte(const std::string &text) { + return static_cast(std::strtoul(text.c_str(), nullptr, 16) & 0xFF); + } static std::string hex_dump(const std::vector &bytes) { std::string s; for (const auto b : bytes) @@ -169,8 +174,8 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { win.button( "Read", [=]() mutable { - const uint8_t addr = S::parse_hex(addr_box.text()); - const uint8_t reg = S::parse_hex(reg_box.text()); + const uint8_t addr = S::parse_addr(addr_box.text()); + const uint8_t reg = S::parse_byte(reg_box.text()); const size_t len = std::clamp(std::strtoul(len_box.text().c_str(), nullptr, 10), 1, 64); std::vector data(len); @@ -183,8 +188,8 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { win.button( "Write", [=]() mutable { - const uint8_t addr = S::parse_hex(addr_box.text()); - std::vector data{S::parse_hex(reg_box.text())}; + const uint8_t addr = S::parse_addr(addr_box.text()); + std::vector data{S::parse_byte(reg_box.text())}; const auto bytes = S::parse_bytes(val_box.text()); data.insert(data.end(), bytes.begin(), bytes.end()); if (st->bus->write(addr, data.data(), data.size())) diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index d51aadfc8f..8d433f9066 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -22,6 +22,8 @@ #include "nvs_handle_espp.hpp" #include "task.hpp" #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI +#include "esp_wifi.h" +#include "wifi.hpp" #include "wifi_sta.hpp" #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET @@ -38,6 +40,9 @@ struct NetworkState { std::string wifi_ip; std::string wifi_mac; std::vector scan_ssids; // in AP list order + std::vector scan_rows; // the AP list's items (shown again by a new window) + /// A scan is in flight on the scan task: it disconnects and drives the + /// station, so every other station action is refused until it is done. std::atomic scanning{false}; #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI std::unique_ptr wifi; @@ -102,12 +107,39 @@ struct NetworkState { nvs.get("wifi_ssid", ssid, std::string(""), ec); nvs.get("wifi_pass", pass, std::string(""), ec); } + // The app's NVS keys are the only persisted credentials: keep the + // driver's own copy in RAM so esp_wifi_set_config() does not write a + // second one to flash (which Forget could not clear). + auto &stack = espp::Wifi::get(); + if (stack.init()) + stack.set_storage(WIFI_STORAGE_RAM); auto cfg = wifi_config(ssid, pass); cfg.auto_connect = !ssid.empty(); set_status(ssid.empty() ? "idle (no saved network)" : "connecting"); wifi = std::make_unique(cfg); wifi_mac = wifi->get_mac(); } + + /// Drop the saved credentials everywhere: the app's NVS keys, the driver's + /// station config and WifiSta's stored config (reconfigured with empty + /// credentials and auto-connect off, so nothing reconnects). + bool forget() { + std::error_code ec; + espp::NvsHandle nvs("desktop", ec); + if (!ec) { + nvs.erase("wifi_ssid", ec); + nvs.erase("wifi_pass", ec); + } + wifi->disconnect(); + wifi_config_t empty{}; + const bool cleared = esp_wifi_set_config(WIFI_IF_STA, &empty) == ESP_OK; + auto cfg = + wifi_config("", ""); // empty ssid: reconfigure() adopts the (now empty) driver config + cfg.auto_connect = false; + const bool ok = wifi->reconfigure(cfg) && cleared; + set_status("idle (no saved network)"); + return ok; + } #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET @@ -157,13 +189,38 @@ inline void register_network_app(espp::Desktop &desktop) { auto scan_bar = win.row(wifi.id()); auto scan_btn = win.button("Scan", nullptr, scan_bar.id()); auto scan_label = win.label("", scan_bar.id()); - auto ap_list = win.list({}, nullptr, wifi.id()); + std::vector rows_now; + { + std::lock_guard lock(net->mutex); + rows_now = net->scan_rows; // the last scan, if any + } + auto ap_list = win.list(rows_now, nullptr, wifi.id()); ap_list.set_size(0, 140); auto pass_row = win.row(wifi.id()); win.label("Password", pass_row.id()); auto pass_box = win.textbox("", nullptr, pass_row.id(), "leave empty for an open network", D::kTextBoxPassword); auto actions = win.row(wifi.id()); + auto connect_btn = win.button("Connect", nullptr, actions.id(), D::kButtonPrimary); + auto disconnect_btn = win.button("Disconnect", nullptr, actions.id()); + auto forget_btn = win.button("Forget", nullptr, actions.id(), D::kButtonDanger); + // every station action is disabled while a scan drives the + // station from the scan task (and refused if clicked anyway) + auto set_busy = [=](bool busy) mutable { + scan_btn.set_enabled(!busy); + connect_btn.set_enabled(!busy); + disconnect_btn.set_enabled(!busy); + forget_btn.set_enabled(!busy); + }; + auto refuse_if_scanning = [=, &d]() { + if (!net->scanning) + return false; + d.notify({.title = "Wi-Fi", + .text = "a scan is in progress; try again when it is done", + .level = D::NotifyLevel::Warn}); + return true; + }; + set_busy(net->scanning); refreshers.push_back([=]() mutable { std::string st, ip; @@ -172,6 +229,8 @@ inline void register_network_app(espp::Desktop &desktop) { st = net->wifi_status; ip = net->wifi_ip; } + // also re-syncs a window opened while a scan started elsewhere + set_busy(net->scanning); const bool connected = net->wifi->is_connected(); status.set_text("Status: {}", st); ssid_label.set_text("SSID: {}", connected ? net->wifi->get_ssid() : "-"); @@ -183,11 +242,11 @@ inline void register_network_app(espp::Desktop &desktop) { }); scan_btn.on_event([=](const D::WidgetEvent &e) mutable { - if (e.kind != D::WidgetEventKind::Click || net->scanning) + if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) return; net->scanning = true; net->scan_task.reset(); // the previous (finished) scan - scan_btn.set_enabled(false); + set_busy(true); scan_label.set_text("scanning\xE2\x80\xA6 (disconnects first)"); auto *state = net.get(); // outlives the task (joined by its destructor) net->scan_task = std::make_unique(espp::Task::Config{ @@ -206,75 +265,78 @@ inline void register_network_app(espp::Desktop &desktop) { { std::lock_guard lock(state->mutex); state->scan_ssids = ssids; + state->scan_rows = rows; } ap_list.set_items(rows); scan_label.set_text("{} networks", rows.size()); - scan_btn.set_enabled(true); state->scanning = false; + set_busy(false); return true; // one shot }, .task_config = {.name = "wifi_scan", .stack_size_bytes = 6 * 1024}}); net->scan_task->start(); }); - win.button( - "Connect", - [=, &d]() mutable { - std::string ssid; - { - std::lock_guard lock(net->mutex); - const int32_t i = ap_list.selected(); - if (i >= 0 && static_cast(i) < net->scan_ssids.size()) - ssid = net->scan_ssids[static_cast(i)]; - } - if (ssid.empty()) { - d.notify({.title = "Wi-Fi", - .text = "scan, then select a network", - .level = D::NotifyLevel::Warn}); - return; - } - const std::string pass = pass_box.text(); - std::error_code ec; - espp::NvsHandle nvs("desktop", ec); - if (!ec) { - nvs.set("wifi_ssid", ssid, ec); - nvs.set("wifi_pass", pass, ec); - } - net->set_status(fmt::format("connecting to {}", ssid)); - const bool was_connected = net->wifi->is_connected(); - // reconfigure() reconnects when it was connected; else connect() - bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass)); - if (ok && !was_connected) - ok = net->wifi->connect(); - if (!ok) { - net->set_status("connect failed"); - d.notify({.title = "Wi-Fi", - .text = fmt::format("could not start connecting to {}", ssid), - .level = D::NotifyLevel::Error}); - } - }, - actions.id(), D::kButtonPrimary); - win.button( - "Disconnect", - [=]() mutable { - net->wifi->disconnect(); - net->set_status("disconnected"); - }, - actions.id()); - win.button( - "Forget", - [=, &d]() mutable { - std::error_code ec; - espp::NvsHandle nvs("desktop", ec); - if (!ec) { - nvs.erase("wifi_ssid", ec); - nvs.erase("wifi_pass", ec); - } - net->wifi->disconnect(); - net->set_status("idle (no saved network)"); - d.notify({.title = "Wi-Fi", .text = "saved network forgotten"}); - }, - actions.id(), D::kButtonDanger); + connect_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) + return; + std::string ssid; + { + std::lock_guard lock(net->mutex); + const int32_t i = ap_list.selected(); + if (i >= 0 && static_cast(i) < net->scan_ssids.size()) + ssid = net->scan_ssids[static_cast(i)]; + } + if (ssid.empty()) { + d.notify({.title = "Wi-Fi", + .text = "scan, then select a network", + .level = D::NotifyLevel::Warn}); + return; + } + const std::string pass = pass_box.text(); + // the driver fields are 32 / 64 bytes (WifiSta::reconfigure + // copies the strings as given); the text boxes are unbounded + if (ssid.size() > 32 || pass.size() > 63) { + d.notify({.title = "Wi-Fi", + .text = "SSID must be at most 32 bytes and the password at " + "most 63 bytes", + .level = D::NotifyLevel::Error}); + return; + } + std::error_code ec; + espp::NvsHandle nvs("desktop", ec); + if (!ec) { + nvs.set("wifi_ssid", ssid, ec); + nvs.set("wifi_pass", pass, ec); + } + net->set_status(fmt::format("connecting to {}", ssid)); + const bool was_connected = net->wifi->is_connected(); + // reconfigure() reconnects when it was connected; else connect() + bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass)); + if (ok && !was_connected) + ok = net->wifi->connect(); + if (!ok) { + net->set_status("connect failed"); + d.notify({.title = "Wi-Fi", + .text = fmt::format("could not start connecting to {}", ssid), + .level = D::NotifyLevel::Error}); + } + }); + disconnect_btn.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) + return; + net->wifi->disconnect(); + net->set_status("disconnected"); + }); + forget_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) + return; + const bool ok = net->forget(); + d.notify({.title = "Wi-Fi", + .text = ok ? "saved network forgotten" + : "credentials erased, but the station could not be reset", + .level = ok ? D::NotifyLevel::Ok : D::NotifyLevel::Warn}); + }); #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET From 1e55e543b73cf24d394d25cf4889fa6ccf62f9fa Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 00:34:36 -0500 Subject: [PATCH 03/21] fix(desktop): address the second Copilot review of the hardware apps - I2C: parse the address / register fields as whole hex fields and reject empty, malformed or out-of-range entries (address 0x01..0x7F, register 0x00..0xFF) with a message instead of masking them into real targets (an empty address masked to 0x00 was the general-call broadcast). - Network: commit the NVS erasures (and the saves) -- erase_item()/set_item() only stage the change -- and fold the NVS result into Forget's result so the toast is honest; before a scan, disconnect intentionally (WifiSta::disconnect(), which suppresses the retry that its DISCONNECTED handler would otherwise fire during the scan), wait for the event, scan, then reconnect explicitly when the station was connected. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../example/main/apps/i2c_scanner_app.hpp | 70 ++++++++++++++----- .../desktop/example/main/apps/network_app.hpp | 57 ++++++++++++--- 2 files changed, 101 insertions(+), 26 deletions(-) diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp index 8efb314e64..d411a99032 100644 --- a/components/desktop/example/main/apps/i2c_scanner_app.hpp +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -44,13 +45,39 @@ struct I2cSession { } return out; } - /// A 7-bit device address ("3C" / "0x3C"). - static uint8_t parse_addr(const std::string &text) { - return static_cast(std::strtoul(text.c_str(), nullptr, 16) & 0x7F); + /// One hex number ("3C" / "0x3C") taking the whole field (blanks around it + /// allowed); nullopt when the field is empty, malformed or above `max`, so + /// a bad entry never becomes a real target (an empty address masked to 0x00 + /// would be the general-call broadcast). + static std::optional parse_field(const std::string &text, unsigned long max) { + const char *p = text.c_str(); + while (*p == ' ') + ++p; + if (!*p) + return std::nullopt; + char *end = nullptr; + const unsigned long v = std::strtoul(p, &end, 16); + if (end == p) + return std::nullopt; + while (*end == ' ') + ++end; + if (*end || v > max) + return std::nullopt; + return v; + } + /// A 7-bit device address in the addressable range 0x01..0x7F. + static std::optional parse_addr(const std::string &text) { + const auto v = parse_field(text, 0x7F); + if (!v || *v == 0) + return std::nullopt; + return static_cast(*v); } - /// A full 8-bit register value ("80" / "0xFF"). - static uint8_t parse_byte(const std::string &text) { - return static_cast(std::strtoul(text.c_str(), nullptr, 16) & 0xFF); + /// A full 8-bit register value (0x00..0xFF). + static std::optional parse_byte(const std::string &text) { + const auto v = parse_field(text, 0xFF); + if (!v) + return std::nullopt; + return static_cast(*v); } static std::string hex_dump(const std::vector &bytes) { std::string s; @@ -174,28 +201,39 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { win.button( "Read", [=]() mutable { - const uint8_t addr = S::parse_addr(addr_box.text()); - const uint8_t reg = S::parse_byte(reg_box.text()); + const auto addr = S::parse_addr(addr_box.text()); + const auto reg = S::parse_byte(reg_box.text()); + if (!addr || !reg) { + result.set_text(!addr ? "address must be a hex value in 0x01..0x7F" + : "register must be a hex value in 0x00..0xFF"); + return; + } const size_t len = std::clamp(std::strtoul(len_box.text().c_str(), nullptr, 10), 1, 64); std::vector data(len); - if (st->bus->read_at_register(addr, reg, data.data(), data.size())) - result.set_text("0x{:02X} reg 0x{:02X}: {}", addr, reg, S::hex_dump(data)); + if (st->bus->read_at_register(*addr, *reg, data.data(), data.size())) + result.set_text("0x{:02X} reg 0x{:02X}: {}", *addr, *reg, S::hex_dump(data)); else - result.set_text("0x{:02X} reg 0x{:02X}: read failed (no ACK?)", addr, reg); + result.set_text("0x{:02X} reg 0x{:02X}: read failed (no ACK?)", *addr, *reg); }, actions.id(), D::kButtonPrimary); win.button( "Write", [=]() mutable { - const uint8_t addr = S::parse_addr(addr_box.text()); - std::vector data{S::parse_byte(reg_box.text())}; + const auto addr = S::parse_addr(addr_box.text()); + const auto reg = S::parse_byte(reg_box.text()); + if (!addr || !reg) { + result.set_text(!addr ? "address must be a hex value in 0x01..0x7F" + : "register must be a hex value in 0x00..0xFF"); + return; + } + std::vector data{*reg}; const auto bytes = S::parse_bytes(val_box.text()); data.insert(data.end(), bytes.begin(), bytes.end()); - if (st->bus->write(addr, data.data(), data.size())) - result.set_text("0x{:02X} <- {}ok", addr, S::hex_dump(data)); + if (st->bus->write(*addr, data.data(), data.size())) + result.set_text("0x{:02X} <- {}ok", *addr, S::hex_dump(data)); else - result.set_text("0x{:02X} <- {}failed (no ACK?)", addr, S::hex_dump(data)); + result.set_text("0x{:02X} <- {}failed (no ACK?)", *addr, S::hex_dump(data)); }, actions.id()); }, diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 8d433f9066..f45f8c6a4d 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include "sdkconfig.h" @@ -126,9 +127,14 @@ struct NetworkState { bool forget() { std::error_code ec; espp::NvsHandle nvs("desktop", ec); - if (!ec) { - nvs.erase("wifi_ssid", ec); - nvs.erase("wifi_pass", ec); + bool erased = !ec; + if (erased) { + // a missing key is fine (nothing was saved); any other failure is not + std::error_code e1, e2; + nvs.erase("wifi_ssid", e1); + nvs.erase("wifi_pass", e2); + nvs.commit(ec); // erase_item() only stages the change + erased = !ec; } wifi->disconnect(); wifi_config_t empty{}; @@ -136,10 +142,24 @@ struct NetworkState { auto cfg = wifi_config("", ""); // empty ssid: reconfigure() adopts the (now empty) driver config cfg.auto_connect = false; - const bool ok = wifi->reconfigure(cfg) && cleared; + const bool ok = wifi->reconfigure(cfg) && cleared && erased; set_status("idle (no saved network)"); return ok; } + + /// Save the credentials (committed, so they survive a reboot). + bool save_credentials(const std::string &ssid, const std::string &pass) { + std::error_code ec; + espp::NvsHandle nvs("desktop", ec); + if (ec) + return false; + nvs.set("wifi_ssid", ssid, ec); + if (!ec) + nvs.set("wifi_pass", pass, ec); + if (!ec) + nvs.commit(ec); + return !ec; + } #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET @@ -252,7 +272,26 @@ inline void register_network_app(espp::Desktop &desktop) { net->scan_task = std::make_unique(espp::Task::Config{ .callback = [=]() mutable { + // WifiSta::scan() drops the link with a bare + // esp_wifi_disconnect(), which WifiSta's DISCONNECTED + // handler treats as unintentional and answers with a + // reconnect attempt (num_connect_retries > 0) while the + // scan is starting. Disconnect intentionally first (that + // suppresses the retries), give the event time to land, + // then scan and reconnect ourselves afterwards. + const bool was_connected = state->wifi->is_connected(); + if (was_connected) { + state->wifi->disconnect(); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + state->set_status("scanning"); const auto aps = state->wifi->scan(20); + if (was_connected) { + state->set_status("reconnecting"); + state->wifi->connect(); + } else { + state->set_status("disconnected"); + } std::vector rows, ssids; for (const auto &ap : aps) { const std::string ssid(reinterpret_cast(ap.ssid)); @@ -303,12 +342,10 @@ inline void register_network_app(espp::Desktop &desktop) { .level = D::NotifyLevel::Error}); return; } - std::error_code ec; - espp::NvsHandle nvs("desktop", ec); - if (!ec) { - nvs.set("wifi_ssid", ssid, ec); - nvs.set("wifi_pass", pass, ec); - } + if (!net->save_credentials(ssid, pass)) + d.notify({.title = "Wi-Fi", + .text = "could not save the credentials to NVS (connecting anyway)", + .level = D::NotifyLevel::Warn}); net->set_status(fmt::format("connecting to {}", ssid)); const bool was_connected = net->wifi->is_connected(); // reconfigure() reconnects when it was connected; else connect() From 2c9244a7e10780b1054189da6dd2694e22529d4f Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 01:43:10 -0500 Subject: [PATCH 04/21] fix(desktop): address the third Copilot review of the hardware apps - Network: strip the trailing NUL NvsHandle::get() leaves on strings (it sizes the string to the stored length including the terminator, so a 32-byte SSID came back as 33 bytes and reconfigure() would copy 33 into the 32-byte driver field) and re-check the 32 / 63 byte bounds on the loaded credentials before building the station config. - CANopen: parse the node id / SDO index / sub-index / value fields as whole numbers (no trailing junk, no sign, no overflow) within the destination width (index <= 0xFFFF, sub <= 0xFF, value within the selected u8 / u16 / u32) and show the reason in the result label instead of narrowing. - I2C: the byte-list parser rejects the whole field on any malformed or > 0xFF token (no prefix acceptance / truncation) and Write stops on it. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/canopen_app.hpp | 78 +++++++++++++++---- .../example/main/apps/i2c_scanner_app.hpp | 22 ++++-- .../desktop/example/main/apps/network_app.hpp | 24 +++++- 3 files changed, 100 insertions(+), 24 deletions(-) diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index f39d23d47b..bdf8466eb1 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -10,6 +10,7 @@ // task may call the mutators). #include +#include #include #include #include @@ -17,6 +18,7 @@ #include #include #include +#include #include #include @@ -67,6 +69,28 @@ struct CanopenSession { bool stopping{false}; std::unique_ptr task; + /// One unsigned number taking the whole field (blanks around it allowed; + /// `base` 0 = "0x.." hex or decimal); nullopt when the field is empty, + /// malformed (trailing junk, a sign), overflows or exceeds `max`, so a bad + /// entry is never narrowed into a different object or value. + static std::optional parse_field(const std::string &text, int base, uint32_t max) { + const char *p = text.c_str(); + while (*p == ' ') + ++p; + if (!*p || *p == '-' || *p == '+') + return std::nullopt; + char *end = nullptr; + errno = 0; + const unsigned long v = std::strtoul(p, &end, base); + if (end == p || errno == ERANGE) + return std::nullopt; + while (*end == ' ') + ++end; + if (*end || v > max) + return std::nullopt; + return static_cast(v); + } + static const char *nmt_name(NmtState s) { switch (s) { case NmtState::BootUp: @@ -231,9 +255,7 @@ inline void register_canopen_app(espp::Desktop &desktop) { {.title = "CANopen / DS402", .app = app, .w = 560, .h = 0, .on_close = [st]() { st->shutdown(); }}); - auto parse = [](const std::string &s, int base) { - return static_cast(std::strtoul(s.c_str(), nullptr, base)); - }; + using S = desktop_example::CanopenSession; // ---- bus / node ---- auto top = win.row(); @@ -244,16 +266,17 @@ inline void register_canopen_app(espp::Desktop &desktop) { win.button( "Apply", [=]() mutable { - const uint32_t id = parse(node_box.text(), 10); - if (id < 1 || id > 127) { + const auto parsed = S::parse_field(node_box.text(), 10, 127); + if (!parsed || *parsed < 1) { st->desktop.notify({.title = "CANopen", - .text = "node id must be 1..127", + .text = "node id must be a decimal number 1..127", .level = D::NotifyLevel::Error}); return; } + const uint8_t id = static_cast(*parsed); st->run([=]() { std::error_code ec; - if (!st->open(static_cast(id), ec)) + if (!st->open(id, ec)) st->fail("bus init", ec); else st->desktop.notify({.title = "CANopen", @@ -390,11 +413,27 @@ inline void register_canopen_app(espp::Desktop &desktop) { auto val_box = win.textbox("0", nullptr, sdo_row.id(), "value (0x.. or decimal)"); val_box.set_size(110, 0); st->sdo_result = win.label("", sdo.id(), D::kLabelMonospace | D::kLabelWrap); + // index / sub-index from the text boxes, or nullopt (with the + // reason in the result label) when either field is not a whole + // hex number within its width + auto object = [=]() mutable -> std::optional> { + const auto index = S::parse_field(idx_box.text(), 16, 0xFFFF); + const auto sub = S::parse_field(sub_box.text(), 16, 0xFF); + if (!index || !sub) { + st->sdo_result.set_text(!index ? "index must be a hex value 0000..FFFF" + : "sub-index must be a hex value 00..FF"); + return std::nullopt; + } + return std::pair{static_cast(*index), + static_cast(*sub)}; + }; win.button( "Read", - [=]() { - const uint16_t index = static_cast(parse(idx_box.text(), 16)); - const uint8_t sub = static_cast(parse(sub_box.text(), 16)); + [=]() mutable { + const auto obj = object(); + if (!obj) + return; + const auto [index, sub] = *obj; st->run([=]() { std::error_code ec; std::array buf{}; @@ -413,11 +452,22 @@ inline void register_canopen_app(espp::Desktop &desktop) { sdo_row.id()); win.button( "Write", - [=]() { - const uint16_t index = static_cast(parse(idx_box.text(), 16)); - const uint8_t sub = static_cast(parse(sub_box.text(), 16)); - const uint32_t value = parse(val_box.text(), 0); + [=]() mutable { + const auto obj = object(); + if (!obj) + return; + const auto [index, sub] = *obj; const int32_t w = width.selected(); + const uint32_t max = w == 0 ? 0xFF : w == 1 ? 0xFFFF : 0xFFFFFFFFu; + const auto parsed = S::parse_field(val_box.text(), 0, max); + if (!parsed) { + st->sdo_result.set_text("value must be a decimal or 0x.. number within {}", + w == 0 ? "u8 (0..255)" + : w == 1 ? "u16 (0..65535)" + : "u32 (0..4294967295)"); + return; + } + const uint32_t value = *parsed; st->run([=]() { std::error_code ec; bool ok = false; diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp index d411a99032..fe7c92b4bf 100644 --- a/components/desktop/example/main/apps/i2c_scanner_app.hpp +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -26,9 +26,11 @@ struct I2cSession { std::vector found; // addresses in table order (scan task writes, handlers read) std::unique_ptr scan; // one-shot; joined before the bus goes away - /// Parse "0x3C", "3c" or "60" (hex unless prefixed) into a byte list of - /// space / comma separated values. - static std::vector parse_bytes(const std::string &text) { + /// Parse a space / comma separated list of hex bytes ("01 ff, 0x10") into + /// a byte list; nullopt when any token is malformed or above 0xFF (the + /// whole field is rejected, nothing is truncated). An empty field is an + /// empty list. + static std::optional> parse_bytes(const std::string &text) { std::vector out; const char *p = text.c_str(); while (*p) { @@ -38,9 +40,9 @@ struct I2cSession { break; char *end = nullptr; const unsigned long v = std::strtoul(p, &end, 16); - if (end == p) - break; - out.push_back(static_cast(v & 0xFF)); + if (end == p || v > 0xFF || (*end && *end != ' ' && *end != ',')) + return std::nullopt; + out.push_back(static_cast(v)); p = end; } return out; @@ -227,9 +229,13 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { : "register must be a hex value in 0x00..0xFF"); return; } - std::vector data{*reg}; const auto bytes = S::parse_bytes(val_box.text()); - data.insert(data.end(), bytes.begin(), bytes.end()); + if (!bytes) { + result.set_text("bytes must be hex values 00..FF, space / comma separated"); + return; + } + std::vector data{*reg}; + data.insert(data.end(), bytes->begin(), bytes->end()); if (st->bus->write(*addr, data.data(), data.size())) result.set_text("0x{:02X} <- {}ok", *addr, S::hex_dump(data)); else diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index f45f8c6a4d..9f5b97321e 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -97,6 +97,20 @@ struct NetworkState { .log_level = espp::Logger::Verbosity::INFO}; } + /// A string from NVS ("" when unset). NvsHandle::get() sizes the string to + /// the stored length INCLUDING the terminating NUL, so strip trailing NULs + /// (a 32-byte SSID would otherwise come back as 33 bytes). + static std::string nvs_string(espp::NvsHandle &nvs, const char *key) { + std::error_code ec; + std::string s; + nvs.get(key, s, std::string(""), ec); + if (ec) + return ""; + while (!s.empty() && s.back() == '\0') + s.pop_back(); + return s; + } + /// Bring the station up once, with the saved credentials (none = idle). void ensure_wifi() { if (wifi) @@ -105,8 +119,14 @@ struct NetworkState { espp::NvsHandle nvs("desktop", ec); std::string ssid, pass; if (!ec) { - nvs.get("wifi_ssid", ssid, std::string(""), ec); - nvs.get("wifi_pass", pass, std::string(""), ec); + ssid = nvs_string(nvs, "wifi_ssid"); + pass = nvs_string(nvs, "wifi_pass"); + } + // the same bounds Connect enforces before saving (the driver fields are + // 32 / 64 bytes); anything else in flash is treated as no credentials + if (ssid.size() > 32 || pass.size() > 63) { + ssid.clear(); + pass.clear(); } // The app's NVS keys are the only persisted credentials: keep the // driver's own copy in RAM so esp_wifi_set_config() does not write a From 6b0e710ce76dd0a1bd14e3e04ef62ec740918887 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 03:15:01 -0500 Subject: [PATCH 05/21] fix(desktop): address the fourth Copilot review of the hardware apps - I2C: the read-length field goes through the strict field parser too (decimal, 1..64; empty / junk / signed input is refused with a message and no transaction) instead of strtoul + clamp. - Network: Forget checks each NVS erasure (a key that was never stored counts as already forgotten, any other failure does not) and folds that into its result; a `connecting` intent flag (set on Connect and on auto-connect at start, cleared on got-ip / retries exhausted / Disconnect / Forget) makes a scan suppress and restore an association in progress the same way as an established connection, so WifiSta::scan()'s raw disconnect can no longer trigger a retry during the scan. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../example/main/apps/i2c_scanner_app.hpp | 38 +++++++------ .../desktop/example/main/apps/network_app.hpp | 53 ++++++++++++++----- 2 files changed, 62 insertions(+), 29 deletions(-) diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp index fe7c92b4bf..e70757e674 100644 --- a/components/desktop/example/main/apps/i2c_scanner_app.hpp +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -4,7 +4,6 @@ // a short task and list what answers; read / write device registers from the // window. A bus that fails to initialize shows a hint instead of the tools. -#include #include #include #include @@ -47,36 +46,38 @@ struct I2cSession { } return out; } - /// One hex number ("3C" / "0x3C") taking the whole field (blanks around it - /// allowed); nullopt when the field is empty, malformed or above `max`, so - /// a bad entry never becomes a real target (an empty address masked to 0x00 - /// would be the general-call broadcast). - static std::optional parse_field(const std::string &text, unsigned long max) { + /// One unsigned number in `base` taking the whole field (blanks around it + /// allowed); nullopt when the field is empty, malformed (trailing junk, a + /// sign) or outside `min`..`max`, so a bad entry never becomes a real + /// target (an empty address masked to 0x00 would be the general-call + /// broadcast) or a wrong transfer length. + static std::optional parse_field(const std::string &text, int base, + unsigned long min, unsigned long max) { const char *p = text.c_str(); while (*p == ' ') ++p; - if (!*p) + if (!*p || *p == '-' || *p == '+') return std::nullopt; char *end = nullptr; - const unsigned long v = std::strtoul(p, &end, 16); + const unsigned long v = std::strtoul(p, &end, base); if (end == p) return std::nullopt; while (*end == ' ') ++end; - if (*end || v > max) + if (*end || v < min || v > max) return std::nullopt; return v; } - /// A 7-bit device address in the addressable range 0x01..0x7F. + /// A 7-bit device address in the addressable range 0x01..0x7F (hex). static std::optional parse_addr(const std::string &text) { - const auto v = parse_field(text, 0x7F); - if (!v || *v == 0) + const auto v = parse_field(text, 16, 0x01, 0x7F); + if (!v) return std::nullopt; return static_cast(*v); } - /// A full 8-bit register value (0x00..0xFF). + /// A full 8-bit register value (0x00..0xFF, hex). static std::optional parse_byte(const std::string &text) { - const auto v = parse_field(text, 0xFF); + const auto v = parse_field(text, 16, 0x00, 0xFF); if (!v) return std::nullopt; return static_cast(*v); @@ -210,9 +211,12 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { : "register must be a hex value in 0x00..0xFF"); return; } - const size_t len = - std::clamp(std::strtoul(len_box.text().c_str(), nullptr, 10), 1, 64); - std::vector data(len); + const auto len = S::parse_field(len_box.text(), 10, 1, 64); // decimal + if (!len) { + result.set_text("length must be a decimal number 1..64"); + return; + } + std::vector data(static_cast(*len)); if (st->bus->read_at_register(*addr, *reg, data.data(), data.size())) result.set_text("0x{:02X} reg 0x{:02X}: {}", *addr, *reg, S::hex_dump(data)); else diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 9f5b97321e..21fd6e9bdd 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -45,6 +45,9 @@ struct NetworkState { /// A scan is in flight on the scan task: it disconnects and drives the /// station, so every other station action is refused until it is done. std::atomic scanning{false}; + /// The station is associating (Connect / auto-connect at start) but has no + /// IP yet: the intent a scan must suppress and restore like a connection. + std::atomic connecting{false}; #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI std::unique_ptr wifi; std::unique_ptr scan_task; // after wifi: joined before it goes away @@ -89,9 +92,14 @@ struct NetworkState { .num_connect_retries = 3, .auto_connect = true, .on_connected = [this]() { set_status("connected, waiting for an IP"); }, - .on_disconnected = [this]() { set_status("disconnected"); }, + .on_disconnected = + [this]() { // the retries are exhausted: the intent is over + connecting = false; + set_status("disconnected"); + }, .on_got_ip = [this](ip_event_got_ip_t *e) { + connecting = false; set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); }, .log_level = espp::Logger::Verbosity::INFO}; @@ -111,6 +119,18 @@ struct NetworkState { return s; } + /// Erase one key; true when it is gone (it was erased, or was never + /// stored -- NvsHandle::erase() reports a missing key as a failure, so the + /// key is probed first), false on any other failure. + static bool erase_key(espp::NvsHandle &nvs, const char *key) { + std::error_code ec; + std::string probe; + nvs.get(key, probe, ec); // no default: fails when the key is not stored + if (ec) + return true; + return nvs.erase(key, ec); + } + /// Bring the station up once, with the saved credentials (none = idle). void ensure_wifi() { if (wifi) @@ -136,6 +156,7 @@ struct NetworkState { stack.set_storage(WIFI_STORAGE_RAM); auto cfg = wifi_config(ssid, pass); cfg.auto_connect = !ssid.empty(); + connecting = cfg.auto_connect; set_status(ssid.empty() ? "idle (no saved network)" : "connecting"); wifi = std::make_unique(cfg); wifi_mac = wifi->get_mac(); @@ -149,13 +170,14 @@ struct NetworkState { espp::NvsHandle nvs("desktop", ec); bool erased = !ec; if (erased) { - // a missing key is fine (nothing was saved); any other failure is not - std::error_code e1, e2; - nvs.erase("wifi_ssid", e1); - nvs.erase("wifi_pass", e2); + // both erasures must succeed (a key that was never stored counts as + // already forgotten), and the staged change must be committed + erased = erase_key(nvs, "wifi_ssid"); + erased = erase_key(nvs, "wifi_pass") && erased; nvs.commit(ec); // erase_item() only stages the change - erased = !ec; + erased = erased && !ec; } + connecting = false; wifi->disconnect(); wifi_config_t empty{}; const bool cleared = esp_wifi_set_config(WIFI_IF_STA, &empty) == ESP_OK; @@ -296,18 +318,22 @@ inline void register_network_app(espp::Desktop &desktop) { // esp_wifi_disconnect(), which WifiSta's DISCONNECTED // handler treats as unintentional and answers with a // reconnect attempt (num_connect_retries > 0) while the - // scan is starting. Disconnect intentionally first (that - // suppresses the retries), give the event time to land, - // then scan and reconnect ourselves afterwards. - const bool was_connected = state->wifi->is_connected(); - if (was_connected) { + // scan is starting -- whether the station was connected + // or still associating (`connecting`). Disconnect + // intentionally first (that suppresses the retries), + // give the event time to land, then scan and restore + // the intent (connect again) afterwards. + const bool intent = state->wifi->is_connected() || state->connecting; + if (intent) { + state->connecting = false; state->wifi->disconnect(); std::this_thread::sleep_for(std::chrono::milliseconds(200)); } state->set_status("scanning"); const auto aps = state->wifi->scan(20); - if (was_connected) { + if (intent) { state->set_status("reconnecting"); + state->connecting = true; state->wifi->connect(); } else { state->set_status("disconnected"); @@ -368,11 +394,13 @@ inline void register_network_app(espp::Desktop &desktop) { .level = D::NotifyLevel::Warn}); net->set_status(fmt::format("connecting to {}", ssid)); const bool was_connected = net->wifi->is_connected(); + net->connecting = true; // reconfigure() reconnects when it was connected; else connect() bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass)); if (ok && !was_connected) ok = net->wifi->connect(); if (!ok) { + net->connecting = false; net->set_status("connect failed"); d.notify({.title = "Wi-Fi", .text = fmt::format("could not start connecting to {}", ssid), @@ -382,6 +410,7 @@ inline void register_network_app(espp::Desktop &desktop) { disconnect_btn.on_event([=](const D::WidgetEvent &e) mutable { if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) return; + net->connecting = false; net->wifi->disconnect(); net->set_status("disconnected"); }); From c953e44b92f8483455ba304a9e1a09d201ea5c9e Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 03:27:25 -0500 Subject: [PATCH 06/21] fix(desktop): address the fifth Copilot review of the hardware apps - Network: tell "key not stored" from a genuine NVS failure with a raw nvs_get_str() size probe (NvsHandle maps every string read failure to one error code): erase_key() only reports success when the key is gone, and the credential loader treats a genuine failure as "no credentials" and logs it instead of silently reading an empty value. - CANopen: check Task::start(); on failure the session is marked not running, the user is told and run() refuses every command with a toast instead of enqueueing for a task that never consumes; the command queue is bounded (64) regardless. - I2C: on a Task::start() failure release the task, re-enable Scan, restore the status label and show the error. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/canopen_app.hpp | 43 ++++++++++--- .../example/main/apps/i2c_scanner_app.hpp | 12 +++- .../desktop/example/main/apps/network_app.hpp | 63 ++++++++++++++----- 3 files changed, 95 insertions(+), 23 deletions(-) diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index bdf8466eb1..70fd95bd07 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -65,8 +65,9 @@ struct CanopenSession { std::unique_ptr bus; std::mutex mutex; std::condition_variable cv; - std::deque> queue; + std::deque> queue; // bounded by kMaxQueued (see run()) bool stopping{false}; + bool running{false}; // the drive task consumes `queue` (guarded by `mutex`) std::unique_ptr task; /// One unsigned number taking the whole field (blanks around it allowed; @@ -153,13 +154,26 @@ struct CanopenSession { return true; } - /// Queue a bus transaction for the drive task. - void run(std::function fn) { + /// Queue a bus transaction for the drive task; refused (with a toast) when + /// the task is not running or too many commands are already waiting. + static constexpr size_t kMaxQueued = 64; + bool run(std::function fn) { + const char *refused = nullptr; { std::lock_guard lock(mutex); - queue.push_back(std::move(fn)); + if (!running) + refused = "the drive task is not running"; + else if (queue.size() >= kMaxQueued) + refused = "too many commands pending; wait for the bus"; + else + queue.push_back(std::move(fn)); + } + if (refused) { + desktop.notify({.title = "CANopen", .text = refused, .level = D::NotifyLevel::Error}); + return false; } cv.notify_all(); + return true; } /// `what` failed with `ec`: a toast (level error). @@ -171,17 +185,31 @@ struct CanopenSession { desktop.notify({.title = "CANopen", .text = text, .level = D::NotifyLevel::Error}); } - void start() { + /// Start the drive task; false (with a toast) when it could not be started, + /// in which case run() refuses every command. + bool start() { task = std::make_unique( espp::Task::Config{.callback = [this]() { return step(); }, .task_config = {.name = "canopen_ui", .stack_size_bytes = 6 * 1024}}); - task->start(); + const bool started = task->start(); + { + std::lock_guard lock(mutex); + running = started; + } + if (!started) { + task.reset(); + desktop.notify({.title = "CANopen", + .text = "could not start the drive task; the window is inert", + .level = D::NotifyLevel::Error}); + } + return started; } void shutdown() { { std::lock_guard lock(mutex); stopping = true; + running = false; queue.clear(); // queued commands hold a reference to this session } cv.notify_all(); @@ -498,7 +526,8 @@ inline void register_canopen_app(espp::Desktop &desktop) { ec.message()); st->fail("bus init", ec); } - st->start(); + if (!st->start()) + st->state_label.set_text("State: the drive task could not be started"); }, }); } diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp index e70757e674..3b3a6be656 100644 --- a/components/desktop/example/main/apps/i2c_scanner_app.hpp +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -145,7 +145,7 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { auto status = win.label("Press Scan to probe addresses 0x01..0x7F.", bar.id()); auto table = win.table({"Address", "Decimal", "Note"}, {}, nullptr); auto scan_btn = win.button("Scan", nullptr, bar.id(), D::kButtonPrimary); - scan_btn.on_event([=](const D::WidgetEvent &e) mutable { + scan_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { if (e.kind != D::WidgetEventKind::Click) return; if (st->scan && st->scan->is_running()) @@ -179,7 +179,15 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { return true; // one shot }, .task_config = {.name = "i2c_scan", .stack_size_bytes = 4 * 1024}}); - st->scan->start(); + if (!st->scan->start()) { + // no task will ever re-enable the button: restore the window + st->scan.reset(); + scan_btn.set_enabled(true); + status.set_text("Could not start the scan task."); + d.notify({.title = "I2C", + .text = "could not start the scan task (out of memory?)", + .level = D::NotifyLevel::Error}); + } }); // ---- register access ---- diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 21fd6e9bdd..899614d190 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -13,13 +13,17 @@ #include #include #include +#include #include #include #include #include "sdkconfig.h" +#include "nvs.h" + #include "desktop.hpp" +#include "logger.hpp" #include "nvs_handle_espp.hpp" #include "task.hpp" #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI @@ -36,6 +40,7 @@ namespace desktop_example { /// The interfaces, shared by every Network window (created on first launch). struct NetworkState { + espp::Logger logger{{.tag = "Network app", .level = espp::Logger::Verbosity::INFO}}; std::mutex mutex; std::string wifi_status{"idle"}; std::string wifi_ip; @@ -105,29 +110,50 @@ struct NetworkState { .log_level = espp::Logger::Verbosity::INFO}; } - /// A string from NVS ("" when unset). NvsHandle::get() sizes the string to - /// the stored length INCLUDING the terminating NUL, so strip trailing NULs - /// (a 32-byte SSID would otherwise come back as 33 bytes). - static std::string nvs_string(espp::NvsHandle &nvs, const char *key) { + /// Whether a string key exists in the app's namespace: ESP_OK (it does), + /// ESP_ERR_NVS_NOT_FOUND (it does not) or another error (a genuine NVS + /// failure). NvsHandle maps every string read failure to one error code, + /// so the raw nvs_get_str() size query is used to tell the two apart. + static esp_err_t probe_key(const char *key) { + nvs_handle_t h = 0; + esp_err_t err = nvs_open("desktop", NVS_READONLY, &h); + if (err != ESP_OK) + return err; // ESP_ERR_NVS_NOT_FOUND when the namespace was never written + size_t len = 0; + err = nvs_get_str(h, key, nullptr, &len); + nvs_close(h); + return err; + } + + /// A string from NVS: "" when the key is not stored, nullopt on a genuine + /// NVS failure. NvsHandle::get() sizes the string to the stored length + /// INCLUDING the terminating NUL, so trailing NULs are stripped (a 32-byte + /// SSID would otherwise come back as 33 bytes). + static std::optional nvs_string(espp::NvsHandle &nvs, const char *key) { + const esp_err_t probed = probe_key(key); + if (probed == ESP_ERR_NVS_NOT_FOUND) + return std::string{}; + if (probed != ESP_OK) + return std::nullopt; std::error_code ec; std::string s; - nvs.get(key, s, std::string(""), ec); + nvs.get(key, s, ec); // no default: never writes if (ec) - return ""; + return std::nullopt; while (!s.empty() && s.back() == '\0') s.pop_back(); return s; } /// Erase one key; true when it is gone (it was erased, or was never - /// stored -- NvsHandle::erase() reports a missing key as a failure, so the - /// key is probed first), false on any other failure. + /// stored), false on a genuine NVS failure (probe or erase). static bool erase_key(espp::NvsHandle &nvs, const char *key) { - std::error_code ec; - std::string probe; - nvs.get(key, probe, ec); // no default: fails when the key is not stored - if (ec) + const esp_err_t probed = probe_key(key); + if (probed == ESP_ERR_NVS_NOT_FOUND) return true; + if (probed != ESP_OK) + return false; + std::error_code ec; return nvs.erase(key, ec); } @@ -139,8 +165,17 @@ struct NetworkState { espp::NvsHandle nvs("desktop", ec); std::string ssid, pass; if (!ec) { - ssid = nvs_string(nvs, "wifi_ssid"); - pass = nvs_string(nvs, "wifi_pass"); + const auto saved_ssid = nvs_string(nvs, "wifi_ssid"); + const auto saved_pass = nvs_string(nvs, "wifi_pass"); + if (saved_ssid && saved_pass) { + ssid = *saved_ssid; + pass = *saved_pass; + } else { + // a genuine NVS failure (not "nothing saved"): start idle, say why + logger.error("could not read the saved Wi-Fi credentials from NVS; starting idle"); + } + } else { + logger.error("could not open the NVS namespace: {}", ec.message()); } // the same bounds Connect enforces before saving (the driver fields are // 32 / 64 bytes); anything else in flash is treated as no credentials From 0deb512c5d69ae43ddd68cfff5d67d1a6ac17082 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 03:39:12 -0500 Subject: [PATCH 07/21] fix(desktop): address the sixth Copilot review of the hardware apps - CANopen: only append the client's last SDO abort code to a failure toast when the failing call was an SDO transaction (set mode, the controlword / statusword drive commands, inject fault, target velocity, SDO read / write); NMT sends and bus init never touch it, so appending it there showed a stale, unrelated abort. - Network: Forget reports the NVS erasure and the station reset separately; the persistent status only claims "no saved network" when the erase + commit succeeded and otherwise says the credentials may still be saved, with a matching error / warning / ok toast. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/canopen_app.hpp | 21 +++++++------ .../desktop/example/main/apps/network_app.hpp | 30 ++++++++++++++----- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index 70fd95bd07..034330ba09 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -176,10 +176,13 @@ struct CanopenSession { return true; } - /// `what` failed with `ec`: a toast (level error). - void fail(std::string_view what, const std::error_code &ec) { + /// `what` failed with `ec`: a toast (level error). `sdo` says the failing + /// call was an SDO transaction, the only case in which the client's + /// last_abort_code() describes THIS failure (NMT sends and bus init never + /// touch it, so appending it there would show a stale, unrelated abort). + void fail(std::string_view what, const std::error_code &ec, bool sdo = false) { std::string text = fmt::format("{}: {}", what, ec.message()); - if (client && client->last_abort_code()) + if (sdo && client && client->last_abort_code()) text += fmt::format(" (SDO abort 0x{:08X}: {})", client->last_abort_code(), espp::CanopenClient::abort_code_to_string(client->last_abort_code())); desktop.notify({.title = "CANopen", .text = text, .level = D::NotifyLevel::Error}); @@ -367,7 +370,7 @@ inline void register_canopen_app(espp::Desktop &desktop) { st->run([=]() { std::error_code ec; if (!st->drive || !st->drive->set_mode(m, ec)) - st->fail("set mode", ec); + st->fail("set mode", ec, /*sdo=*/true); }); }, mode_row.id()); @@ -380,7 +383,7 @@ inline void register_canopen_app(espp::Desktop &desktop) { st->run([=]() { std::error_code ec; if (!st->drive || !((*st->drive).*fn)(ec)) - st->fail(text, ec); + st->fail(text, ec, /*sdo=*/true); // controlword / statusword SDOs else st->desktop.notify({.title = "CANopen", .text = fmt::format("{}: ok", text), @@ -401,7 +404,7 @@ inline void register_canopen_app(espp::Desktop &desktop) { st->run([=]() { // manufacturer object 0x2000 of the simulated node std::error_code ec; if (!st->client || !st->client->write_u8(0x2000, 0, 1, ec)) - st->fail("inject fault", ec); + st->fail("inject fault", ec, /*sdo=*/true); }); }, ctl.id()); @@ -422,7 +425,7 @@ inline void register_canopen_app(espp::Desktop &desktop) { st->run([=]() { std::error_code ec; if (!st->drive || !st->drive->set_target_velocity(v, ec)) - st->fail("target velocity", ec); + st->fail("target velocity", ec, /*sdo=*/true); }); }, vel_row.id(), D::kButtonPrimary); @@ -469,7 +472,7 @@ inline void register_canopen_app(espp::Desktop &desktop) { if (!n) { st->sdo_result.set_text("read 0x{:04X}:{:02X} failed: {}", index, sub, ec.message()); - st->fail("SDO read", ec); + st->fail("SDO read", ec, /*sdo=*/true); return; } const uint32_t v = espp::detail::canopen::get_le(buf.data(), n); @@ -512,7 +515,7 @@ inline void register_canopen_app(espp::Desktop &desktop) { else { st->sdo_result.set_text("write 0x{:04X}:{:02X} failed: {}", index, sub, ec.message()); - st->fail("SDO write", ec); + st->fail("SDO write", ec, /*sdo=*/true); } }); }, diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 899614d190..3417794d3d 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -197,10 +197,16 @@ struct NetworkState { wifi_mac = wifi->get_mac(); } + /// What forget() managed to do (each part is reported separately). + struct ForgetResult { + bool erased{false}; ///< the NVS keys are gone (erased + committed, or never stored) + bool station_reset{false}; ///< the driver + WifiSta configs are empty + }; + /// Drop the saved credentials everywhere: the app's NVS keys, the driver's /// station config and WifiSta's stored config (reconfigured with empty /// credentials and auto-connect off, so nothing reconnects). - bool forget() { + ForgetResult forget() { std::error_code ec; espp::NvsHandle nvs("desktop", ec); bool erased = !ec; @@ -219,9 +225,12 @@ struct NetworkState { auto cfg = wifi_config("", ""); // empty ssid: reconfigure() adopts the (now empty) driver config cfg.auto_connect = false; - const bool ok = wifi->reconfigure(cfg) && cleared && erased; - set_status("idle (no saved network)"); - return ok; + const bool station_reset = wifi->reconfigure(cfg) && cleared; + // the persistent status must not claim the credentials are gone when the + // NVS erase / commit failed + set_status(erased ? "idle (no saved network)" + : "Forget failed: the credentials may still be saved"); + return {.erased = erased, .station_reset = station_reset}; } /// Save the credentials (committed, so they survive a reboot). @@ -452,11 +461,16 @@ inline void register_network_app(espp::Desktop &desktop) { forget_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) return; - const bool ok = net->forget(); + const auto r = net->forget(); d.notify({.title = "Wi-Fi", - .text = ok ? "saved network forgotten" - : "credentials erased, but the station could not be reset", - .level = ok ? D::NotifyLevel::Ok : D::NotifyLevel::Warn}); + .text = !r.erased ? "Forget failed: the credentials may still be saved " + "in NVS" + : !r.station_reset + ? "credentials erased, but the station could not be reset" + : "saved network forgotten", + .level = !r.erased ? D::NotifyLevel::Error + : !r.station_reset ? D::NotifyLevel::Warn + : D::NotifyLevel::Ok}); }); #endif From 59796c5c143abe85e4bfb2baabf7aac9caee168e Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 03:55:33 -0500 Subject: [PATCH 08/21] fix(desktop): address the seventh Copilot review of the hardware apps - CANopen: size the SDO Read upload span from the width selector (1 / 2 / 4 bytes) -- sdo_upload() returns the caller's width when an expedited response omits its size, so a fixed 4-byte span showed un-sized u8 / u16 objects as 4 bytes. - Network: track DESIRED connectivity (`want_connected`: set by Connect and the saved-credentials auto-connect, cleared only by Disconnect / Forget) instead of a transient association flag that on_disconnected cleared -- it also fires for the intentional disconnect inside reconfigure() when switching APs; the scan suppresses / restores the station from the desired state. Handle a Wi-Fi scan Task::start() failure (release the task, clear busy, restore the label, toast). - Ethernet: a DESKTOP_EXAMPLE_ETHERNET_BOARD choice selects the RMII wiring (ESP32-Ethernet-Kit on the ESP32, ESP32-P4-Function-EV-Board with its routable data pins on the P4 -- the gate already allowed the P4, whose EMAC needs RmiiConfig::data_pins); Kconfig help + README updated. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- components/desktop/example/README.md | 6 +- .../desktop/example/main/Kconfig.projbuild | 25 +++++- .../desktop/example/main/apps/canopen_app.hpp | 10 ++- .../desktop/example/main/apps/network_app.hpp | 88 ++++++++++++------- 4 files changed, 94 insertions(+), 35 deletions(-) diff --git a/components/desktop/example/README.md b/components/desktop/example/README.md index a3e0c81cba..8695de0cc1 100644 --- a/components/desktop/example/README.md +++ b/components/desktop/example/README.md @@ -71,8 +71,9 @@ The hardware apps need nothing extra by default: the CANopen app talks to a simulated node, the I2C scanner just reports an empty bus and the Network app scans for Wi-Fi. For a real CAN bus select the TWAI peripheral and wire a transceiver (SN65HVD230 or similar) to the configured TX / RX GPIOs; for the -Ethernet group an ESP32-Ethernet-Kit-style RMII PHY (the pins are in -`main/apps/network_app.hpp`). +Ethernet group an ESP32-Ethernet-Kit or an ESP32-P4-Function-EV-Board (the +RMII wiring is selected by `DESKTOP_EXAMPLE_ETHERNET_BOARD`; other boards: +edit `rmii_config()` in `main/apps/network_app.hpp`). ### Configuration @@ -89,6 +90,7 @@ Ethernet group an ESP32-Ethernet-Kit-style RMII PHY (the pins are in | `DESKTOP_EXAMPLE_I2C_PORT` / `_SDA_GPIO` / `_SCL_GPIO` / `_FREQ_HZ` | 0 / 8 / 9 / 400000 | The I2C bus it scans | | `DESKTOP_EXAMPLE_ENABLE_WIFI` | y | The Network app's Wi-Fi station group (`SOC_WIFI_SUPPORTED`) | | `DESKTOP_EXAMPLE_ENABLE_ETHERNET` | n | The Network app's RMII Ethernet group (`SOC_EMAC_SUPPORTED`: ESP32 / -P4) | +| `DESKTOP_EXAMPLE_ETHERNET_BOARD` | per target | RMII wiring: `ETHERNET_KIT` (ESP32-Ethernet-Kit) or `P4_FUNCTION_EV` (ESP32-P4-Function-EV-Board, with its routable data pins) | The hardware components (`canopen`, `twai`, `i2c`, `wifi`, `ethernet`, `cli`) are always part of the build (`REQUIRES` cannot depend on Kconfig); the diff --git a/components/desktop/example/main/Kconfig.projbuild b/components/desktop/example/main/Kconfig.projbuild index ef95d573b2..9210c2cd21 100644 --- a/components/desktop/example/main/Kconfig.projbuild +++ b/components/desktop/example/main/Kconfig.projbuild @@ -109,7 +109,28 @@ menu "Desktop Example Configuration" default n help Register the Network app's Ethernet group: link / IP / MAC / speed - of the internal EMAC over RMII (espp::Ethernet, ESP32-Ethernet-Kit - pins in network_app.hpp). + of the internal EMAC over RMII (espp::Ethernet) with the wiring of + the board selected below. + + choice DESKTOP_EXAMPLE_ETHERNET_BOARD + prompt "RMII board wiring" + depends on DESKTOP_EXAMPLE_ENABLE_ETHERNET + default DESKTOP_EXAMPLE_ETHERNET_BOARD_ETHERNET_KIT if IDF_TARGET_ESP32 + default DESKTOP_EXAMPLE_ETHERNET_BOARD_P4_FUNCTION_EV if IDF_TARGET_ESP32P4 + help + Which board's RMII pins network_app.hpp uses: the ESP32-Ethernet-Kit + (IP101 PHY, MDC 23 / MDIO 18 / reset 5, 50 MHz clock in on GPIO 0; + the ESP32's data pins are fixed) or the ESP32-P4-Function-EV-Board + (IP101 PHY, MDC 31 / MDIO 52 / reset 51, clock in on GPIO 50, and + the routable data pins TX_EN 49 / TXD0 34 / TXD1 35 / CRS_DV 28 / + RXD0 29 / RXD1 30). Other boards: edit rmii_config(). + + config DESKTOP_EXAMPLE_ETHERNET_BOARD_ETHERNET_KIT + bool "ESP32-Ethernet-Kit" + depends on IDF_TARGET_ESP32 + config DESKTOP_EXAMPLE_ETHERNET_BOARD_P4_FUNCTION_EV + bool "ESP32-P4-Function-EV-Board" + depends on IDF_TARGET_ESP32P4 + endchoice endmenu diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index 034330ba09..52a65248a9 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -465,10 +466,17 @@ inline void register_canopen_app(espp::Desktop &desktop) { if (!obj) return; const auto [index, sub] = *obj; + // the width selector sizes the upload buffer: when an + // expedited response omits its size, sdo_upload() returns + // the caller's width, so a 4-byte span would show an + // un-sized u8 / u16 object as 4 bytes + const int32_t w = width.selected(); + const size_t want = w == 0 ? 1 : w == 1 ? 2 : 4; st->run([=]() { std::error_code ec; std::array buf{}; - const size_t n = st->client ? st->client->sdo_upload(index, sub, buf, ec) : 0; + const std::span out(buf.data(), want); + const size_t n = st->client ? st->client->sdo_upload(index, sub, out, ec) : 0; if (!n) { st->sdo_result.set_text("read 0x{:04X}:{:02X} failed: {}", index, sub, ec.message()); diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 3417794d3d..c23d2c73e6 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -50,9 +50,13 @@ struct NetworkState { /// A scan is in flight on the scan task: it disconnects and drives the /// station, so every other station action is refused until it is done. std::atomic scanning{false}; - /// The station is associating (Connect / auto-connect at start) but has no - /// IP yet: the intent a scan must suppress and restore like a connection. - std::atomic connecting{false}; + /// DESIRED connectivity: set by Connect and by the saved-credentials + /// auto-connect at start, cleared only by Disconnect / Forget -- never by + /// the driver callbacks (on_disconnected also fires for the intentional + /// disconnect inside WifiSta::reconfigure(), e.g. when switching APs). A + /// scan suppresses the station while it wants to be connected and restores + /// it afterwards, whatever the transient association state. + std::atomic want_connected{false}; #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI std::unique_ptr wifi; std::unique_ptr scan_task; // after wifi: joined before it goes away @@ -98,13 +102,13 @@ struct NetworkState { .auto_connect = true, .on_connected = [this]() { set_status("connected, waiting for an IP"); }, .on_disconnected = - [this]() { // the retries are exhausted: the intent is over - connecting = false; - set_status("disconnected"); + [this]() { // retries exhausted, or reconfigure()'s own disconnect + set_status(want_connected ? "disconnected (retries exhausted; Connect or a " + "scan retries)" + : "disconnected"); }, .on_got_ip = [this](ip_event_got_ip_t *e) { - connecting = false; set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); }, .log_level = espp::Logger::Verbosity::INFO}; @@ -191,7 +195,7 @@ struct NetworkState { stack.set_storage(WIFI_STORAGE_RAM); auto cfg = wifi_config(ssid, pass); cfg.auto_connect = !ssid.empty(); - connecting = cfg.auto_connect; + want_connected = cfg.auto_connect; set_status(ssid.empty() ? "idle (no saved network)" : "connecting"); wifi = std::make_unique(cfg); wifi_mac = wifi->get_mac(); @@ -218,7 +222,7 @@ struct NetworkState { nvs.commit(ec); // erase_item() only stages the change erased = erased && !ec; } - connecting = false; + want_connected = false; wifi->disconnect(); wifi_config_t empty{}; const bool cleared = esp_wifi_set_config(WIFI_IF_STA, &empty) == ESP_OK; @@ -249,18 +253,36 @@ struct NetworkState { #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET - /// Bring the RMII link up once (ESP32-Ethernet-Kit pins: IP101 PHY, 50 MHz - /// reference clock in on GPIO0; adjust for your board). + /// The RMII wiring of the board Kconfig selects (IP101 PHY at address 1, + /// 50 MHz reference clock in). The ESP32's EMAC data pins are fixed by the + /// IO_MUX; the ESP32-P4's are routable and must be given (the pins of the + /// ESP32-P4-Function-EV-Board, as in components/esp32-p4-function-ev-board). + static espp::Ethernet::RmiiConfig rmii_config() { +#if CONFIG_DESKTOP_EXAMPLE_ETHERNET_BOARD_P4_FUNCTION_EV + return {.mdc_gpio = 31, + .mdio_gpio = 52, + .phy_addr = 1, + .phy_reset_gpio = 51, + .clock_ext_in = true, + .clock_gpio = 50, + .data_pins = espp::Ethernet::RmiiConfig::DataPins{ + .tx_en = 49, .txd0 = 34, .txd1 = 35, .crs_dv = 28, .rxd0 = 29, .rxd1 = 30}}; +#else // ESP32-Ethernet-Kit + return {.mdc_gpio = 23, + .mdio_gpio = 18, + .phy_addr = 1, + .phy_reset_gpio = 5, + .clock_ext_in = true, + .clock_gpio = 0}; +#endif + } + + /// Bring the RMII link up once. void ensure_ethernet() { if (eth) return; eth = std::make_unique( - espp::Ethernet::Config{.interface = espp::Ethernet::RmiiConfig{.mdc_gpio = 23, - .mdio_gpio = 18, - .phy_addr = 1, - .phy_reset_gpio = 5, - .clock_ext_in = true, - .clock_gpio = 0}, + espp::Ethernet::Config{.interface = rmii_config(), .hostname = "espp-desktop", .log_level = espp::Logger::Verbosity::INFO}); std::error_code ec; @@ -347,7 +369,7 @@ inline void register_network_app(espp::Desktop &desktop) { rssi_label.set_text("RSSI: -"); }); - scan_btn.on_event([=](const D::WidgetEvent &e) mutable { + scan_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) return; net->scanning = true; @@ -363,13 +385,12 @@ inline void register_network_app(espp::Desktop &desktop) { // handler treats as unintentional and answers with a // reconnect attempt (num_connect_retries > 0) while the // scan is starting -- whether the station was connected - // or still associating (`connecting`). Disconnect - // intentionally first (that suppresses the retries), - // give the event time to land, then scan and restore - // the intent (connect again) afterwards. - const bool intent = state->wifi->is_connected() || state->connecting; + // or still associating. The DESIRED state decides: + // disconnect intentionally first (that suppresses the + // retries), give the event time to land, then scan and + // restore it (connect again) afterwards. + const bool intent = state->want_connected || state->wifi->is_connected(); if (intent) { - state->connecting = false; state->wifi->disconnect(); std::this_thread::sleep_for(std::chrono::milliseconds(200)); } @@ -377,7 +398,6 @@ inline void register_network_app(espp::Desktop &desktop) { const auto aps = state->wifi->scan(20); if (intent) { state->set_status("reconnecting"); - state->connecting = true; state->wifi->connect(); } else { state->set_status("disconnected"); @@ -403,7 +423,16 @@ inline void register_network_app(espp::Desktop &desktop) { return true; // one shot }, .task_config = {.name = "wifi_scan", .stack_size_bytes = 6 * 1024}}); - net->scan_task->start(); + if (!net->scan_task->start()) { + // no task will ever release the busy state: restore the window + net->scan_task.reset(); + net->scanning = false; + set_busy(false); + scan_label.set_text("could not start the scan task"); + d.notify({.title = "Wi-Fi", + .text = "could not start the scan task (out of memory?)", + .level = D::NotifyLevel::Error}); + } }); connect_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { @@ -438,14 +467,13 @@ inline void register_network_app(espp::Desktop &desktop) { .level = D::NotifyLevel::Warn}); net->set_status(fmt::format("connecting to {}", ssid)); const bool was_connected = net->wifi->is_connected(); - net->connecting = true; + net->want_connected = true; // reconfigure() reconnects when it was connected; else connect() bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass)); if (ok && !was_connected) ok = net->wifi->connect(); if (!ok) { - net->connecting = false; - net->set_status("connect failed"); + net->set_status("connect failed (Connect or a scan retries)"); d.notify({.title = "Wi-Fi", .text = fmt::format("could not start connecting to {}", ssid), .level = D::NotifyLevel::Error}); @@ -454,7 +482,7 @@ inline void register_network_app(espp::Desktop &desktop) { disconnect_btn.on_event([=](const D::WidgetEvent &e) mutable { if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) return; - net->connecting = false; + net->want_connected = false; net->wifi->disconnect(); net->set_status("disconnected"); }); From 33828195ac1a333c7743c802cccde5d26779f6f8 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 04:16:47 -0500 Subject: [PATCH 09/21] fix(desktop): address the eighth Copilot review of the hardware apps - CANopen: one error code per telemetry read in poll() -- CanopenClient only sets ec on failure and never clears it, so a shared one hid every field after a failed read. - Network: Disconnect / Forget go through stop_station(), which only calls WifiSta::disconnect() when the station is connected or a Connect was issued (an association may be in flight); on an idle station no DISCONNECTED event would clear WifiSta's private disconnecting_ flag and the next Connect would lose its retries. A scan-result generation counter lets a window's 1 s refresher re-sync the AP list from the shared rows when a scan completes after the window opened (the scan task outlives windows and only updates the one it started from). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/canopen_app.hpp | 15 ++++---- .../desktop/example/main/apps/network_app.hpp | 34 ++++++++++++++++--- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index 52a65248a9..0b391c410e 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -256,14 +256,17 @@ struct CanopenSession { state_label.set_text("State: {}", espp::Ds402Drive::to_string(espp::Ds402Drive::state_from_statusword(sw))); status_label.set_text("Statusword: 0x{:04X}", sw); - const int8_t mode = drive->get_mode_display(ec); - if (!ec) + // one error code per field: the client only sets ec on failure (never + // clears it), so a shared one would hide every field after a failed read + std::error_code mode_ec, pos_ec, vel_ec; + const int8_t mode = drive->get_mode_display(mode_ec); + if (!mode_ec) mode_label.set_text("Mode (0x6061): {}", mode); - const int32_t pos = drive->get_position_actual(ec); - if (!ec) + const int32_t pos = drive->get_position_actual(pos_ec); + if (!pos_ec) pos_label.set_text("Position: {}", pos); - const int32_t vel = drive->get_velocity_actual(ec); - if (!ec) + const int32_t vel = drive->get_velocity_actual(vel_ec); + if (!vel_ec) vel_label.set_text("Velocity: {}", vel); } }; diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index c23d2c73e6..e201e2f6a2 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -47,6 +47,7 @@ struct NetworkState { std::string wifi_mac; std::vector scan_ssids; // in AP list order std::vector scan_rows; // the AP list's items (shown again by a new window) + uint32_t scan_generation{0}; // bumped per completed scan (windows re-sync on change) /// A scan is in flight on the scan task: it disconnects and drives the /// station, so every other station action is refused until it is done. std::atomic scanning{false}; @@ -149,6 +150,18 @@ struct NetworkState { return s; } + /// Give up the desired connectivity and disconnect -- but only when the + /// station is connected or a Connect was issued (an association may be in + /// flight). WifiSta::disconnect() sets its private `disconnecting_`, which + /// only the DISCONNECTED event clears: on an idle station no event comes, + /// the flag goes stale and the next Connect loses its retries on the + /// first failure. + void stop_station() { + const bool was_wanted = want_connected.exchange(false); + if (wifi->is_connected() || was_wanted) + wifi->disconnect(); + } + /// Erase one key; true when it is gone (it was erased, or was never /// stored), false on a genuine NVS failure (probe or erase). static bool erase_key(espp::NvsHandle &nvs, const char *key) { @@ -222,8 +235,7 @@ struct NetworkState { nvs.commit(ec); // erase_item() only stages the change erased = erased && !ec; } - want_connected = false; - wifi->disconnect(); + stop_station(); wifi_config_t empty{}; const bool cleared = esp_wifi_set_config(WIFI_IF_STA, &empty) == ESP_OK; auto cfg = @@ -318,9 +330,11 @@ inline void register_network_app(espp::Desktop &desktop) { auto scan_btn = win.button("Scan", nullptr, scan_bar.id()); auto scan_label = win.label("", scan_bar.id()); std::vector rows_now; + auto seen_generation = std::make_shared(0); { std::lock_guard lock(net->mutex); rows_now = net->scan_rows; // the last scan, if any + *seen_generation = net->scan_generation; } auto ap_list = win.list(rows_now, nullptr, wifi.id()); ap_list.set_size(0, 140); @@ -352,10 +366,22 @@ inline void register_network_app(espp::Desktop &desktop) { refreshers.push_back([=]() mutable { std::string st, ip; + std::optional> new_rows; { std::lock_guard lock(net->mutex); st = net->wifi_status; ip = net->wifi_ip; + // a scan that completed since this window opened (the scan + // task outlives windows and only updates the one it started + // from): re-sync the list from the shared rows + if (net->scan_generation != *seen_generation) { + *seen_generation = net->scan_generation; + new_rows = net->scan_rows; + } + } + if (new_rows) { + ap_list.set_items(*new_rows); + scan_label.set_text("{} networks", new_rows->size()); } // also re-syncs a window opened while a scan started elsewhere set_busy(net->scanning); @@ -415,6 +441,7 @@ inline void register_network_app(espp::Desktop &desktop) { std::lock_guard lock(state->mutex); state->scan_ssids = ssids; state->scan_rows = rows; + ++state->scan_generation; } ap_list.set_items(rows); scan_label.set_text("{} networks", rows.size()); @@ -482,8 +509,7 @@ inline void register_network_app(espp::Desktop &desktop) { disconnect_btn.on_event([=](const D::WidgetEvent &e) mutable { if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) return; - net->want_connected = false; - net->wifi->disconnect(); + net->stop_station(); net->set_status("disconnected"); }); forget_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { From 76045b5bde3dcccef3cf34b2575d7a049acd42db Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 04:26:49 -0500 Subject: [PATCH 10/21] fix(desktop): SDO value field parses decimal unless prefixed with 0x (no implicit octal) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- components/desktop/example/main/apps/canopen_app.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index 0b391c410e..1e0622b5f8 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -81,6 +81,10 @@ struct CanopenSession { ++p; if (!*p || *p == '-' || *p == '+') return std::nullopt; + // base 0 would also treat a leading zero as octal ("010" -> 8); the UI + // documents decimal with an optional 0x prefix, so choose the base here + if (base == 0) + base = (p[0] == '0' && (p[1] == 'x' || p[1] == 'X')) ? 16 : 10; char *end = nullptr; errno = 0; const unsigned long v = std::strtoul(p, &end, base); From 7fefe072e2711f0fa7e9b04509361bc8170bb826 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 04:38:50 -0500 Subject: [PATCH 11/21] fix(desktop): address the tenth Copilot review of the hardware apps - CANopen: clear every status widget (clear_status()) when a bus init fails, so the previous node's state / measurements are not left up while nothing is talking to it. - I2C: a rescan clears the table and its address mapping before the worker starts and keeps the table disabled for the whole scan, so a selection can never map an old row to a new address. - Network: an `associating` flag (set once connect() / the start-up auto-connect was issued, cleared on got-ip, on retries exhausted and on an explicit stop) decides whether WifiSta::disconnect() is called at all -- only when connected or associating, where a DISCONNECTED event will clear WifiSta's private disconnecting_; `want_connected` stays the desired-state flag only (it may be true with the station idle after the retries). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/canopen_app.hpp | 12 +++++++ .../example/main/apps/i2c_scanner_app.hpp | 13 +++++++ .../desktop/example/main/apps/network_app.hpp | 34 +++++++++++++++---- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index 1e0622b5f8..2eeeaf38c6 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -153,12 +153,24 @@ struct CanopenSession { bus = std::make_unique(cfg); if (!bus->initialize(ec)) { bus.reset(); + clear_status("no bus"); // nothing polls now: do not leave the old node's values up return false; } nmt_label.set_text("NMT state: (nothing heard from node {} yet)", id); return true; } + /// Reset every status widget (the previous node's values must not stay + /// shown once nothing is talking to it). + void clear_status(std::string_view why) { + nmt_label.set_text("NMT state: - ({})", why); + state_label.set_text("State: - ({})", why); + status_label.set_text("Statusword: -"); + mode_label.set_text("Mode (0x6061): -"); + pos_label.set_text("Position: -"); + vel_label.set_text("Velocity: -"); + } + /// Queue a bus transaction for the drive task; refused (with a toast) when /// the task is not running or too many commands are already waiting. static constexpr size_t kMaxQueued = 64; diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp index 3b3a6be656..b0973179bb 100644 --- a/components/desktop/example/main/apps/i2c_scanner_app.hpp +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -152,6 +152,17 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { return; st->scan.reset(); scan_btn.set_enabled(false); + // the old rows and their address mapping go BEFORE the worker + // starts (it publishes the new mapping before the new rows) and + // the table stays disabled for the whole scan, so a selection + // can never map an old row to a new address + { + std::lock_guard lock(st->mutex); + st->found.clear(); + } + table.set_items({}); + table.set_selected(D::kNoSelection); + table.set_enabled(false); status.set_text("Scanning\xE2\x80\xA6"); // the bus outlives the task: on_close joins the scan first auto *bus = st->bus.get(); @@ -173,6 +184,7 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { session->found = found; } table.set_items(rows); + table.set_enabled(true); status.set_text("{} device{} found", rows.size(), rows.size() == 1 ? "" : "s"); scan_btn.set_enabled(true); @@ -183,6 +195,7 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { // no task will ever re-enable the button: restore the window st->scan.reset(); scan_btn.set_enabled(true); + table.set_enabled(true); status.set_text("Could not start the scan task."); d.notify({.title = "I2C", .text = "could not start the scan task (out of memory?)", diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index e201e2f6a2..722b0841ae 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -58,6 +58,12 @@ struct NetworkState { /// scan suppresses the station while it wants to be connected and restores /// it afterwards, whatever the transient association state. std::atomic want_connected{false}; + /// An association is in progress (connect() was issued and neither an IP + /// nor "retries exhausted" arrived yet): the only state, besides being + /// connected, in which WifiSta::disconnect() is safe to call -- on an idle + /// station it sets its private `disconnecting_` and no DISCONNECTED event + /// ever clears it, so the next Connect would lose its retries. + std::atomic associating{false}; #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI std::unique_ptr wifi; std::unique_ptr scan_task; // after wifi: joined before it goes away @@ -104,12 +110,14 @@ struct NetworkState { .on_connected = [this]() { set_status("connected, waiting for an IP"); }, .on_disconnected = [this]() { // retries exhausted, or reconfigure()'s own disconnect + associating = false; set_status(want_connected ? "disconnected (retries exhausted; Connect or a " "scan retries)" : "disconnected"); }, .on_got_ip = [this](ip_event_got_ip_t *e) { + associating = false; set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); }, .log_level = espp::Logger::Verbosity::INFO}; @@ -157,8 +165,15 @@ struct NetworkState { /// the flag goes stale and the next Connect loses its retries on the /// first failure. void stop_station() { - const bool was_wanted = want_connected.exchange(false); - if (wifi->is_connected() || was_wanted) + want_connected = false; + disconnect_if_active(); + } + + /// WifiSta::disconnect() only when it will produce a DISCONNECTED event + /// (connected, or an association in progress); see `associating`. + void disconnect_if_active() { + const bool was_associating = associating.exchange(false); + if (wifi->is_connected() || was_associating) wifi->disconnect(); } @@ -209,6 +224,7 @@ struct NetworkState { auto cfg = wifi_config(ssid, pass); cfg.auto_connect = !ssid.empty(); want_connected = cfg.auto_connect; + associating = cfg.auto_connect; // WifiSta connects from its STA_START event set_status(ssid.empty() ? "idle (no saved network)" : "connecting"); wifi = std::make_unique(cfg); wifi_mac = wifi->get_mac(); @@ -416,15 +432,16 @@ inline void register_network_app(espp::Desktop &desktop) { // retries), give the event time to land, then scan and // restore it (connect again) afterwards. const bool intent = state->want_connected || state->wifi->is_connected(); - if (intent) { - state->wifi->disconnect(); + if (state->wifi->is_connected() || state->associating) { + state->disconnect_if_active(); std::this_thread::sleep_for(std::chrono::milliseconds(200)); } state->set_status("scanning"); const auto aps = state->wifi->scan(20); if (intent) { state->set_status("reconnecting"); - state->wifi->connect(); + if (state->wifi->connect()) + state->associating = true; } else { state->set_status("disconnected"); } @@ -495,10 +512,15 @@ inline void register_network_app(espp::Desktop &desktop) { net->set_status(fmt::format("connecting to {}", ssid)); const bool was_connected = net->wifi->is_connected(); net->want_connected = true; - // reconfigure() reconnects when it was connected; else connect() + // reconfigure() reconnects when it was connected; else connect(). + // `associating` is set only once the association was issued: + // reconfigure()'s own disconnect fires on_disconnected first, + // which would clear it again. bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass)); if (ok && !was_connected) ok = net->wifi->connect(); + if (ok) + net->associating = true; if (!ok) { net->set_status("connect failed (Connect or a scan retries)"); d.notify({.title = "Wi-Fi", From f215329ab8938155f37d0530b02ac6045375ffd7 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 04:54:15 -0500 Subject: [PATCH 12/21] fix(desktop): only require canopen / twai on IDF >= 6; bring Ethernet up before Wi-Fi - The CANopen app is built on espp's twai component (the simulated bus is a Twai drop-in), which needs the IDF >= 6.0 esp_driver_twai node API, while the rest of the example builds on IDF 5.5. The example's CMakeLists now adds canopen + twai to EXTRA_COMPONENT_DIRS / COMPONENTS / main's REQUIRES and compiles the app (DESKTOP_EXAMPLE_CANOPEN_AVAILABLE) only when the CMake option DESKTOP_EXAMPLE_CANOPEN is ON (default: IDF >= 6, like the cli component's version.cmake gate; override with -DDESKTOP_EXAMPLE_CANOPEN= OFF). The decision reaches main/CMakeLists.txt as a build property, since IDF evaluates REQUIRES in a separate script process without the CMake cache. With the app not built, CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN has no effect and app_main logs a warning. README documents the minimum IDF. - Network: when both groups are enabled, bring Ethernet up before the Wi-Fi stack -- espp::Ethernet::initialize() treats esp_netif_init()'s ESP_ERR_INVALID_STATE (already initialized) as fatal while Wifi::init() tolerates it. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- components/desktop/example/CMakeLists.txt | 36 +++++++++++++++++-- components/desktop/example/README.md | 22 ++++++++---- .../desktop/example/main/CMakeLists.txt | 33 ++++++++++++----- .../desktop/example/main/Kconfig.projbuild | 6 ++++ .../desktop/example/main/apps/network_app.hpp | 8 ++++- .../desktop/example/main/desktop_example.cpp | 14 ++++++-- 6 files changed, 98 insertions(+), 21 deletions(-) diff --git a/components/desktop/example/CMakeLists.txt b/components/desktop/example/CMakeLists.txt index 8b23022922..b543a4f73a 100644 --- a/components/desktop/example/CMakeLists.txt +++ b/components/desktop/example/CMakeLists.txt @@ -18,10 +18,40 @@ set(CMAKE_CXX_STANDARD 20) # from the vendored git submodules under external/ and littlefs from the # components/littlefs submodule. include($ENV{IDF_PATH}/tools/cmake/project.cmake) +include($ENV{IDF_PATH}/tools/cmake/version.cmake) + +# The CANopen app (simulated node or TWAI peripheral) is built on espp's twai +# component, which needs the IDF >= 6.0 esp_driver_twai node API (the simulated +# bus is a Twai drop-in and shares its Message type). The rest of the example +# builds on IDF 5.5, so the canopen / twai components are only pulled in when +# the IDF supports them -- or when overridden on the command line: +# idf.py -DDESKTOP_EXAMPLE_CANOPEN=OFF build (leave the CANopen app out) +# With it OFF, CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN has no effect (the app is +# not compiled; app_main logs a warning if it is enabled). +if(IDF_VERSION_MAJOR GREATER_EQUAL 6) + set(DESKTOP_EXAMPLE_CANOPEN_DEFAULT ON) +else() + set(DESKTOP_EXAMPLE_CANOPEN_DEFAULT OFF) +endif() +option(DESKTOP_EXAMPLE_CANOPEN "Build the CANopen / DS402 app (needs IDF >= 6.0 for twai)" + ${DESKTOP_EXAMPLE_CANOPEN_DEFAULT}) +# main/CMakeLists.txt needs the decision for its REQUIRES, which IDF evaluates +# in a separate script process (no CMake cache there): hand it over as a build +# property, which IDF writes into that process. +idf_build_set_property(DESKTOP_EXAMPLE_CANOPEN "${DESKTOP_EXAMPLE_CANOPEN}") +if(DESKTOP_EXAMPLE_CANOPEN) + set(DESKTOP_EXAMPLE_CANOPEN_DIRS "../../../components/canopen" "../../../components/twai") + set(DESKTOP_EXAMPLE_CANOPEN_COMPONENTS "canopen twai") + message(STATUS "desktop example: CANopen / DS402 app ON (IDF ${IDF_VERSION_MAJOR}.${IDF_VERSION_MINOR})") +else() + set(DESKTOP_EXAMPLE_CANOPEN_DIRS "") + set(DESKTOP_EXAMPLE_CANOPEN_COMPONENTS "") + message(STATUS "desktop example: CANopen / DS402 app OFF (DESKTOP_EXAMPLE_CANOPEN; twai needs IDF >= 6.0)") +endif() set(EXTRA_COMPONENT_DIRS "../../../components/base_component" - "../../../components/canopen" + ${DESKTOP_EXAMPLE_CANOPEN_DIRS} "../../../components/cli" "../../../components/coredump" "../../../components/desktop" @@ -37,7 +67,6 @@ set(EXTRA_COMPONENT_DIRS "../../../components/stream_frame" "../../../components/system" "../../../components/task" - "../../../components/twai" "../../../components/usb_device" "../../../components/wifi" ) @@ -52,9 +81,10 @@ endif() set( COMPONENTS - "main esptool_py base_component desktop dispatcher file_system format logger monitor nvs stream_frame system task usb_device esp_tinyusb esp_psram coredump ota espcoredump littlefs canopen twai i2c wifi ethernet cli" + "main esptool_py base_component desktop dispatcher file_system format logger monitor nvs stream_frame system task usb_device esp_tinyusb esp_psram coredump ota espcoredump littlefs i2c wifi ethernet cli ${DESKTOP_EXAMPLE_CANOPEN_COMPONENTS}" CACHE STRING "List of components to include" + FORCE ) project(desktop_example) diff --git a/components/desktop/example/README.md b/components/desktop/example/README.md index 8695de0cc1..2b17e1d87d 100644 --- a/components/desktop/example/README.md +++ b/components/desktop/example/README.md @@ -92,12 +92,22 @@ edit `rmii_config()` in `main/apps/network_app.hpp`). | `DESKTOP_EXAMPLE_ENABLE_ETHERNET` | n | The Network app's RMII Ethernet group (`SOC_EMAC_SUPPORTED`: ESP32 / -P4) | | `DESKTOP_EXAMPLE_ETHERNET_BOARD` | per target | RMII wiring: `ETHERNET_KIT` (ESP32-Ethernet-Kit) or `P4_FUNCTION_EV` (ESP32-P4-Function-EV-Board, with its routable data pins) | -The hardware components (`canopen`, `twai`, `i2c`, `wifi`, `ethernet`, `cli`) -are always part of the build (`REQUIRES` cannot depend on Kconfig); the -options only decide which apps are registered. The simulated CAN bus / -DS402 node headers are included from the CAN bridge example -(`components/canopen/can_bridge_example/main`); promoting them into the -`canopen` component is a follow-up. +The hardware components (`i2c`, `wifi`, `ethernet`, `cli`) are always part +of the build (`REQUIRES` cannot depend on Kconfig); the options only decide +which apps are registered. + +**Minimum IDF.** The example itself builds on IDF 5.5 (the `desktop`, +`usb_device`, `ethernet` (>= 5.4), `wifi`, `i2c` and `canopen` components +all support it). The CANopen app is the exception: it is built on the `twai` +component (the simulated bus is a `Twai` drop-in), which needs the IDF >= 6.0 +`esp_driver_twai` node API. The CMakeLists therefore adds `canopen` + `twai` +and compiles the app only on IDF >= 6.0, controlled by the CMake option +`DESKTOP_EXAMPLE_CANOPEN` (default ON on IDF >= 6, OFF below; override with +`idf.py -DDESKTOP_EXAMPLE_CANOPEN=OFF build`). On an older IDF +`CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN` has no effect and `app_main` logs a +warning. The simulated CAN bus / DS402 node headers are included from the CAN +bridge example (`components/canopen/can_bridge_example/main`); promoting them +into the `canopen` component is a follow-up. ### Build and Flash diff --git a/components/desktop/example/main/CMakeLists.txt b/components/desktop/example/main/CMakeLists.txt index cfaed2613b..9a7fe2d78c 100644 --- a/components/desktop/example/main/CMakeLists.txt +++ b/components/desktop/example/main/CMakeLists.txt @@ -1,11 +1,28 @@ -idf_component_register( - SRC_DIRS "." +# The hardware components are required unconditionally (REQUIRES cannot depend +# on Kconfig); the apps themselves are gated by CONFIG_DESKTOP_EXAMPLE_ENABLE_* +# in desktop_example.cpp. The exception is the CANopen app: espp's twai +# component needs IDF >= 6.0, so canopen / twai are only required when the +# top-level CMakeLists enabled DESKTOP_EXAMPLE_CANOPEN (ON by default on IDF +# >= 6; read back as a build property because IDF evaluates REQUIRES in a +# separate script process), and the app is compiled only when +# DESKTOP_EXAMPLE_CANOPEN_AVAILABLE is defined. +idf_build_get_property(desktop_example_canopen DESKTOP_EXAMPLE_CANOPEN) +set(desktop_example_requires desktop system monitor file_system nvs dispatcher task usb_device + esp_tinyusb coredump ota espcoredump i2c wifi ethernet) +set(desktop_example_include_dirs "." "apps") +if(desktop_example_canopen) + list(APPEND desktop_example_requires canopen twai) # The CANopen app's simulated bus / DS402 node live in the can_bridge example # for now (simulated_can_bus.hpp, simulated_ds402_node.hpp); promoting them # into the canopen component is a follow-up. - INCLUDE_DIRS "." "apps" "../../../canopen/can_bridge_example/main" - # Every hardware component is required unconditionally (REQUIRES cannot - # depend on Kconfig); the apps themselves are gated by - # CONFIG_DESKTOP_EXAMPLE_ENABLE_* in desktop_example.cpp. - REQUIRES desktop system monitor file_system nvs dispatcher task usb_device esp_tinyusb coredump ota espcoredump - canopen twai i2c wifi ethernet) + list(APPEND desktop_example_include_dirs "../../../canopen/can_bridge_example/main") +endif() + +idf_component_register( + SRC_DIRS "." + INCLUDE_DIRS ${desktop_example_include_dirs} + REQUIRES ${desktop_example_requires}) + +if(desktop_example_canopen) + target_compile_definitions(${COMPONENT_LIB} PRIVATE DESKTOP_EXAMPLE_CANOPEN_AVAILABLE=1) +endif() diff --git a/components/desktop/example/main/Kconfig.projbuild b/components/desktop/example/main/Kconfig.projbuild index 9210c2cd21..5c677c031f 100644 --- a/components/desktop/example/main/Kconfig.projbuild +++ b/components/desktop/example/main/Kconfig.projbuild @@ -26,6 +26,12 @@ menu "Desktop Example Configuration" (espp::CanopenClient) and a CiA 402 drive panel (espp::Ds402Drive) on the bus selected below. + The app is built on espp's twai component (the simulated bus is a + Twai drop-in), which needs IDF >= 6.0; the example's CMakeLists + therefore only compiles it on IDF >= 6 (or with + -DDESKTOP_EXAMPLE_CANOPEN=ON/OFF on the idf.py command line). On an + older IDF this option has no effect and app_main logs a warning. + choice DESKTOP_EXAMPLE_CANOPEN_BUS prompt "CAN bus for the CANopen app" depends on DESKTOP_EXAMPLE_ENABLE_CANOPEN diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 722b0841ae..be0b17cfaf 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -334,6 +334,12 @@ inline void register_network_app(espp::Desktop &desktop) { auto win = d.create_window({.title = "Network", .app = app, .w = 520, .h = 0}); std::vector> refreshers; + // Ethernet FIRST: espp::Ethernet::initialize() treats + // esp_netif_init()'s ESP_ERR_INVALID_STATE (already initialized by + // the Wi-Fi stack) as fatal, while Wifi::init() tolerates it. +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET + net->ensure_ethernet(); +#endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI net->ensure_wifi(); auto wifi = win.group("Wi-Fi station"); @@ -551,7 +557,7 @@ inline void register_network_app(espp::Desktop &desktop) { #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET - net->ensure_ethernet(); + // (the link was brought up above, before the Wi-Fi stack) auto eth = win.group("Ethernet (RMII)"); auto link = win.label("Link: -", eth.id(), D::kLabelBold); auto eth_ip = win.label("IP: -", eth.id(), D::kLabelMonospace); diff --git a/components/desktop/example/main/desktop_example.cpp b/components/desktop/example/main/desktop_example.cpp index e2bf755ab0..c396366fa9 100644 --- a/components/desktop/example/main/desktop_example.cpp +++ b/components/desktop/example/main/desktop_example.cpp @@ -29,9 +29,14 @@ #include "settings_app.hpp" #include "system_monitor_app.hpp" #include "task_manager_app.hpp" -// hardware apps, each behind its Kconfig option (Desktop Example Configuration) -#if CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN +// hardware apps, each behind its Kconfig option (Desktop Example Configuration). +// The CANopen app additionally needs the canopen / twai components, which the +// CMakeLists only add on IDF >= 6.0 (DESKTOP_EXAMPLE_CANOPEN_AVAILABLE). +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN && defined(DESKTOP_EXAMPLE_CANOPEN_AVAILABLE) +#define DESKTOP_EXAMPLE_CANOPEN_APP 1 #include "canopen_app.hpp" +#else +#define DESKTOP_EXAMPLE_CANOPEN_APP 0 #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_I2C #include "i2c_scanner_app.hpp" @@ -116,8 +121,11 @@ extern "C" void app_main(void) { register_task_manager_app(desktop); register_log_viewer_app(desktop); register_files_app(desktop); -#if CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN +#if DESKTOP_EXAMPLE_CANOPEN_APP register_canopen_app(desktop); +#elif CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN + logger.warn("CONFIG_DESKTOP_EXAMPLE_ENABLE_CANOPEN is set but the CANopen app was not built: " + "it needs IDF >= 6.0 (twai) and the CMake option DESKTOP_EXAMPLE_CANOPEN=ON"); #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_I2C register_i2c_scanner_app(desktop); From dafa84feccb64775675e51ae9eabb969605443c1 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 05:08:16 -0500 Subject: [PATCH 13/21] fix(desktop): address the twelfth Copilot review of the hardware apps - CANopen: a failed statusword read clears the dependent statusword / mode / position / velocity fields (clear_status) next to the no-response message instead of leaving the previous values up. - I2C: every bus transaction (scan, register read, register write) runs on the one-shot worker task with the three buttons disabled until it is done, never on the desktop task; writes are bounded to 64 data bytes like reads. The bodies capture raw pointers so the worker stored in the session does not keep the session alive. - Network: a failed post-scan reconnect no longer leaves "reconnecting" up: the association flag is cleared, the status says "reconnect failed: use Connect" and an error toast is shown. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/canopen_app.hpp | 2 + .../example/main/apps/i2c_scanner_app.hpp | 234 ++++++++++-------- .../desktop/example/main/apps/network_app.hpp | 12 +- 3 files changed, 145 insertions(+), 103 deletions(-) diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index 2eeeaf38c6..6933d4829b 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -266,6 +266,8 @@ struct CanopenSession { std::error_code ec; const uint16_t sw = drive->get_statusword(ec); if (ec) { + // the dependent fields are stale too: clear them next to the message + clear_status("no response"); state_label.set_text("State: no response from node {} ({})", node_id, ec.message()); return; } diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp index b0973179bb..a7d13890cf 100644 --- a/components/desktop/example/main/apps/i2c_scanner_app.hpp +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -5,6 +5,7 @@ // window. A bus that fails to initialize shows a hint instead of the tools. #include +#include #include #include #include @@ -23,7 +24,8 @@ struct I2cSession { std::unique_ptr bus; std::mutex mutex; std::vector found; // addresses in table order (scan task writes, handlers read) - std::unique_ptr scan; // one-shot; joined before the bus goes away + std::unique_ptr scan; // the one-shot worker (scan / read / write); joined before + // the bus goes away /// Parse a space / comma separated list of hex bytes ("01 ff, 0x10") into /// a byte list; nullopt when any token is malformed or above 0xFF (the @@ -140,18 +142,76 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { return; } - // ---- scan ---- + // ---- widgets ---- auto bar = win.row(); auto status = win.label("Press Scan to probe addresses 0x01..0x7F.", bar.id()); auto table = win.table({"Address", "Decimal", "Note"}, {}, nullptr); auto scan_btn = win.button("Scan", nullptr, bar.id(), D::kButtonPrimary); - scan_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { + auto tools = win.group("Register access"); + auto fields = win.row(tools.id()); + auto addr_box = win.textbox("", nullptr, fields.id(), "addr (hex)"); + addr_box.set_size(80, 0); + auto reg_box = win.textbox("0", nullptr, fields.id(), "register (hex)"); + reg_box.set_size(90, 0); + auto len_box = win.textbox("1", nullptr, fields.id(), "length"); + len_box.set_size(60, 0); + auto val_box = win.textbox("", nullptr, fields.id(), "bytes to write (hex, <= 64)"); + auto actions = win.row(tools.id()); + auto result = win.label("", tools.id(), D::kLabelMonospace | D::kLabelWrap); + auto read_btn = win.button("Read", nullptr, actions.id(), D::kButtonPrimary); + auto write_btn = win.button("Write", nullptr, actions.id()); + table.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Select || e.value < 0) + return; + std::lock_guard lock(st->mutex); + if (static_cast(e.value) < st->found.size()) + addr_box.set_text("{:02X}", st->found[static_cast(e.value)]); + }); + + // ---- the worker ---- + // Every bus transaction (a scan, a register read or write) runs on + // one one-shot worker task, never on the desktop task (a scan is up + // to ~1.3 s, a 64-byte write at 100 kHz ~6 ms); the three buttons + // are disabled until it is done and the result is posted back + // through the widget mutators. The bus outlives the task: on_close + // joins it first. The bodies capture raw pointers (not `st`) so the + // task stored in the session does not keep the session alive. + auto set_busy = [=](bool busy) mutable { + scan_btn.set_enabled(!busy); + read_btn.set_enabled(!busy); + write_btn.set_enabled(!busy); + }; + auto run_worker = [=, &d](std::function body) mutable { + if (st->scan && st->scan->is_running()) + return; + st->scan.reset(); // the previous (finished) worker + set_busy(true); + st->scan = std::make_unique(espp::Task::Config{ + .callback = + [=]() mutable { + body(); + set_busy(false); + return true; // one shot + }, + .task_config = {.name = "i2c_worker", .stack_size_bytes = 4 * 1024}}); + if (!st->scan->start()) { + // no task will ever re-enable the controls: restore the window + st->scan.reset(); + set_busy(false); + table.set_enabled(true); + status.set_text("Could not start the I2C worker task."); + d.notify({.title = "I2C", + .text = "could not start the I2C worker task (out of memory?)", + .level = D::NotifyLevel::Error}); + } + }; + + // ---- scan ---- + scan_btn.on_event([=](const D::WidgetEvent &e) mutable { if (e.kind != D::WidgetEventKind::Click) return; if (st->scan && st->scan->is_running()) return; - st->scan.reset(); - scan_btn.set_enabled(false); // the old rows and their address mapping go BEFORE the worker // starts (it publishes the new mapping before the new rows) and // the table stays disabled for the whole scan, so a selection @@ -164,109 +224,81 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { table.set_selected(D::kNoSelection); table.set_enabled(false); status.set_text("Scanning\xE2\x80\xA6"); - // the bus outlives the task: on_close joins the scan first auto *bus = st->bus.get(); auto *session = st.get(); - st->scan = std::make_unique(espp::Task::Config{ - .callback = - [=]() mutable { - std::vector rows; - std::vector found; - for (uint8_t addr = 0x01; addr <= 0x7F; ++addr) { - if (!bus->probe_device(addr)) - continue; - found.push_back(addr); - rows.push_back( - fmt::format("0x{:02X}\t{}\t{}", addr, addr, S::note(addr))); - } - { - std::lock_guard lock(session->mutex); - session->found = found; - } - table.set_items(rows); - table.set_enabled(true); - status.set_text("{} device{} found", rows.size(), - rows.size() == 1 ? "" : "s"); - scan_btn.set_enabled(true); - return true; // one shot - }, - .task_config = {.name = "i2c_scan", .stack_size_bytes = 4 * 1024}}); - if (!st->scan->start()) { - // no task will ever re-enable the button: restore the window - st->scan.reset(); - scan_btn.set_enabled(true); + run_worker([=]() mutable { + std::vector rows; + std::vector found; + for (uint8_t addr = 0x01; addr <= 0x7F; ++addr) { + if (!bus->probe_device(addr)) + continue; + found.push_back(addr); + rows.push_back(fmt::format("0x{:02X}\t{}\t{}", addr, addr, S::note(addr))); + } + { + std::lock_guard lock(session->mutex); + session->found = found; + } + table.set_items(rows); table.set_enabled(true); - status.set_text("Could not start the scan task."); - d.notify({.title = "I2C", - .text = "could not start the scan task (out of memory?)", - .level = D::NotifyLevel::Error}); - } + status.set_text("{} device{} found", rows.size(), rows.size() == 1 ? "" : "s"); + }); }); - // ---- register access ---- - auto tools = win.group("Register access"); - auto fields = win.row(tools.id()); - auto addr_box = win.textbox("", nullptr, fields.id(), "addr (hex)"); - addr_box.set_size(80, 0); - auto reg_box = win.textbox("0", nullptr, fields.id(), "register (hex)"); - reg_box.set_size(90, 0); - auto len_box = win.textbox("1", nullptr, fields.id(), "length"); - len_box.set_size(60, 0); - auto val_box = win.textbox("", nullptr, fields.id(), "bytes to write (hex)"); - auto actions = win.row(tools.id()); - auto result = win.label("", tools.id(), D::kLabelMonospace | D::kLabelWrap); - table.on_event([=](const D::WidgetEvent &e) mutable { - if (e.kind != D::WidgetEventKind::Select || e.value < 0) + // ---- register access (parsed on the desktop task, done on the worker) ---- + read_btn.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click) return; - std::lock_guard lock(st->mutex); - if (static_cast(e.value) < st->found.size()) - addr_box.set_text("{:02X}", st->found[static_cast(e.value)]); + const auto addr = S::parse_addr(addr_box.text()); + const auto reg = S::parse_byte(reg_box.text()); + if (!addr || !reg) { + result.set_text(!addr ? "address must be a hex value in 0x01..0x7F" + : "register must be a hex value in 0x00..0xFF"); + return; + } + const auto len = S::parse_field(len_box.text(), 10, 1, 64); // decimal + if (!len) { + result.set_text("length must be a decimal number 1..64"); + return; + } + auto *bus = st->bus.get(); + run_worker([=, a = *addr, r = *reg, n = static_cast(*len)]() mutable { + std::vector data(n); + if (bus->read_at_register(a, r, data.data(), data.size())) + result.set_text("0x{:02X} reg 0x{:02X}: {}", a, r, S::hex_dump(data)); + else + result.set_text("0x{:02X} reg 0x{:02X}: read failed (no ACK?)", a, r); + }); + }); + write_btn.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click) + return; + const auto addr = S::parse_addr(addr_box.text()); + const auto reg = S::parse_byte(reg_box.text()); + if (!addr || !reg) { + result.set_text(!addr ? "address must be a hex value in 0x01..0x7F" + : "register must be a hex value in 0x00..0xFF"); + return; + } + const auto bytes = S::parse_bytes(val_box.text()); + if (!bytes) { + result.set_text("bytes must be hex values 00..FF, space / comma separated"); + return; + } + if (bytes->size() > 64) { // the same bound as a read + result.set_text("at most 64 data bytes per write ({} given)", bytes->size()); + return; + } + std::vector data{*reg}; + data.insert(data.end(), bytes->begin(), bytes->end()); + auto *bus = st->bus.get(); + run_worker([=, a = *addr]() mutable { + if (bus->write(a, data.data(), data.size())) + result.set_text("0x{:02X} <- {}ok", a, S::hex_dump(data)); + else + result.set_text("0x{:02X} <- {}failed (no ACK?)", a, S::hex_dump(data)); + }); }); - win.button( - "Read", - [=]() mutable { - const auto addr = S::parse_addr(addr_box.text()); - const auto reg = S::parse_byte(reg_box.text()); - if (!addr || !reg) { - result.set_text(!addr ? "address must be a hex value in 0x01..0x7F" - : "register must be a hex value in 0x00..0xFF"); - return; - } - const auto len = S::parse_field(len_box.text(), 10, 1, 64); // decimal - if (!len) { - result.set_text("length must be a decimal number 1..64"); - return; - } - std::vector data(static_cast(*len)); - if (st->bus->read_at_register(*addr, *reg, data.data(), data.size())) - result.set_text("0x{:02X} reg 0x{:02X}: {}", *addr, *reg, S::hex_dump(data)); - else - result.set_text("0x{:02X} reg 0x{:02X}: read failed (no ACK?)", *addr, *reg); - }, - actions.id(), D::kButtonPrimary); - win.button( - "Write", - [=]() mutable { - const auto addr = S::parse_addr(addr_box.text()); - const auto reg = S::parse_byte(reg_box.text()); - if (!addr || !reg) { - result.set_text(!addr ? "address must be a hex value in 0x01..0x7F" - : "register must be a hex value in 0x00..0xFF"); - return; - } - const auto bytes = S::parse_bytes(val_box.text()); - if (!bytes) { - result.set_text("bytes must be hex values 00..FF, space / comma separated"); - return; - } - std::vector data{*reg}; - data.insert(data.end(), bytes->begin(), bytes->end()); - if (st->bus->write(*addr, data.data(), data.size())) - result.set_text("0x{:02X} <- {}ok", *addr, S::hex_dump(data)); - else - result.set_text("0x{:02X} <- {}failed (no ACK?)", *addr, S::hex_dump(data)); - }, - actions.id()); }, }); } diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index be0b17cfaf..e28fca6e3c 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -427,7 +427,7 @@ inline void register_network_app(espp::Desktop &desktop) { auto *state = net.get(); // outlives the task (joined by its destructor) net->scan_task = std::make_unique(espp::Task::Config{ .callback = - [=]() mutable { + [=, &d]() mutable { // d (the Desktop) outlives every task // WifiSta::scan() drops the link with a bare // esp_wifi_disconnect(), which WifiSta's DISCONNECTED // handler treats as unintentional and answers with a @@ -446,8 +446,16 @@ inline void register_network_app(espp::Desktop &desktop) { const auto aps = state->wifi->scan(20); if (intent) { state->set_status("reconnecting"); - if (state->wifi->connect()) + if (state->wifi->connect()) { state->associating = true; + } else { + // never leave "reconnecting" up: say what to do + state->associating = false; + state->set_status("reconnect failed: use Connect"); + d.notify({.title = "Wi-Fi", + .text = "could not reconnect after the scan; use Connect", + .level = D::NotifyLevel::Error}); + } } else { state->set_status("disconnected"); } From e4686bce5a120b7f5b6541bbe3d233c91fb79fb0 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 05:23:27 -0500 Subject: [PATCH 14/21] fix(desktop): address the thirteenth Copilot review of the hardware apps - Example CMake: the COMPONENTS cache entry is no longer FORCEd; like the other examples it is only set when the caller did not pass -DCOMPONENTS=... (the CANopen decision still travels as a build property, so main's REQUIRES resolve canopen / twai from EXTRA_COMPONENT_DIRS whatever the list says). - Network: connection-attempt generations. Every attempt (start-up auto-connect, Connect, the reconnect after a scan, Forget's reset) bumps a counter and installs callbacks that captured it; events of an older generation are ignored, so WifiSta::disconnect()'s late DISCONNECTED (its connected_ clears at once, the event lands later) cannot clear the new attempt's state. Rule: Connect is refused with a toast while an attempt is still associating (reconfigure() would not stop it -- it only disconnects a station whose connected_ is set); a connected station is stopped explicitly under the new generation before the new config. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- components/desktop/example/CMakeLists.txt | 21 +++-- .../desktop/example/main/apps/network_app.hpp | 85 +++++++++++++++---- 2 files changed, 82 insertions(+), 24 deletions(-) diff --git a/components/desktop/example/CMakeLists.txt b/components/desktop/example/CMakeLists.txt index b543a4f73a..b2c15fafd2 100644 --- a/components/desktop/example/CMakeLists.txt +++ b/components/desktop/example/CMakeLists.txt @@ -79,12 +79,19 @@ if(DEFINED ENV{IDF_COMPONENT_MANAGER} AND "$ENV{IDF_COMPONENT_MANAGER}" STREQUAL ) endif() -set( - COMPONENTS - "main esptool_py base_component desktop dispatcher file_system format logger monitor nvs stream_frame system task usb_device esp_tinyusb esp_psram coredump ota espcoredump littlefs i2c wifi ethernet cli ${DESKTOP_EXAMPLE_CANOPEN_COMPONENTS}" - CACHE STRING - "List of components to include" - FORCE - ) +# The component list is a cache entry like in the other examples, so a caller's +# -DCOMPONENTS=... wins; the CANopen-dependent default is only computed when +# the caller did not supply one. (main's REQUIRES still resolve canopen / twai +# from EXTRA_COMPONENT_DIRS whenever DESKTOP_EXAMPLE_CANOPEN is ON, whatever +# the list says: that decision travels as a build property, not through +# COMPONENTS.) +if(NOT DEFINED COMPONENTS) + set( + COMPONENTS + "main esptool_py base_component desktop dispatcher file_system format logger monitor nvs stream_frame system task usb_device esp_tinyusb esp_psram coredump ota espcoredump littlefs i2c wifi ethernet cli ${DESKTOP_EXAMPLE_CANOPEN_COMPONENTS}" + CACHE STRING + "List of components to include" + ) +endif() project(desktop_example) diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index e28fca6e3c..cc56acf83c 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -64,6 +64,21 @@ struct NetworkState { /// station it sets its private `disconnecting_` and no DISCONNECTED event /// ever clears it, so the next Connect would lose its retries. std::atomic associating{false}; + /// Connection-attempt GENERATION. The driver events are asynchronous: + /// WifiSta::disconnect() clears its `connected_` at once but the + /// DISCONNECTED event lands later, and reconfigure() only disconnects a + /// station whose `connected_` is set (not one still associating). So every + /// attempt (start-up auto-connect, Connect, the reconnect after a scan, + /// Forget's reset) bumps this counter and installs callbacks that captured + /// the new value; an event whose captured generation is older than the + /// current one belongs to a previous attempt and is ignored. The rule for + /// a new Connect: refused while an attempt of the current generation is + /// still associating (Disconnect first); a connected station is stopped + /// explicitly (disconnect_if_active) under the new generation before the + /// new configuration is applied, so its late DISCONNECTED cannot clear the + /// new attempt's state. + std::atomic attempt{0}; + std::string cur_ssid, cur_pass; // the credentials of the current attempt (under `mutex`) #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI std::unique_ptr wifi; std::unique_ptr scan_task; // after wifi: joined before it goes away @@ -100,29 +115,47 @@ struct NetworkState { } } - /// The station config for `ssid` / `password`: the callbacks record the - /// state this struct holds (they run on the event-loop task). - espp::WifiSta::Config wifi_config(std::string ssid, std::string password) { + /// The station config for `ssid` / `password` of attempt `gen`: the + /// callbacks record the state this struct holds (they run on the event-loop + /// task) and ignore events of an older generation (see `attempt`). + espp::WifiSta::Config wifi_config(std::string ssid, std::string password, uint32_t gen) { + { + std::lock_guard lock(mutex); + cur_ssid = ssid; + cur_pass = password; + } return {.ssid = std::move(ssid), .password = std::move(password), .num_connect_retries = 3, .auto_connect = true, - .on_connected = [this]() { set_status("connected, waiting for an IP"); }, + .on_connected = + [this, gen]() { + if (gen == attempt) + set_status("connected, waiting for an IP"); + }, .on_disconnected = - [this]() { // retries exhausted, or reconfigure()'s own disconnect + [this, gen]() { // retries exhausted, or reconfigure()'s own disconnect + if (gen != attempt) + return; // a previous attempt's late event associating = false; set_status(want_connected ? "disconnected (retries exhausted; Connect or a " "scan retries)" : "disconnected"); }, .on_got_ip = - [this](ip_event_got_ip_t *e) { + [this, gen](ip_event_got_ip_t *e) { + if (gen != attempt) + return; associating = false; set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); }, .log_level = espp::Logger::Verbosity::INFO}; } + /// Start a new attempt generation: later events of the previous one are + /// ignored from here on. + uint32_t new_attempt() { return ++attempt; } + /// Whether a string key exists in the app's namespace: ESP_OK (it does), /// ESP_ERR_NVS_NOT_FOUND (it does not) or another error (a genuine NVS /// failure). NvsHandle maps every string read failure to one error code, @@ -221,7 +254,7 @@ struct NetworkState { auto &stack = espp::Wifi::get(); if (stack.init()) stack.set_storage(WIFI_STORAGE_RAM); - auto cfg = wifi_config(ssid, pass); + auto cfg = wifi_config(ssid, pass, new_attempt()); cfg.auto_connect = !ssid.empty(); want_connected = cfg.auto_connect; associating = cfg.auto_connect; // WifiSta connects from its STA_START event @@ -254,8 +287,9 @@ struct NetworkState { stop_station(); wifi_config_t empty{}; const bool cleared = esp_wifi_set_config(WIFI_IF_STA, &empty) == ESP_OK; - auto cfg = - wifi_config("", ""); // empty ssid: reconfigure() adopts the (now empty) driver config + // empty ssid: reconfigure() adopts the (now empty) driver config; a new + // generation so the stopped link's late events are ignored + auto cfg = wifi_config("", "", new_attempt()); cfg.auto_connect = false; const bool station_reset = wifi->reconfigure(cfg) && cleared; // the persistent status must not claim the credentials are gone when the @@ -446,7 +480,17 @@ inline void register_network_app(espp::Desktop &desktop) { const auto aps = state->wifi->scan(20); if (intent) { state->set_status("reconnecting"); - if (state->wifi->connect()) { + // a new attempt generation with fresh callbacks, so the + // scan's own disconnect events cannot clear its state + std::string ssid, pass; + { + std::lock_guard lock(state->mutex); + ssid = state->cur_ssid; + pass = state->cur_pass; + } + const uint32_t gen = state->new_attempt(); + if (state->wifi->reconfigure(state->wifi_config(ssid, pass, gen)) && + state->wifi->connect()) { state->associating = true; } else { // never leave "reconnecting" up: say what to do @@ -524,14 +568,21 @@ inline void register_network_app(espp::Desktop &desktop) { .text = "could not save the credentials to NVS (connecting anyway)", .level = D::NotifyLevel::Warn}); net->set_status(fmt::format("connecting to {}", ssid)); - const bool was_connected = net->wifi->is_connected(); + // The rule (see NetworkState::attempt): an attempt still + // associating is not replaced -- Disconnect first; a connected + // station is stopped explicitly under the new generation, so + // its late DISCONNECTED is ignored, before the new config. + if (net->associating) { + d.notify({.title = "Wi-Fi", + .text = "a connection attempt is in progress; Disconnect first", + .level = D::NotifyLevel::Warn}); + return; + } + const uint32_t gen = net->new_attempt(); + net->disconnect_if_active(); net->want_connected = true; - // reconfigure() reconnects when it was connected; else connect(). - // `associating` is set only once the association was issued: - // reconfigure()'s own disconnect fires on_disconnected first, - // which would clear it again. - bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass)); - if (ok && !was_connected) + bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass, gen)); + if (ok) ok = net->wifi->connect(); if (ok) net->associating = true; From 0dc44aecdf3e5219e1295c0da01afe6c3081d5d6 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 05:45:44 -0500 Subject: [PATCH 15/21] fix(desktop): run every Wi-Fi station transition on a worker that waits for DISCONNECTED Copilot round 14. WifiSta fetches config_.on_disconnected at delivery time, so after reconfigure() a late DISCONNECTED of the old attempt invokes the NEW callback with the current generation: the generation check cannot identify late events. Ownership is no longer inferred from the installed callback: every station transition (scan, connect, disconnect, forget) now runs on one worker task as an explicit state machine (Idle -> Stopping -> Configuring -> Associating -> Connected) with the controls disabled, and Stopping actually WAITS (condition variable, 1 s bound) for the stopped station's DISCONNECTED event before the next attempt is configured. The generation stays as a belt-and-braces filter for an event that still arrives after the wait timed out. Each window keeps its own SSID vector, snapshotted together with its AP list rows (at open and whenever a completed scan replaces them, which also drops the selection), and connects from that: a scan completing in another window can no longer remap a selected row to a different SSID. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/network_app.hpp | 489 +++++++++--------- 1 file changed, 258 insertions(+), 231 deletions(-) diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index cc56acf83c..8fa2860662 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -5,12 +5,16 @@ // gate, the RMII Ethernet link (espp::Ethernet). The interfaces are brought // up on the first launch and kept across windows (closing the window does not // disconnect); the window's 1 s timer renders the state the driver callbacks -// record. A Wi-Fi scan blocks (and disconnects first), so it runs on its own -// task. +// record. Every station transition (scan, connect, disconnect, forget) runs +// on one worker task as a small explicit state machine (Idle -> Stopping -> +// Configuring -> Associating -> Connected), never on the desktop task, with +// the controls disabled until it is done. #include #include +#include #include +#include #include #include #include @@ -40,6 +44,12 @@ namespace desktop_example { /// The interfaces, shared by every Network window (created on first launch). struct NetworkState { + using D = espp::Desktop; + + explicit NetworkState(D &desktop) + : desktop(desktop) {} + + D &desktop; ///< outlives every task (toasts from the worker) espp::Logger logger{{.tag = "Network app", .level = espp::Logger::Verbosity::INFO}}; std::mutex mutex; std::string wifi_status{"idle"}; @@ -48,40 +58,35 @@ struct NetworkState { std::vector scan_ssids; // in AP list order std::vector scan_rows; // the AP list's items (shown again by a new window) uint32_t scan_generation{0}; // bumped per completed scan (windows re-sync on change) - /// A scan is in flight on the scan task: it disconnects and drives the - /// station, so every other station action is refused until it is done. - std::atomic scanning{false}; + + /// The station state machine, driven by the jobs below on the worker task + /// and by the driver callbacks (got-ip -> Connected, retries exhausted -> + /// Idle). `Stopping` is the explicit wait for the DISCONNECTED event. + enum class Phase : uint8_t { Idle, Stopping, Configuring, Associating, Connected }; + std::atomic phase{Phase::Idle}; + /// A job (scan / connect / disconnect / forget) is running on the worker: + /// every station control is disabled, and another job is refused. + std::atomic busy{false}; /// DESIRED connectivity: set by Connect and by the saved-credentials /// auto-connect at start, cleared only by Disconnect / Forget -- never by - /// the driver callbacks (on_disconnected also fires for the intentional - /// disconnect inside WifiSta::reconfigure(), e.g. when switching APs). A - /// scan suppresses the station while it wants to be connected and restores - /// it afterwards, whatever the transient association state. + /// the driver callbacks. A scan stops the station and restores it afterwards + /// when this is set, whatever the transient phase. std::atomic want_connected{false}; - /// An association is in progress (connect() was issued and neither an IP - /// nor "retries exhausted" arrived yet): the only state, besides being - /// connected, in which WifiSta::disconnect() is safe to call -- on an idle - /// station it sets its private `disconnecting_` and no DISCONNECTED event - /// ever clears it, so the next Connect would lose its retries. - std::atomic associating{false}; - /// Connection-attempt GENERATION. The driver events are asynchronous: - /// WifiSta::disconnect() clears its `connected_` at once but the - /// DISCONNECTED event lands later, and reconfigure() only disconnects a - /// station whose `connected_` is set (not one still associating). So every - /// attempt (start-up auto-connect, Connect, the reconnect after a scan, - /// Forget's reset) bumps this counter and installs callbacks that captured - /// the new value; an event whose captured generation is older than the - /// current one belongs to a previous attempt and is ignored. The rule for - /// a new Connect: refused while an attempt of the current generation is - /// still associating (Disconnect first); a connected station is stopped - /// explicitly (disconnect_if_active) under the new generation before the - /// new configuration is applied, so its late DISCONNECTED cannot clear the - /// new attempt's state. + /// Connection-attempt GENERATION, a belt-and-braces check only: the driver + /// events are asynchronous and WifiSta fetches `config_.on_disconnected` + /// at DELIVERY time, so the installed callback cannot tell which attempt an + /// event belongs to. Ownership is therefore never inferred from the + /// callback: before a new attempt the worker actually WAITS for the + /// DISCONNECTED event of the stopped station (stop_and_wait, bounded). The + /// generation only filters the residue (an event that still arrives after + /// the wait timed out). std::atomic attempt{0}; - std::string cur_ssid, cur_pass; // the credentials of the current attempt (under `mutex`) + std::condition_variable disconnected_cv; // with `mutex`: a DISCONNECTED event arrived + uint32_t disconnected_events{0}; // under `mutex`; any generation + std::string cur_ssid, cur_pass; // the credentials of the current attempt (under `mutex`) #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI std::unique_ptr wifi; - std::unique_ptr scan_task; // after wifi: joined before it goes away + std::unique_ptr worker; // after wifi: joined before it goes away #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET std::unique_ptr eth; @@ -93,6 +98,10 @@ struct NetworkState { wifi_ip = std::string(ip); } + void toast(std::string text, D::NotifyLevel level) { + desktop.notify({.title = "Wi-Fi", .text = std::move(text), .level = level}); + } + #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI static const char *auth_name(wifi_auth_mode_t mode) { switch (mode) { @@ -116,8 +125,8 @@ struct NetworkState { } /// The station config for `ssid` / `password` of attempt `gen`: the - /// callbacks record the state this struct holds (they run on the event-loop - /// task) and ignore events of an older generation (see `attempt`). + /// callbacks drive the phase (they run on the event-loop task) and ignore + /// the residue of an older generation (see `attempt`). espp::WifiSta::Config wifi_config(std::string ssid, std::string password, uint32_t gen) { { std::lock_guard lock(mutex); @@ -134,10 +143,19 @@ struct NetworkState { set_status("connected, waiting for an IP"); }, .on_disconnected = - [this, gen]() { // retries exhausted, or reconfigure()'s own disconnect + [this, gen]() { + // every DISCONNECTED (intentional or retries exhausted) wakes + // a stop_and_wait() in progress, whatever its generation + { + std::lock_guard lock(mutex); + ++disconnected_events; + } + disconnected_cv.notify_all(); if (gen != attempt) - return; // a previous attempt's late event - associating = false; + return; // residue of a previous attempt + if (phase == Phase::Stopping) + return; // the worker owns this transition + phase = Phase::Idle; // retries exhausted set_status(want_connected ? "disconnected (retries exhausted; Connect or a " "scan retries)" : "disconnected"); @@ -146,13 +164,13 @@ struct NetworkState { [this, gen](ip_event_got_ip_t *e) { if (gen != attempt) return; - associating = false; + phase = Phase::Connected; set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); }, .log_level = espp::Logger::Verbosity::INFO}; } - /// Start a new attempt generation: later events of the previous one are + /// Start a new attempt generation: the residue of the previous one is /// ignored from here on. uint32_t new_attempt() { return ++attempt; } @@ -191,25 +209,6 @@ struct NetworkState { return s; } - /// Give up the desired connectivity and disconnect -- but only when the - /// station is connected or a Connect was issued (an association may be in - /// flight). WifiSta::disconnect() sets its private `disconnecting_`, which - /// only the DISCONNECTED event clears: on an idle station no event comes, - /// the flag goes stale and the next Connect loses its retries on the - /// first failure. - void stop_station() { - want_connected = false; - disconnect_if_active(); - } - - /// WifiSta::disconnect() only when it will produce a DISCONNECTED event - /// (connected, or an association in progress); see `associating`. - void disconnect_if_active() { - const bool was_associating = associating.exchange(false); - if (wifi->is_connected() || was_associating) - wifi->disconnect(); - } - /// Erase one key; true when it is gone (it was erased, or was never /// stored), false on a genuine NVS failure (probe or erase). static bool erase_key(espp::NvsHandle &nvs, const char *key) { @@ -222,7 +221,22 @@ struct NetworkState { return nvs.erase(key, ec); } + /// Save the credentials (committed, so they survive a reboot). + bool save_credentials(const std::string &ssid, const std::string &pass) { + std::error_code ec; + espp::NvsHandle nvs("desktop", ec); + if (ec) + return false; + nvs.set("wifi_ssid", ssid, ec); + if (!ec) + nvs.set("wifi_pass", pass, ec); + if (!ec) + nvs.commit(ec); + return !ec; + } + /// Bring the station up once, with the saved credentials (none = idle). + /// Desktop task, first launch. void ensure_wifi() { if (wifi) return; @@ -257,22 +271,150 @@ struct NetworkState { auto cfg = wifi_config(ssid, pass, new_attempt()); cfg.auto_connect = !ssid.empty(); want_connected = cfg.auto_connect; - associating = cfg.auto_connect; // WifiSta connects from its STA_START event + // WifiSta connects from its STA_START event: that is an association + phase = cfg.auto_connect ? Phase::Associating : Phase::Idle; set_status(ssid.empty() ? "idle (no saved network)" : "connecting"); wifi = std::make_unique(cfg); wifi_mac = wifi->get_mac(); } - /// What forget() managed to do (each part is reported separately). - struct ForgetResult { - bool erased{false}; ///< the NVS keys are gone (erased + committed, or never stored) - bool station_reset{false}; ///< the driver + WifiSta configs are empty - }; + // ---- the worker: one job at a time, every station control disabled ---- + + /// Run `job` on the worker task; refused (with a toast) while another job + /// runs or when the task cannot be started. Desktop task. + bool run_job(const char *name, std::function job) { + if (busy.exchange(true)) { + toast("the station is busy; try again when it is done", D::NotifyLevel::Warn); + return false; + } + worker.reset(); // the previous (finished) job + worker = std::make_unique( + espp::Task::Config{.callback = + [this, job = std::move(job)]() { + job(); + busy = false; + return true; // one shot + }, + .task_config = {.name = name, .stack_size_bytes = 6 * 1024}}); + if (!worker->start()) { + worker.reset(); + busy = false; + toast("could not start the Wi-Fi worker task (out of memory?)", D::NotifyLevel::Error); + return false; + } + return true; + } + + /// Stopping: if the station is connected or associating, disconnect + /// intentionally (that suppresses WifiSta's retries) and WAIT for its + /// DISCONNECTED event (bounded), so a later attempt can never be confused + /// with the stopped one. On an idle station WifiSta::disconnect() is not + /// called at all: no event would come and its private `disconnecting_` + /// would stay set, costing the next attempt its retries. Worker task. + /// Returns true once the station is idle. + bool stop_and_wait(std::chrono::milliseconds timeout = std::chrono::milliseconds(1000)) { + const Phase p = phase.load(); + if (p != Phase::Connected && p != Phase::Associating) + return true; + phase = Phase::Stopping; + set_status("stopping"); + uint32_t seen = 0; + { + std::lock_guard lock(mutex); + seen = disconnected_events; + } + if (!wifi->disconnect()) { + // the driver rejected it (nothing was associated): no event will come + phase = Phase::Idle; + return true; + } + bool got = false; + { + std::unique_lock lock(mutex); + got = disconnected_cv.wait_for(lock, timeout, [&] { return disconnected_events != seen; }); + } + if (!got) + logger.warn("no DISCONNECTED event within {} ms; continuing (the generation check drops " + "it if it still arrives)", + timeout.count()); + phase = Phase::Idle; + return got; + } + + /// Configuring -> Associating: a new attempt with `ssid` / `pass` (the + /// station must be idle: call stop_and_wait() first). Worker task. + bool start_attempt(const std::string &ssid, const std::string &pass, const char *what) { + phase = Phase::Configuring; + set_status(fmt::format("configuring {}", ssid)); + const uint32_t gen = new_attempt(); + // after stop_and_wait() WifiSta's connected_ is false, so reconfigure() + // does not connect by itself: connect() explicitly + const bool ok = wifi->reconfigure(wifi_config(ssid, pass, gen)) && wifi->connect(); + if (!ok) { + phase = Phase::Idle; + set_status(fmt::format("{} failed: use Connect", what)); + toast(fmt::format("could not start the {} to {}", what, ssid), D::NotifyLevel::Error); + return false; + } + phase = Phase::Associating; + set_status(fmt::format("connecting to {}", ssid)); + return true; + } - /// Drop the saved credentials everywhere: the app's NVS keys, the driver's - /// station config and WifiSta's stored config (reconfigured with empty - /// credentials and auto-connect off, so nothing reconnects). - ForgetResult forget() { + /// Connect: stop whatever is active (waiting for its event), then attempt. + void connect_job(const std::string &ssid, const std::string &pass) { + want_connected = true; + stop_and_wait(); + start_attempt(ssid, pass, "connection"); + } + + /// Disconnect: give up the desired state and stop the station. + void disconnect_job() { + want_connected = false; + stop_and_wait(); + set_status("disconnected"); + } + + /// Scan: stop the station first (WifiSta::scan() would otherwise drop the + /// link with a bare esp_wifi_disconnect() that its DISCONNECTED handler + /// answers with a retry while the scan starts), scan, publish the rows for + /// every window, then restore the desired state with a fresh attempt. + void scan_job() { + const bool intent = want_connected; + stop_and_wait(); + set_status("scanning"); + const auto aps = wifi->scan(20); + std::vector rows, ssids; + for (const auto &ap : aps) { + const std::string ssid(reinterpret_cast(ap.ssid)); + ssids.push_back(ssid); + rows.push_back(fmt::format("{} ({} dBm, {}, ch {})", ssid.empty() ? "" : ssid, + static_cast(ap.rssi), auth_name(ap.authmode), + static_cast(ap.primary))); + } + { + std::lock_guard lock(mutex); + scan_ssids = ssids; + scan_rows = rows; + ++scan_generation; + } + if (intent) { + std::string ssid, pass; + { + std::lock_guard lock(mutex); + ssid = cur_ssid; + pass = cur_pass; + } + start_attempt(ssid, pass, "reconnect after the scan"); + } else { + set_status("disconnected"); + } + } + + /// Forget: drop the saved credentials everywhere -- the app's NVS keys, + /// the driver's station config and WifiSta's stored config (reconfigured + /// with empty credentials and auto-connect off, so nothing reconnects). + void forget_job() { std::error_code ec; espp::NvsHandle nvs("desktop", ec); bool erased = !ec; @@ -284,33 +426,25 @@ struct NetworkState { nvs.commit(ec); // erase_item() only stages the change erased = erased && !ec; } - stop_station(); + want_connected = false; + stop_and_wait(); wifi_config_t empty{}; const bool cleared = esp_wifi_set_config(WIFI_IF_STA, &empty) == ESP_OK; - // empty ssid: reconfigure() adopts the (now empty) driver config; a new - // generation so the stopped link's late events are ignored + // empty ssid: reconfigure() adopts the (now empty) driver config auto cfg = wifi_config("", "", new_attempt()); cfg.auto_connect = false; const bool station_reset = wifi->reconfigure(cfg) && cleared; + phase = Phase::Idle; // the persistent status must not claim the credentials are gone when the // NVS erase / commit failed set_status(erased ? "idle (no saved network)" : "Forget failed: the credentials may still be saved"); - return {.erased = erased, .station_reset = station_reset}; - } - - /// Save the credentials (committed, so they survive a reboot). - bool save_credentials(const std::string &ssid, const std::string &pass) { - std::error_code ec; - espp::NvsHandle nvs("desktop", ec); - if (ec) - return false; - nvs.set("wifi_ssid", ssid, ec); - if (!ec) - nvs.set("wifi_pass", pass, ec); - if (!ec) - nvs.commit(ec); - return !ec; + if (!erased) + toast("Forget failed: the credentials may still be saved in NVS", D::NotifyLevel::Error); + else if (!station_reset) + toast("credentials erased, but the station could not be reset", D::NotifyLevel::Warn); + else + toast("saved network forgotten", D::NotifyLevel::Ok); } #endif @@ -356,7 +490,7 @@ struct NetworkState { } // namespace desktop_example inline void register_network_app(espp::Desktop &desktop) { - auto net = std::make_shared(); + auto net = std::make_shared(desktop); desktop.register_app({ .name = "Network", .icon = "\xF0\x9F\x93\xA1", // satellite antenna @@ -376,6 +510,7 @@ inline void register_network_app(espp::Desktop &desktop) { #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI net->ensure_wifi(); + auto *state = net.get(); // outlives every window and task auto wifi = win.group("Wi-Fi station"); auto status = win.label("Status: -", wifi.id(), D::kLabelBold); auto ssid_label = win.label("SSID: -", wifi.id(), D::kLabelMonospace); @@ -385,15 +520,23 @@ inline void register_network_app(espp::Desktop &desktop) { auto scan_bar = win.row(wifi.id()); auto scan_btn = win.button("Scan", nullptr, scan_bar.id()); auto scan_label = win.label("", scan_bar.id()); - std::vector rows_now; + // THIS window's AP list and the SSIDs behind its rows: snapshotted + // together whenever the rows are replaced, so a selection always + // maps to the SSID of the row the user sees (another window's scan + // may replace the shared rows at any time) + auto ssids = std::make_shared>(); auto seen_generation = std::make_shared(0); + std::vector rows_now; { std::lock_guard lock(net->mutex); rows_now = net->scan_rows; // the last scan, if any + *ssids = net->scan_ssids; *seen_generation = net->scan_generation; } auto ap_list = win.list(rows_now, nullptr, wifi.id()); ap_list.set_size(0, 140); + if (!rows_now.empty()) + scan_label.set_text("{} networks", rows_now.size()); auto pass_row = win.row(wifi.id()); win.label("Password", pass_row.id()); auto pass_box = win.textbox("", nullptr, pass_row.id(), @@ -402,23 +545,15 @@ inline void register_network_app(espp::Desktop &desktop) { auto connect_btn = win.button("Connect", nullptr, actions.id(), D::kButtonPrimary); auto disconnect_btn = win.button("Disconnect", nullptr, actions.id()); auto forget_btn = win.button("Forget", nullptr, actions.id(), D::kButtonDanger); - // every station action is disabled while a scan drives the - // station from the scan task (and refused if clicked anyway) + // every station control is disabled while a job runs on the + // worker (and a click is refused by run_job() anyway) auto set_busy = [=](bool busy) mutable { scan_btn.set_enabled(!busy); connect_btn.set_enabled(!busy); disconnect_btn.set_enabled(!busy); forget_btn.set_enabled(!busy); }; - auto refuse_if_scanning = [=, &d]() { - if (!net->scanning) - return false; - d.notify({.title = "Wi-Fi", - .text = "a scan is in progress; try again when it is done", - .level = D::NotifyLevel::Warn}); - return true; - }; - set_busy(net->scanning); + set_busy(net->busy); refreshers.push_back([=]() mutable { std::string st, ip; @@ -427,20 +562,22 @@ inline void register_network_app(espp::Desktop &desktop) { std::lock_guard lock(net->mutex); st = net->wifi_status; ip = net->wifi_ip; - // a scan that completed since this window opened (the scan - // task outlives windows and only updates the one it started - // from): re-sync the list from the shared rows + // a scan completed since this window last synced (from any + // window): replace the rows AND the SSIDs behind them together, + // and drop the selection, which pointed at the old rows if (net->scan_generation != *seen_generation) { *seen_generation = net->scan_generation; new_rows = net->scan_rows; + *ssids = net->scan_ssids; } } if (new_rows) { ap_list.set_items(*new_rows); + ap_list.set_selected(D::kNoSelection); scan_label.set_text("{} networks", new_rows->size()); } - // also re-syncs a window opened while a scan started elsewhere - set_busy(net->scanning); + // also re-syncs a window opened while a job started elsewhere + set_busy(net->busy); const bool connected = net->wifi->is_connected(); status.set_text("Status: {}", st); ssid_label.set_text("SSID: {}", connected ? net->wifi->get_ssid() : "-"); @@ -451,102 +588,22 @@ inline void register_network_app(espp::Desktop &desktop) { rssi_label.set_text("RSSI: -"); }); - scan_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { - if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) + scan_btn.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click) return; - net->scanning = true; - net->scan_task.reset(); // the previous (finished) scan - set_busy(true); - scan_label.set_text("scanning\xE2\x80\xA6 (disconnects first)"); - auto *state = net.get(); // outlives the task (joined by its destructor) - net->scan_task = std::make_unique(espp::Task::Config{ - .callback = - [=, &d]() mutable { // d (the Desktop) outlives every task - // WifiSta::scan() drops the link with a bare - // esp_wifi_disconnect(), which WifiSta's DISCONNECTED - // handler treats as unintentional and answers with a - // reconnect attempt (num_connect_retries > 0) while the - // scan is starting -- whether the station was connected - // or still associating. The DESIRED state decides: - // disconnect intentionally first (that suppresses the - // retries), give the event time to land, then scan and - // restore it (connect again) afterwards. - const bool intent = state->want_connected || state->wifi->is_connected(); - if (state->wifi->is_connected() || state->associating) { - state->disconnect_if_active(); - std::this_thread::sleep_for(std::chrono::milliseconds(200)); - } - state->set_status("scanning"); - const auto aps = state->wifi->scan(20); - if (intent) { - state->set_status("reconnecting"); - // a new attempt generation with fresh callbacks, so the - // scan's own disconnect events cannot clear its state - std::string ssid, pass; - { - std::lock_guard lock(state->mutex); - ssid = state->cur_ssid; - pass = state->cur_pass; - } - const uint32_t gen = state->new_attempt(); - if (state->wifi->reconfigure(state->wifi_config(ssid, pass, gen)) && - state->wifi->connect()) { - state->associating = true; - } else { - // never leave "reconnecting" up: say what to do - state->associating = false; - state->set_status("reconnect failed: use Connect"); - d.notify({.title = "Wi-Fi", - .text = "could not reconnect after the scan; use Connect", - .level = D::NotifyLevel::Error}); - } - } else { - state->set_status("disconnected"); - } - std::vector rows, ssids; - for (const auto &ap : aps) { - const std::string ssid(reinterpret_cast(ap.ssid)); - ssids.push_back(ssid); - rows.push_back(fmt::format( - "{} ({} dBm, {}, ch {})", ssid.empty() ? "" : ssid, - static_cast(ap.rssi), state->auth_name(ap.authmode), - static_cast(ap.primary))); - } - { - std::lock_guard lock(state->mutex); - state->scan_ssids = ssids; - state->scan_rows = rows; - ++state->scan_generation; - } - ap_list.set_items(rows); - scan_label.set_text("{} networks", rows.size()); - state->scanning = false; - set_busy(false); - return true; // one shot - }, - .task_config = {.name = "wifi_scan", .stack_size_bytes = 6 * 1024}}); - if (!net->scan_task->start()) { - // no task will ever release the busy state: restore the window - net->scan_task.reset(); - net->scanning = false; - set_busy(false); - scan_label.set_text("could not start the scan task"); - d.notify({.title = "Wi-Fi", - .text = "could not start the scan task (out of memory?)", - .level = D::NotifyLevel::Error}); + if (net->run_job("wifi_scan", [state]() { state->scan_job(); })) { + set_busy(true); + scan_label.set_text("scanning\xE2\x80\xA6 (stops the station first)"); } }); connect_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { - if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) + if (e.kind != D::WidgetEventKind::Click) return; std::string ssid; - { - std::lock_guard lock(net->mutex); - const int32_t i = ap_list.selected(); - if (i >= 0 && static_cast(i) < net->scan_ssids.size()) - ssid = net->scan_ssids[static_cast(i)]; - } + const int32_t i = ap_list.selected(); + if (i >= 0 && static_cast(i) < ssids->size()) + ssid = (*ssids)[static_cast(i)]; if (ssid.empty()) { d.notify({.title = "Wi-Fi", .text = "scan, then select a network", @@ -567,51 +624,21 @@ inline void register_network_app(espp::Desktop &desktop) { d.notify({.title = "Wi-Fi", .text = "could not save the credentials to NVS (connecting anyway)", .level = D::NotifyLevel::Warn}); - net->set_status(fmt::format("connecting to {}", ssid)); - // The rule (see NetworkState::attempt): an attempt still - // associating is not replaced -- Disconnect first; a connected - // station is stopped explicitly under the new generation, so - // its late DISCONNECTED is ignored, before the new config. - if (net->associating) { - d.notify({.title = "Wi-Fi", - .text = "a connection attempt is in progress; Disconnect first", - .level = D::NotifyLevel::Warn}); - return; - } - const uint32_t gen = net->new_attempt(); - net->disconnect_if_active(); - net->want_connected = true; - bool ok = net->wifi->reconfigure(net->wifi_config(ssid, pass, gen)); - if (ok) - ok = net->wifi->connect(); - if (ok) - net->associating = true; - if (!ok) { - net->set_status("connect failed (Connect or a scan retries)"); - d.notify({.title = "Wi-Fi", - .text = fmt::format("could not start connecting to {}", ssid), - .level = D::NotifyLevel::Error}); - } + if (net->run_job("wifi_connect", + [state, ssid, pass]() { state->connect_job(ssid, pass); })) + set_busy(true); }); disconnect_btn.on_event([=](const D::WidgetEvent &e) mutable { - if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) + if (e.kind != D::WidgetEventKind::Click) return; - net->stop_station(); - net->set_status("disconnected"); + if (net->run_job("wifi_disconnect", [state]() { state->disconnect_job(); })) + set_busy(true); }); - forget_btn.on_event([=, &d](const D::WidgetEvent &e) mutable { - if (e.kind != D::WidgetEventKind::Click || refuse_if_scanning()) + forget_btn.on_event([=](const D::WidgetEvent &e) mutable { + if (e.kind != D::WidgetEventKind::Click) return; - const auto r = net->forget(); - d.notify({.title = "Wi-Fi", - .text = !r.erased ? "Forget failed: the credentials may still be saved " - "in NVS" - : !r.station_reset - ? "credentials erased, but the station could not be reset" - : "saved network forgotten", - .level = !r.erased ? D::NotifyLevel::Error - : !r.station_reset ? D::NotifyLevel::Warn - : D::NotifyLevel::Ok}); + if (net->run_job("wifi_forget", [state]() { state->forget_job(); })) + set_busy(true); }); #endif From 46d765129a1ffbf0d0f6663786babc711f3271d7 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 06:03:11 -0500 Subject: [PATCH 16/21] fix(desktop): address the fifteenth Copilot review of the Network app - start_attempt() enters Associating (and sets the status) BEFORE calling WifiSta::connect(): GOT_IP can fire on the event-loop task before connect() returns, and the later unconditional write would have moved Connected back to Associating for good. After connect() the callbacks own the phase; the failure path only falls back to Idle with a compare-exchange when the phase is still Associating. - Connect saves the credentials from the ACCEPTED worker job (after the busy check and a started task), so a refused Connect leaves the saved network in NVS unchanged. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/network_app.hpp | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 8fa2860662..8b53a079af 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -349,15 +349,26 @@ struct NetworkState { const uint32_t gen = new_attempt(); // after stop_and_wait() WifiSta's connected_ is false, so reconfigure() // does not connect by itself: connect() explicitly - const bool ok = wifi->reconfigure(wifi_config(ssid, pass, gen)) && wifi->connect(); - if (!ok) { + if (!wifi->reconfigure(wifi_config(ssid, pass, gen))) { phase = Phase::Idle; set_status(fmt::format("{} failed: use Connect", what)); - toast(fmt::format("could not start the {} to {}", what, ssid), D::NotifyLevel::Error); + toast(fmt::format("could not configure the {} to {}", what, ssid), D::NotifyLevel::Error); return false; } + // Associating is entered BEFORE connect(): GOT_IP can fire on the + // event-loop task before connect() returns, and the callbacks own the + // phase from here on (Connected / Idle). Nothing below writes the phase + // unconditionally. phase = Phase::Associating; set_status(fmt::format("connecting to {}", ssid)); + if (!wifi->connect()) { + // only back to Idle if no callback moved the phase on meanwhile + Phase expected = Phase::Associating; + phase.compare_exchange_strong(expected, Phase::Idle); + set_status(fmt::format("{} failed: use Connect", what)); + toast(fmt::format("could not start the {} to {}", what, ssid), D::NotifyLevel::Error); + return false; + } return true; } @@ -620,12 +631,14 @@ inline void register_network_app(espp::Desktop &desktop) { .level = D::NotifyLevel::Error}); return; } - if (!net->save_credentials(ssid, pass)) - d.notify({.title = "Wi-Fi", - .text = "could not save the credentials to NVS (connecting anyway)", - .level = D::NotifyLevel::Warn}); - if (net->run_job("wifi_connect", - [state, ssid, pass]() { state->connect_job(ssid, pass); })) + // the credentials are saved by the ACCEPTED job (after the busy + // check and a started worker): a refused Connect leaves NVS alone + if (net->run_job("wifi_connect", [state, ssid, pass]() { + if (!state->save_credentials(ssid, pass)) + state->toast("could not save the credentials to NVS (connecting anyway)", + D::NotifyLevel::Warn); + state->connect_job(ssid, pass); + })) set_busy(true); }); disconnect_btn.on_event([=](const D::WidgetEvent &e) mutable { From 3d8c9c05bfc7cf4349f0f43236e48da9bb53b607 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 06:15:40 -0500 Subject: [PATCH 17/21] fix(desktop,wifi): address the sixteenth Copilot review of the hardware apps - CANopen: a telemetry field whose SDO read failed shows "unavailable ()" instead of its previous value (mode, position, velocity independently). - wifi: WifiSta::disconnect() rolls its state back when esp_wifi_disconnect() fails -- disconnecting_ is cleared and connected_ restored -- since no DISCONNECTED event will arrive to consume the intentional-disconnect flag; before, the next (unintentional) disconnect was taken for the failed one and skipped its retries. - Network: a rejected disconnect or a missing DISCONNECTED event ABORTS the transition into the recoverable Failed phase (status "stopping failed / timed out: try again", toast, nothing reconfigured, the station left as it is) instead of continuing past the race the wait exists to close; the jobs (connect, disconnect, scan, forget) stop there and the user retries, the next job starting with a fresh stop_and_wait(). A later driver event resolves Failed through the callbacks; if none came and the driver reports no association, the station is taken as idle. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/canopen_app.hpp | 7 ++ .../desktop/example/main/apps/network_app.hpp | 66 ++++++++++++++----- components/wifi/include/wifi_sta.hpp | 8 ++- 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/components/desktop/example/main/apps/canopen_app.hpp b/components/desktop/example/main/apps/canopen_app.hpp index 6933d4829b..422276aa4c 100644 --- a/components/desktop/example/main/apps/canopen_app.hpp +++ b/components/desktop/example/main/apps/canopen_app.hpp @@ -276,16 +276,23 @@ struct CanopenSession { status_label.set_text("Statusword: 0x{:04X}", sw); // one error code per field: the client only sets ec on failure (never // clears it), so a shared one would hide every field after a failed read + // ... and a field whose read failed shows that, not its previous value std::error_code mode_ec, pos_ec, vel_ec; const int8_t mode = drive->get_mode_display(mode_ec); if (!mode_ec) mode_label.set_text("Mode (0x6061): {}", mode); + else + mode_label.set_text("Mode (0x6061): unavailable ({})", mode_ec.message()); const int32_t pos = drive->get_position_actual(pos_ec); if (!pos_ec) pos_label.set_text("Position: {}", pos); + else + pos_label.set_text("Position: unavailable ({})", pos_ec.message()); const int32_t vel = drive->get_velocity_actual(vel_ec); if (!vel_ec) vel_label.set_text("Velocity: {}", vel); + else + vel_label.set_text("Velocity: unavailable ({})", vel_ec.message()); } }; diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 8b53a079af..e9d2ec862e 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -62,7 +62,13 @@ struct NetworkState { /// The station state machine, driven by the jobs below on the worker task /// and by the driver callbacks (got-ip -> Connected, retries exhausted -> /// Idle). `Stopping` is the explicit wait for the DISCONNECTED event. - enum class Phase : uint8_t { Idle, Stopping, Configuring, Associating, Connected }; + /// `Failed` is the recoverable error state a transition aborts into when + /// the station could not be stopped (the disconnect was rejected, or its + /// DISCONNECTED event did not arrive in time): nothing is reconfigured, the + /// station is left as it is, the user retries. A later event resolves it + /// (the callbacks still own the phase), and the next job starts with a + /// fresh stop_and_wait(). + enum class Phase : uint8_t { Idle, Stopping, Configuring, Associating, Connected, Failed }; std::atomic phase{Phase::Idle}; /// A job (scan / connect / disconnect / forget) is running on the worker: /// every station control is disabled, and another job is refused. @@ -311,9 +317,24 @@ struct NetworkState { /// with the stopped one. On an idle station WifiSta::disconnect() is not /// called at all: no event would come and its private `disconnecting_` /// would stay set, costing the next attempt its retries. Worker task. + /// + /// The rule on failure: a rejected disconnect or a missing event ABORTS the + /// transition (phase Failed, a toast, false returned) -- the caller must + /// not reconfigure, because continuing would re-open the stale-event race + /// this wait exists to close. The station is left as it is and the user + /// retries. From Failed, a later event has usually resolved the phase + /// (Idle / Connected); if none came and the driver reports no association, + /// the station is taken as idle. /// Returns true once the station is idle. bool stop_and_wait(std::chrono::milliseconds timeout = std::chrono::milliseconds(1000)) { - const Phase p = phase.load(); + Phase p = phase.load(); + if (p == Phase::Failed) { + wifi_ap_record_t ap{}; + if (esp_wifi_sta_get_ap_info(&ap) == ESP_OK) + p = Phase::Connected; // still associated: stop it like a connected station + else + return true; // nothing associated: idle + } if (p != Phase::Connected && p != Phase::Associating) return true; phase = Phase::Stopping; @@ -324,21 +345,27 @@ struct NetworkState { seen = disconnected_events; } if (!wifi->disconnect()) { - // the driver rejected it (nothing was associated): no event will come - phase = Phase::Idle; - return true; + // the driver rejected it (WifiSta rolls its flags back): a failed + // transition, not an idle station + phase = Phase::Failed; + set_status("stopping failed: try again"); + toast("the station could not be stopped; try again", D::NotifyLevel::Error); + return false; } bool got = false; { std::unique_lock lock(mutex); got = disconnected_cv.wait_for(lock, timeout, [&] { return disconnected_events != seen; }); } - if (!got) - logger.warn("no DISCONNECTED event within {} ms; continuing (the generation check drops " - "it if it still arrives)", - timeout.count()); + if (!got) { + logger.warn("no DISCONNECTED event within {} ms; aborting the transition", timeout.count()); + phase = Phase::Failed; + set_status("stopping timed out: try again"); + toast("the station did not report its disconnect in time; try again", D::NotifyLevel::Error); + return false; + } phase = Phase::Idle; - return got; + return true; } /// Configuring -> Associating: a new attempt with `ssid` / `pass` (the @@ -375,15 +402,16 @@ struct NetworkState { /// Connect: stop whatever is active (waiting for its event), then attempt. void connect_job(const std::string &ssid, const std::string &pass) { want_connected = true; - stop_and_wait(); + if (!stop_and_wait()) + return; // aborted (Failed): nothing is reconfigured, the user retries start_attempt(ssid, pass, "connection"); } /// Disconnect: give up the desired state and stop the station. void disconnect_job() { want_connected = false; - stop_and_wait(); - set_status("disconnected"); + if (stop_and_wait()) + set_status("disconnected"); } /// Scan: stop the station first (WifiSta::scan() would otherwise drop the @@ -392,7 +420,8 @@ struct NetworkState { /// every window, then restore the desired state with a fresh attempt. void scan_job() { const bool intent = want_connected; - stop_and_wait(); + if (!stop_and_wait()) + return; // aborted: scan() would drop the link with a bare disconnect set_status("scanning"); const auto aps = wifi->scan(20); std::vector rows, ssids; @@ -438,7 +467,14 @@ struct NetworkState { erased = erased && !ec; } want_connected = false; - stop_and_wait(); + if (!stop_and_wait()) { + // the credentials are gone (or not: say so), the station is not reset + toast(erased ? "credentials erased, but the station could not be stopped; Forget again " + "to reset it" + : "Forget failed: the credentials may still be saved in NVS", + D::NotifyLevel::Error); + return; + } wifi_config_t empty{}; const bool cleared = esp_wifi_set_config(WIFI_IF_STA, &empty) == ESP_OK; // empty ssid: reconfigure() adopts the (now empty) driver config diff --git a/components/wifi/include/wifi_sta.hpp b/components/wifi/include/wifi_sta.hpp index 3a0088bf21..e0fc90f98d 100644 --- a/components/wifi/include/wifi_sta.hpp +++ b/components/wifi/include/wifi_sta.hpp @@ -602,12 +602,18 @@ class WifiSta : public WifiBase { * @return true if the disconnection was initiated successfully, false otherwise. */ bool disconnect() { - connected_ = false; + const bool was_connected = connected_.exchange(false); attempts_ = 0; disconnecting_ = true; esp_err_t err = esp_wifi_disconnect(); if (err != ESP_OK) { logger_.error("Could not disconnect from WiFi: {}", esp_err_to_name(err)); + // Nothing was initiated, so no DISCONNECTED event will arrive to consume + // the intentional-disconnect flag: leave the state as it was, otherwise + // the next (unintentional) disconnect would be taken for this one and + // skip its retries. + disconnecting_ = false; + connected_ = was_connected; return false; } return true; From 25f3c92a188c27359eabe9032602ca43c1271d21 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 06:29:28 -0500 Subject: [PATCH 18/21] fix(desktop,wifi): correlate DISCONNECTED events by draining stale ones; disconnect() leaves connected_ to the event Copilot round 17. - Network: WifiSta fetches its callbacks at delivery time, so the callback installed can never identify an event. The three callbacks are now installed ONCE (long-lived std::function members passed to every reconfigure()) and consult the shared state under the mutex: a stale_disconnects credit counter records every disconnect the worker requested but never saw confirmed (the wait timed out); the next DISCONNECTED deliveries consume those credits and are ignored, which drains the late confirmation whenever it arrives instead of letting it satisfy a later wait or move a later attempt to Idle. A disconnect the driver rejected leaves no credit (nothing was initiated); the Failed + no-association early return keeps the credit of the timed-out request. The attempt generation is gone (the credits replace it); the state machine stays on the worker. - wifi: WifiSta::disconnect() only initiates the disconnect -- it sets disconnecting_, calls esp_wifi_disconnect() and clears the flag again on error; connected_ is left to the event handler, which clears it when the DISCONNECTED event reports the disconnect done (touching it in disconnect() raced the event loop). reconfigure()'s wait loop now really waits for that event; the app's stop_and_wait() already did. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/network_app.hpp | 125 +++++++++--------- components/wifi/include/wifi_sta.hpp | 11 +- 2 files changed, 72 insertions(+), 64 deletions(-) diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index e9d2ec862e..7792b5fdd0 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -78,21 +78,53 @@ struct NetworkState { /// the driver callbacks. A scan stops the station and restores it afterwards /// when this is set, whatever the transient phase. std::atomic want_connected{false}; - /// Connection-attempt GENERATION, a belt-and-braces check only: the driver - /// events are asynchronous and WifiSta fetches `config_.on_disconnected` - /// at DELIVERY time, so the installed callback cannot tell which attempt an - /// event belongs to. Ownership is therefore never inferred from the - /// callback: before a new attempt the worker actually WAITS for the - /// DISCONNECTED event of the stopped station (stop_and_wait, bounded). The - /// generation only filters the residue (an event that still arrives after - /// the wait timed out). - std::atomic attempt{0}; + /// Correlating the asynchronous DISCONNECTED events with the worker's + /// requests. WifiSta fetches its callbacks at DELIVERY time, so "which + /// callback is installed" can never identify an event: the three callbacks + /// are therefore installed ONCE (ensure_wifi) and never replaced -- every + /// reconfigure() passes the same long-lived std::function objects -- and + /// they consult this state under `mutex`: + /// - `disconnected_events` counts the deliveries stop_and_wait() waits on; + /// - `stale_disconnects` is the number of disconnects the worker requested + /// but never saw confirmed (the wait timed out): the next DISCONNECTED + /// deliveries consume those credits and are ignored, which DRAINS the + /// late event whenever it arrives instead of letting it satisfy a later + /// wait or move a later attempt to Idle. std::condition_variable disconnected_cv; // with `mutex`: a DISCONNECTED event arrived - uint32_t disconnected_events{0}; // under `mutex`; any generation + uint32_t disconnected_events{0}; // under `mutex` + uint32_t stale_disconnects{0}; // under `mutex` std::string cur_ssid, cur_pass; // the credentials of the current attempt (under `mutex`) #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI std::unique_ptr wifi; std::unique_ptr worker; // after wifi: joined before it goes away + // the long-lived callbacks (see above); they run on the event-loop task + espp::WifiSta::connect_callback on_connected_fn = [this]() { + set_status("connected, waiting for an IP"); + }; + espp::WifiSta::disconnect_callback on_disconnected_fn = [this]() { + { + std::lock_guard lock(mutex); + if (stale_disconnects > 0) { + // the late confirmation of a disconnect whose wait timed out + --stale_disconnects; + logger.info("drained a stale DISCONNECTED ({} left)", stale_disconnects); + return; + } + ++disconnected_events; // a confirmation stop_and_wait() may be waiting on + } + disconnected_cv.notify_all(); + if (phase == Phase::Stopping) + return; // the worker owns this transition + phase = Phase::Idle; // retries exhausted + set_status(want_connected ? "disconnected (retries exhausted; Connect or a scan retries)" + : "disconnected"); + }; + espp::WifiSta::ip_callback on_got_ip_fn = [this](ip_event_got_ip_t *e) { + if (phase == Phase::Stopping) + return; // being stopped: the DISCONNECTED that follows settles it + phase = Phase::Connected; + set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); + }; #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET std::unique_ptr eth; @@ -130,10 +162,11 @@ struct NetworkState { } } - /// The station config for `ssid` / `password` of attempt `gen`: the - /// callbacks drive the phase (they run on the event-loop task) and ignore - /// the residue of an older generation (see `attempt`). - espp::WifiSta::Config wifi_config(std::string ssid, std::string password, uint32_t gen) { + /// The station config for `ssid` / `password`: only the credentials change + /// between attempts, the callbacks are always the same long-lived objects + /// (WifiSta::reconfigure() replaces its whole config, so they are passed + /// again each time). + espp::WifiSta::Config wifi_config(std::string ssid, std::string password) { { std::lock_guard lock(mutex); cur_ssid = ssid; @@ -143,43 +176,12 @@ struct NetworkState { .password = std::move(password), .num_connect_retries = 3, .auto_connect = true, - .on_connected = - [this, gen]() { - if (gen == attempt) - set_status("connected, waiting for an IP"); - }, - .on_disconnected = - [this, gen]() { - // every DISCONNECTED (intentional or retries exhausted) wakes - // a stop_and_wait() in progress, whatever its generation - { - std::lock_guard lock(mutex); - ++disconnected_events; - } - disconnected_cv.notify_all(); - if (gen != attempt) - return; // residue of a previous attempt - if (phase == Phase::Stopping) - return; // the worker owns this transition - phase = Phase::Idle; // retries exhausted - set_status(want_connected ? "disconnected (retries exhausted; Connect or a " - "scan retries)" - : "disconnected"); - }, - .on_got_ip = - [this, gen](ip_event_got_ip_t *e) { - if (gen != attempt) - return; - phase = Phase::Connected; - set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); - }, + .on_connected = on_connected_fn, + .on_disconnected = on_disconnected_fn, + .on_got_ip = on_got_ip_fn, .log_level = espp::Logger::Verbosity::INFO}; } - /// Start a new attempt generation: the residue of the previous one is - /// ignored from here on. - uint32_t new_attempt() { return ++attempt; } - /// Whether a string key exists in the app's namespace: ESP_OK (it does), /// ESP_ERR_NVS_NOT_FOUND (it does not) or another error (a genuine NVS /// failure). NvsHandle maps every string read failure to one error code, @@ -274,7 +276,7 @@ struct NetworkState { auto &stack = espp::Wifi::get(); if (stack.init()) stack.set_storage(WIFI_STORAGE_RAM); - auto cfg = wifi_config(ssid, pass, new_attempt()); + auto cfg = wifi_config(ssid, pass); cfg.auto_connect = !ssid.empty(); want_connected = cfg.auto_connect; // WifiSta connects from its STA_START event: that is an association @@ -322,9 +324,12 @@ struct NetworkState { /// transition (phase Failed, a toast, false returned) -- the caller must /// not reconfigure, because continuing would re-open the stale-event race /// this wait exists to close. The station is left as it is and the user - /// retries. From Failed, a later event has usually resolved the phase + /// retries. A timed-out wait leaves a `stale_disconnects` credit, so the + /// confirmation that never came is drained by the callback whenever it + /// arrives (never counted for a later wait, never applied to a later + /// attempt). From Failed, a later event has usually resolved the phase /// (Idle / Connected); if none came and the driver reports no association, - /// the station is taken as idle. + /// the station is taken as idle (its credit stays until the event comes). /// Returns true once the station is idle. bool stop_and_wait(std::chrono::milliseconds timeout = std::chrono::milliseconds(1000)) { Phase p = phase.load(); @@ -333,7 +338,7 @@ struct NetworkState { if (esp_wifi_sta_get_ap_info(&ap) == ESP_OK) p = Phase::Connected; // still associated: stop it like a connected station else - return true; // nothing associated: idle + return true; // nothing associated: idle (the timed-out credit remains) } if (p != Phase::Connected && p != Phase::Associating) return true; @@ -345,8 +350,8 @@ struct NetworkState { seen = disconnected_events; } if (!wifi->disconnect()) { - // the driver rejected it (WifiSta rolls its flags back): a failed - // transition, not an idle station + // the driver rejected it (WifiSta rolls its flag back; nothing was + // initiated, so no credit): a failed transition, not an idle station phase = Phase::Failed; set_status("stopping failed: try again"); toast("the station could not be stopped; try again", D::NotifyLevel::Error); @@ -356,6 +361,8 @@ struct NetworkState { { std::unique_lock lock(mutex); got = disconnected_cv.wait_for(lock, timeout, [&] { return disconnected_events != seen; }); + if (!got) + ++stale_disconnects; // requested, never confirmed: drain it when it comes } if (!got) { logger.warn("no DISCONNECTED event within {} ms; aborting the transition", timeout.count()); @@ -373,10 +380,10 @@ struct NetworkState { bool start_attempt(const std::string &ssid, const std::string &pass, const char *what) { phase = Phase::Configuring; set_status(fmt::format("configuring {}", ssid)); - const uint32_t gen = new_attempt(); - // after stop_and_wait() WifiSta's connected_ is false, so reconfigure() - // does not connect by itself: connect() explicitly - if (!wifi->reconfigure(wifi_config(ssid, pass, gen))) { + // after stop_and_wait() the DISCONNECTED event has cleared WifiSta's + // connected_, so reconfigure() does not connect by itself: connect() + // explicitly + if (!wifi->reconfigure(wifi_config(ssid, pass))) { phase = Phase::Idle; set_status(fmt::format("{} failed: use Connect", what)); toast(fmt::format("could not configure the {} to {}", what, ssid), D::NotifyLevel::Error); @@ -478,7 +485,7 @@ struct NetworkState { wifi_config_t empty{}; const bool cleared = esp_wifi_set_config(WIFI_IF_STA, &empty) == ESP_OK; // empty ssid: reconfigure() adopts the (now empty) driver config - auto cfg = wifi_config("", "", new_attempt()); + auto cfg = wifi_config("", ""); cfg.auto_connect = false; const bool station_reset = wifi->reconfigure(cfg) && cleared; phase = Phase::Idle; diff --git a/components/wifi/include/wifi_sta.hpp b/components/wifi/include/wifi_sta.hpp index e0fc90f98d..e991f6973e 100644 --- a/components/wifi/include/wifi_sta.hpp +++ b/components/wifi/include/wifi_sta.hpp @@ -602,18 +602,19 @@ class WifiSta : public WifiBase { * @return true if the disconnection was initiated successfully, false otherwise. */ bool disconnect() { - const bool was_connected = connected_.exchange(false); + // Only INITIATES the disconnect: connected_ is cleared by the event + // handler when the DISCONNECTED event reports it done (touching it here + // would race the event loop), so is_connected() stays true until then. attempts_ = 0; disconnecting_ = true; esp_err_t err = esp_wifi_disconnect(); if (err != ESP_OK) { logger_.error("Could not disconnect from WiFi: {}", esp_err_to_name(err)); // Nothing was initiated, so no DISCONNECTED event will arrive to consume - // the intentional-disconnect flag: leave the state as it was, otherwise - // the next (unintentional) disconnect would be taken for this one and - // skip its retries. + // the intentional-disconnect flag; clear it, otherwise the next + // (unintentional) disconnect would be taken for this one and skip its + // retries. disconnecting_ = false; - connected_ = was_connected; return false; } return true; From 920873e76784341daa0f3082d6f6cd12ccd23bcc Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 07:39:36 -0500 Subject: [PATCH 19/21] fix(desktop): recover the Wi-Fi station deterministically when a DISCONNECTED wait times out Copilot round 18. A timed-out wait does not guarantee the missing DISCONNECTED ever arrives; carrying a drain credit forward could consume the NEXT real confirmation and wedge every later stop. The worker therefore never resumes with an unconfirmed disconnect outstanding: on timeout it RECOVERS the station -- esp_wifi_stop() (tears down any in-flight association; every event of the old session is delivered before WIFI_EVENT_STA_STOP and nothing trails after it), a bounded wait for STA_STOP (observed through an esp_event handler registered once, since WifiSta does not expose it), then esp_wifi_start() -- and continues the transition from a known-idle station; only a failed recovery aborts into Failed. The credit / drain bookkeeping is gone. Every station config now has auto_connect off (the start-up association is issued explicitly), so the recovery's start cannot associate to stale credentials by itself. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/network_app.hpp | 163 ++++++++++++------ 1 file changed, 113 insertions(+), 50 deletions(-) diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index 7792b5fdd0..af01b73a46 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -62,13 +62,22 @@ struct NetworkState { /// The station state machine, driven by the jobs below on the worker task /// and by the driver callbacks (got-ip -> Connected, retries exhausted -> /// Idle). `Stopping` is the explicit wait for the DISCONNECTED event. - /// `Failed` is the recoverable error state a transition aborts into when - /// the station could not be stopped (the disconnect was rejected, or its - /// DISCONNECTED event did not arrive in time): nothing is reconfigured, the - /// station is left as it is, the user retries. A later event resolves it - /// (the callbacks still own the phase), and the next job starts with a - /// fresh stop_and_wait(). - enum class Phase : uint8_t { Idle, Stopping, Configuring, Associating, Connected, Failed }; + /// `Recovering` is the deterministic stop / start of the whole station + /// (recover()) after a DISCONNECTED wait timed out. `Failed` is the + /// recoverable error state a transition aborts into when the station could + /// not be stopped (the disconnect was rejected, or the recovery itself + /// failed): nothing is reconfigured, the station is left as it is, the user + /// retries. A later event resolves it (the callbacks still own the phase), + /// and the next job starts with a fresh stop_and_wait(). + enum class Phase : uint8_t { + Idle, + Stopping, + Recovering, + Configuring, + Associating, + Connected, + Failed + }; std::atomic phase{Phase::Idle}; /// A job (scan / connect / disconnect / forget) is running on the worker: /// every station control is disabled, and another job is refused. @@ -83,20 +92,23 @@ struct NetworkState { /// callback is installed" can never identify an event: the three callbacks /// are therefore installed ONCE (ensure_wifi) and never replaced -- every /// reconfigure() passes the same long-lived std::function objects -- and - /// they consult this state under `mutex`: - /// - `disconnected_events` counts the deliveries stop_and_wait() waits on; - /// - `stale_disconnects` is the number of disconnects the worker requested - /// but never saw confirmed (the wait timed out): the next DISCONNECTED - /// deliveries consume those credits and are ignored, which DRAINS the - /// late event whenever it arrives instead of letting it satisfy a later - /// wait or move a later attempt to Idle. + /// they consult this state under `mutex`. `disconnected_events` counts the + /// deliveries stop_and_wait() waits on. When that wait times out the + /// station is RECOVERED deterministically (recover(): esp_wifi_stop(), + /// wait for WIFI_EVENT_STA_STOP, esp_wifi_start()) -- the stop tears down + /// any in-flight association, every event of the old session is delivered + /// before STA_STOP, and nothing trails after it -- so the worker never + /// resumes with an unconfirmed disconnect outstanding and no credit / + /// drain bookkeeping is needed. std::condition_variable disconnected_cv; // with `mutex`: a DISCONNECTED event arrived uint32_t disconnected_events{0}; // under `mutex` - uint32_t stale_disconnects{0}; // under `mutex` + std::condition_variable stopped_cv; // with `mutex`: WIFI_EVENT_STA_STOP arrived + uint32_t stopped_events{0}; // under `mutex` std::string cur_ssid, cur_pass; // the credentials of the current attempt (under `mutex`) #if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI std::unique_ptr wifi; - std::unique_ptr worker; // after wifi: joined before it goes away + std::unique_ptr worker; // after wifi: joined before it goes away + esp_event_handler_instance_t sta_stop_handler{nullptr}; // WIFI_EVENT_STA_STOP -> stopped_cv // the long-lived callbacks (see above); they run on the event-loop task espp::WifiSta::connect_callback on_connected_fn = [this]() { set_status("connected, waiting for an IP"); @@ -104,27 +116,32 @@ struct NetworkState { espp::WifiSta::disconnect_callback on_disconnected_fn = [this]() { { std::lock_guard lock(mutex); - if (stale_disconnects > 0) { - // the late confirmation of a disconnect whose wait timed out - --stale_disconnects; - logger.info("drained a stale DISCONNECTED ({} left)", stale_disconnects); - return; - } ++disconnected_events; // a confirmation stop_and_wait() may be waiting on } disconnected_cv.notify_all(); - if (phase == Phase::Stopping) + const Phase p = phase.load(); + if (p == Phase::Stopping || p == Phase::Recovering) return; // the worker owns this transition phase = Phase::Idle; // retries exhausted set_status(want_connected ? "disconnected (retries exhausted; Connect or a scan retries)" : "disconnected"); }; espp::WifiSta::ip_callback on_got_ip_fn = [this](ip_event_got_ip_t *e) { - if (phase == Phase::Stopping) - return; // being stopped: the DISCONNECTED that follows settles it + const Phase p = phase.load(); + if (p == Phase::Stopping || p == Phase::Recovering) + return; // being stopped: the DISCONNECTED / STA_STOP that follows settles it phase = Phase::Connected; set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); }; + /// WIFI_EVENT_STA_STOP (event-loop task): wakes recover(). + static void on_sta_stop(void *arg, esp_event_base_t, int32_t, void *) { + auto *self = static_cast(arg); + { + std::lock_guard lock(self->mutex); + ++self->stopped_events; + } + self->stopped_cv.notify_all(); + } #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET std::unique_ptr eth; @@ -175,7 +192,10 @@ struct NetworkState { return {.ssid = std::move(ssid), .password = std::move(password), .num_connect_retries = 3, - .auto_connect = true, + // never auto-connect from STA_START: every association is issued + // explicitly by the worker (and once at start-up), so a recovery's + // esp_wifi_start() cannot connect to stale credentials by itself + .auto_connect = false, .on_connected = on_connected_fn, .on_disconnected = on_disconnected_fn, .on_got_ip = on_got_ip_fn, @@ -276,14 +296,25 @@ struct NetworkState { auto &stack = espp::Wifi::get(); if (stack.init()) stack.set_storage(WIFI_STORAGE_RAM); - auto cfg = wifi_config(ssid, pass); - cfg.auto_connect = !ssid.empty(); - want_connected = cfg.auto_connect; - // WifiSta connects from its STA_START event: that is an association - phase = cfg.auto_connect ? Phase::Associating : Phase::Idle; - set_status(ssid.empty() ? "idle (no saved network)" : "connecting"); - wifi = std::make_unique(cfg); + wifi = std::make_unique(wifi_config(ssid, pass)); wifi_mac = wifi->get_mac(); + // recover() waits for the station's STOP event, which WifiSta does not + // expose: observe it directly + esp_event_handler_instance_register(WIFI_EVENT, WIFI_EVENT_STA_STOP, &on_sta_stop, this, + &sta_stop_handler); + // the start-up association is issued explicitly (auto_connect is off) + want_connected = !ssid.empty(); + if (ssid.empty()) { + phase = Phase::Idle; + set_status("idle (no saved network)"); + } else { + phase = Phase::Associating; + set_status(fmt::format("connecting to {}", ssid)); + if (!wifi->connect()) { + phase = Phase::Idle; + set_status("connect failed: use Connect"); + } + } } // ---- the worker: one job at a time, every station control disabled ---- @@ -320,16 +351,16 @@ struct NetworkState { /// called at all: no event would come and its private `disconnecting_` /// would stay set, costing the next attempt its retries. Worker task. /// - /// The rule on failure: a rejected disconnect or a missing event ABORTS the - /// transition (phase Failed, a toast, false returned) -- the caller must - /// not reconfigure, because continuing would re-open the stale-event race - /// this wait exists to close. The station is left as it is and the user - /// retries. A timed-out wait leaves a `stale_disconnects` credit, so the - /// confirmation that never came is drained by the callback whenever it - /// arrives (never counted for a later wait, never applied to a later - /// attempt). From Failed, a later event has usually resolved the phase - /// (Idle / Connected); if none came and the driver reports no association, - /// the station is taken as idle (its credit stays until the event comes). + /// The rule on failure: a rejected disconnect ABORTS the transition (phase + /// Failed, a toast, false returned) -- the caller must not reconfigure. A + /// missing DISCONNECTED (the wait timed out) is never resumed from with the + /// confirmation outstanding -- it may never come, and resuming would re-open + /// the stale-event race this wait exists to close: the station is RECOVERED + /// (recover(): stop the whole station, wait for STA_STOP, start it again), + /// after which it is known idle and the transition continues; only a failed + /// recovery aborts into Failed. From Failed, a later event has usually + /// resolved the phase (Idle / Connected); if none came and the driver + /// reports no association, the station is taken as idle. /// Returns true once the station is idle. bool stop_and_wait(std::chrono::milliseconds timeout = std::chrono::milliseconds(1000)) { Phase p = phase.load(); @@ -338,7 +369,7 @@ struct NetworkState { if (esp_wifi_sta_get_ap_info(&ap) == ESP_OK) p = Phase::Connected; // still associated: stop it like a connected station else - return true; // nothing associated: idle (the timed-out credit remains) + return true; // nothing associated: idle } if (p != Phase::Connected && p != Phase::Associating) return true; @@ -351,7 +382,7 @@ struct NetworkState { } if (!wifi->disconnect()) { // the driver rejected it (WifiSta rolls its flag back; nothing was - // initiated, so no credit): a failed transition, not an idle station + // initiated): a failed transition, not an idle station phase = Phase::Failed; set_status("stopping failed: try again"); toast("the station could not be stopped; try again", D::NotifyLevel::Error); @@ -361,14 +392,46 @@ struct NetworkState { { std::unique_lock lock(mutex); got = disconnected_cv.wait_for(lock, timeout, [&] { return disconnected_events != seen; }); - if (!got) - ++stale_disconnects; // requested, never confirmed: drain it when it comes } if (!got) { - logger.warn("no DISCONNECTED event within {} ms; aborting the transition", timeout.count()); + logger.warn("no DISCONNECTED event within {} ms; recovering the station", timeout.count()); + return recover(); + } + phase = Phase::Idle; + return true; + } + + /// Recovering: stop the whole station (esp_wifi_stop() tears down any + /// in-flight association; every event of the old session -- a DISCONNECTED + /// included -- is delivered before WIFI_EVENT_STA_STOP and nothing trails + /// after it), wait for STA_STOP (bounded), then start it again. With + /// auto_connect off nothing associates by itself, so the station is known + /// idle afterwards and the interrupted transition continues from there. + /// espp::Wifi::stop() only serves the registry's active interface (this + /// station is standalone), hence the driver calls. Worker task. + bool recover(std::chrono::milliseconds timeout = std::chrono::milliseconds(2000)) { + phase = Phase::Recovering; + set_status("recovering the station"); + uint32_t seen = 0; + { + std::lock_guard lock(mutex); + seen = stopped_events; + } + const esp_err_t stop_err = esp_wifi_stop(); + bool stopped = stop_err == ESP_OK; + if (stopped) { + std::unique_lock lock(mutex); + stopped = stopped_cv.wait_for(lock, timeout, [&] { return stopped_events != seen; }); + } + // start again even after a failed stop (the station may already be + // stopped); WifiSta::start() is esp_wifi_start() with its logging + const bool started = wifi->start(); + if (!stopped || !started) { + logger.error("station recovery failed (stop: {}, STA_STOP: {}, start: {})", + esp_err_to_name(stop_err), stopped ? "seen" : "missing", started); phase = Phase::Failed; - set_status("stopping timed out: try again"); - toast("the station did not report its disconnect in time; try again", D::NotifyLevel::Error); + set_status("recovery failed: try again"); + toast("the station could not be recovered; try again", D::NotifyLevel::Error); return false; } phase = Phase::Idle; From c804fb29b9938a124cb9edfa908472cb25d0c4b9 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 07:52:30 -0500 Subject: [PATCH 20/21] fix(desktop): withdraw the Network app's STA_STOP handler before the station goes away Copilot round 19. The WIFI_EVENT_STA_STOP instance handler registered in ensure_wifi() was never unregistered: ~WifiSta stops the station, which queues a STA_STOP, and a late delivery would have called on_sta_stop with a freed `this`. The registration result is now checked (a failure is logged; recovery then cannot observe STA_STOP and fails cleanly), the NetworkState destructor joins the worker, withdraws the handler -- the registry entry, then the esp_event registration -- and only then destroys the station, and the handler itself only touches instances still present in a static registry of live NetworkStates, so a delivery that outlives its instance is dropped whatever the event loop's unregister ordering. In practice the destructor is never reached (the state lives as long as the desktop); it is correct anyway. NetworkState is non-copyable. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../desktop/example/main/apps/network_app.hpp | 58 ++++++++++++++++++- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index af01b73a46..d040e94bad 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -48,6 +49,42 @@ struct NetworkState { explicit NetworkState(D &desktop) : desktop(desktop) {} + NetworkState(const NetworkState &) = delete; + NetworkState &operator=(const NetworkState &) = delete; + + /// In practice never reached: the state is owned by the app's launch + /// callback and lives as long as the desktop. Still correct: the worker is + /// joined first (nothing drives the station any more), the STA_STOP + /// handler is withdrawn (registry entry, then the esp_event registration) + /// before the station is destroyed -- ~WifiSta stops the station, which + /// queues a STA_STOP that must not reach a freed `this` -- and the handler + /// itself only touches instances still in the registry. + ~NetworkState() { +#if CONFIG_DESKTOP_EXAMPLE_ENABLE_WIFI + worker.reset(); + { + std::lock_guard lock(registry_mutex()); + live_instances().erase(this); + } + if (sta_stop_handler) { + esp_event_handler_instance_unregister(WIFI_EVENT, WIFI_EVENT_STA_STOP, sta_stop_handler); + sta_stop_handler = nullptr; + } + wifi.reset(); +#endif + } + + /// The live instances a WIFI_EVENT_STA_STOP delivery may touch (a late + /// delivery after an instance withdrew is dropped, whatever the event + /// loop's unregister ordering). + static std::mutex ®istry_mutex() { + static std::mutex m; + return m; + } + static std::set &live_instances() { + static std::set s; + return s; + } D &desktop; ///< outlives every task (toasts from the worker) espp::Logger logger{{.tag = "Network app", .level = espp::Logger::Verbosity::INFO}}; @@ -133,9 +170,14 @@ struct NetworkState { phase = Phase::Connected; set_status("connected", fmt::format("{}.{}.{}.{}", IP2STR(&e->ip_info.ip))); }; - /// WIFI_EVENT_STA_STOP (event-loop task): wakes recover(). + /// WIFI_EVENT_STA_STOP (event-loop task): wakes recover(). `arg` is only + /// dereferenced while it is a live instance (see the registry): a delivery + /// that outlives its NetworkState is dropped. static void on_sta_stop(void *arg, esp_event_base_t, int32_t, void *) { auto *self = static_cast(arg); + std::lock_guard registry(registry_mutex()); + if (!live_instances().count(self)) + return; // stale: the instance withdrew (or is being destroyed) { std::lock_guard lock(self->mutex); ++self->stopped_events; @@ -300,8 +342,18 @@ struct NetworkState { wifi_mac = wifi->get_mac(); // recover() waits for the station's STOP event, which WifiSta does not // expose: observe it directly - esp_event_handler_instance_register(WIFI_EVENT, WIFI_EVENT_STA_STOP, &on_sta_stop, this, - &sta_stop_handler); + { + std::lock_guard lock(registry_mutex()); + live_instances().insert(this); + } + const esp_err_t reg = esp_event_handler_instance_register( + WIFI_EVENT, WIFI_EVENT_STA_STOP, &on_sta_stop, this, &sta_stop_handler); + if (reg != ESP_OK) { + // recover() then cannot observe STA_STOP (its wait times out -> Failed) + sta_stop_handler = nullptr; + logger.error("could not register the STA_STOP handler: {}; station recovery will not work", + esp_err_to_name(reg)); + } // the start-up association is issued explicitly (auto_connect is off) want_connected = !ssid.empty(); if (ssid.empty()) { From 465c8673ca26088ad78943ef29953253a251651c Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sun, 4 Oct 2026 14:59:24 -0500 Subject: [PATCH 21/21] fix(desktop): I2C scan skips reserved addresses by default; Ethernet init failures are shown - I2C scanner: Scan probes 0x08..0x77 unless "include reserved addresses" is ticked (0x01..0x07 / 0x78..0x7F can have side effects); wording fix - Network: a failed espp::Ethernet::initialize() is kept (eth_init_error), shown in the Link label and toasted, instead of a MAC with no link forever (compiled on an esp32p4 build with the Ethernet group enabled) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- .../example/main/apps/i2c_scanner_app.hpp | 20 ++++++++++++++++--- .../desktop/example/main/apps/network_app.hpp | 18 +++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/components/desktop/example/main/apps/i2c_scanner_app.hpp b/components/desktop/example/main/apps/i2c_scanner_app.hpp index a7d13890cf..927fe9baf7 100644 --- a/components/desktop/example/main/apps/i2c_scanner_app.hpp +++ b/components/desktop/example/main/apps/i2c_scanner_app.hpp @@ -144,9 +144,21 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { // ---- widgets ---- auto bar = win.row(); - auto status = win.label("Press Scan to probe addresses 0x01..0x7F.", bar.id()); + auto status = win.label("Press Scan to probe addresses 0x08..0x77.", bar.id()); auto table = win.table({"Address", "Decimal", "Note"}, {}, nullptr); auto scan_btn = win.button("Scan", nullptr, bar.id(), D::kButtonPrimary); + // the reserved 7-bit ranges (0x00..0x07 general call / CBUS / hs-mode + // master codes, 0x78..0x7F 10-bit addressing / device id) are left out + // of a scan unless asked for: traffic there can have side effects + auto include_reserved = std::make_shared(false); + win.checkbox( + "include reserved addresses (0x01..0x07, 0x78..0x7F)", false, + [=](bool on) mutable { + *include_reserved = on; + status.set_text("Press Scan to probe addresses {}.", + on ? "0x01..0x7F" : "0x08..0x77"); + }, + bar.id()); auto tools = win.group("Register access"); auto fields = win.row(tools.id()); auto addr_box = win.textbox("", nullptr, fields.id(), "addr (hex)"); @@ -170,7 +182,7 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { // ---- the worker ---- // Every bus transaction (a scan, a register read or write) runs on - // one one-shot worker task, never on the desktop task (a scan is up + // a single one-shot worker task, never on the desktop task (a scan is up // to ~1.3 s, a 64-byte write at 100 kHz ~6 ms); the three buttons // are disabled until it is done and the result is posted back // through the widget mutators. The bus outlives the task: on_close @@ -226,10 +238,12 @@ inline void register_i2c_scanner_app(espp::Desktop &desktop) { status.set_text("Scanning\xE2\x80\xA6"); auto *bus = st->bus.get(); auto *session = st.get(); + const uint8_t first = *include_reserved ? 0x01 : 0x08; + const uint8_t last = *include_reserved ? 0x7F : 0x77; run_worker([=]() mutable { std::vector rows; std::vector found; - for (uint8_t addr = 0x01; addr <= 0x7F; ++addr) { + for (uint8_t addr = first; addr <= last; ++addr) { if (!bus->probe_device(addr)) continue; found.push_back(addr); diff --git a/components/desktop/example/main/apps/network_app.hpp b/components/desktop/example/main/apps/network_app.hpp index d040e94bad..227fa3d5a0 100644 --- a/components/desktop/example/main/apps/network_app.hpp +++ b/components/desktop/example/main/apps/network_app.hpp @@ -187,6 +187,7 @@ struct NetworkState { #endif #if CONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET std::unique_ptr eth; + std::string eth_init_error{}; ///< why initialize() failed (empty = ok / not attempted) #endif void set_status(std::string_view status, std::string_view ip = "") { @@ -651,7 +652,12 @@ struct NetworkState { .hostname = "espp-desktop", .log_level = espp::Logger::Verbosity::INFO}); std::error_code ec; - eth->initialize(ec); + if (!eth->initialize(ec)) { + eth_init_error = ec ? ec.message() : "initialize failed"; + toast("RMII Ethernet init failed: " + eth_init_error + + " (check the PHY wiring / board choice)", + D::NotifyLevel::Error); + } } #endif }; @@ -823,9 +829,13 @@ inline void register_network_app(espp::Desktop &desktop) { auto speed = win.label("Speed: -", eth.id(), D::kLabelMonospace); refreshers.push_back([=]() mutable { const bool up = net->eth->link_up(); - link.set_text("Link: {}", !net->eth->is_initialized() ? "not initialized" - : up ? "up" - : "down"); + if (!net->eth->is_initialized()) + link.set_text("Link: not initialized{}", + net->eth_init_error.empty() + ? "" + : " (init failed: " + net->eth_init_error + ")"); + else + link.set_text("Link: {}", up ? "up" : "down"); eth_ip.set_text("IP: {}", net->eth->is_connected() ? net->eth->get_ip_address() : "-"); if (const auto sd = net->eth->link_speed_duplex(); sd)