You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(desktop): Kconfig-gated CANopen/DS402, I2C scanner and Network apps for the desktop example - #840
Stacked on feat/desktop (PR A): the three Kconfig-gated hardware apps for the desktop example, per the plan's "PR B".
Apps (components/desktop/example/main/apps/)
CANopen / DS402 (canopen_app.hpp) — espp::CanopenClient + espp::Ds402Drive on the bus Kconfig selects: the can_bridge example's simulated DS402 node (can_bridge::SimulatedCanBus, the default, no hardware) or the TWAI peripheral (using CanBus = ... exactly like can_bridge_example.cpp). Window: node id + Apply (rebuilds bus/client/drive), NMT Start / Stop / Pre-operational / Reset node, NMT state (heartbeat callback) + DS402 state + statusword + mode-display labels, mode Select (PP / PV / PT / Homing), target-velocity Slider + Apply, Enable / Disable / Quick stop / Fault reset, position / velocity labels, an SDO row (index / sub / width / value + Read / Write + result label) and, on the simulated bus, Inject fault (writes 1 to 0x2000). Every bus transaction runs on the app's own espp::Task (SDO calls block; bus.on_receive -> client.process_frame never does SDO); the widget handlers only queue commands and the task polls statusword / position / velocity / mode every 250 ms and updates the widgets from there. Errors -> notify level Error (with the SDO abort code when there is one). Teardown happens in the window's on_close (desktop task, no lock held) so the blocking task join never runs under the model mutex.
I2C scanner (i2c_scanner_app.hpp) — espp::I2c (new API, auto_init = false + init(ec); a failed init shows a hint label instead of the tools). Scan probes 0x01..0x7F on a one-shot task into a Table (hex / decimal / note), selecting a row fills the address field; addr / register / length / value TextBoxes with Read (read_at_register -> hex dump) and Write (write of register + bytes).
Network (network_app.hpp) — Wi-Fi group (espp::WifiSta, constructed on first launch with the NVS credentials desktop/wifi_ssid + wifi_pass, idle when none): status / SSID / IP / RSSI / MAC labels on a 1 s timer, Scan on its own task (label "scanning…", AP list "SSID (RSSI, auth, ch)"), password TextBox (Password flag), Connect (selected AP + password, saved to NVS; reconfigure() with a fresh config so stale SSID / password bytes never leak) / Disconnect / Forget. Ethernet group (own gate, espp::Ethernet::RmiiConfig with the ESP32-Ethernet-Kit pins): link / IP / MAC / speed labels on the same timer. The interfaces persist across windows (closing the window does not disconnect).
Build wiring: canopen twai i2c wifi ethernet cli added to the example's EXTRA_COMPONENT_DIRS / COMPONENTS and mainREQUIRES (unconditionally — REQUIRES cannot depend on Kconfig; the options only decide which apps are registered). main/CMakeLists.txt adds components/canopen/can_bridge_example/main to INCLUDE_DIRS for the simulated bus / node headers (promoting them into the canopen component is a follow-up). READMEs: app list + Kconfig table in the example README (included by the docs), app mention in the component README.
OK — 0x185cb0 B, 13% of the 1792K app partition free
every optional app off (ENABLE_CANOPEN/I2C/WIFI/ETHERNET=n, LOG_CAPTURE=n)
OK — 0xd08f0 B, 53% free
CANopen on TWAI (CANOPEN_BUS_TWAI=y)
OK
Ethernet group compile check (-DCONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET=1 via EXTRA_CXXFLAGS, since the S3 has no EMAC)
OK
No warnings from the new code (-Werror=missing-field-initializers satisfied; only pre-existing deprecation / unused warnings in ethernet.cpp). cppcheck --check-level=exhaustive --force --enable=all ... on the example main dir and on each app header with the config macros defined: clean.
Submodules needed for the manager-off build in a fresh worktree: components/format/detail/fmt external/esp-usb external/tinyusb components/cli/detail/cli and components/littlefs (recursive).
Notes
Hardware-untested: compiled only; the user flashes and tests (simulated CAN, I2C bus, Wi-Fi scan / connect, Ethernet on an ESP32-Ethernet-Kit).
The CANopen app is ~450 lines (the widget list from the plan is long); the other two are ~200 / ~300.
Masking arbitrary input turns invalid or blank addresses into real bus targets: for example, an empty field becomes 0x00, so clicking Write sends an I2C general-call broadcast, and 0x80 also aliases to 0x00. Parse the whole field, reject malformed/out-of-range values, and only let both Read and Write proceed for addresses 0x01..0x7F.
Commit NVS key erasures to persist Forget operation
Erasing NVS keys also requires commit(). As written, Forget clears the runtime driver/config but the previously committed wifi_ssid and wifi_pass can return after reboot, while the success toast says they were forgotten. Commit both erasures and include the NVS result in ok.
This issue also appears on line 309 of the same file.
This call's internal esp_wifi_disconnect() does not set WifiSta::disconnecting_. Because this app configures three retries, the resulting disconnect event calls esp_wifi_connect() while the blocking scan is starting (wifi_sta.hpp:680-695), making connected-state scans race the retry loop and potentially fail. Perform an intentional disconnect() first (which suppresses retries), remember whether the station was connected, and reconnect explicitly after the scan.
Second Copilot review (summary-only findings) addressed in d791417:
I2C address masking (i2c_scanner_app.hpp) — the address / register fields are now parsed as whole hex fields; empty, malformed or out-of-range entries are rejected with a message in the result label (address must be 0x01..0x7F — the range Scan probes — register 0x00..0xFF). Nothing is masked into a real target any more (an empty address masked to 0x00 was the general-call broadcast).
NVS erasures not committed (network_app.hpp Forget) — erase_item() / set_item() only stage the change, so Forget now commits the two erasures and folds the NVS result into its return value (the toast only claims success when the keys are really gone); Connect's save goes through a new save_credentials() that commits too and warns if it fails.
Scan vs. retry race (network_app.hpp scan task) — WifiSta::scan() drops the link with a bare esp_wifi_disconnect() that the DISCONNECTED handler answers with a retry (num_connect_retries > 0) while the scan starts. The scan task now calls the intentional disconnect() first (which suppresses the retry), waits for the event to land, scans, then reconnects explicitly when the station was connected.
Builds (esp32s3, IDF v6.1, manager off): CI defaults OK, all-off OK, no new warnings; cppcheck (CI flags) clean on both headers. Hardware-untested.
strtoul is used without checking the end pointer or overflow, then callers narrow the result. Inputs such as 6041junk are accepted, oversized indexes/subindexes wrap, and oversized u8/u16 values are silently truncated before an SDO write. Parse the whole field and enforce each destination width before queueing any bus operation.
This issue also appears on line 401 of the same file.
This parser silently accepts only a valid prefix and truncates values to eight bits: for example, 01 ZZ writes 01 and reports success, while 100 becomes 00. For a hardware write tool, malformed or out-of-range input must reject the entire field rather than issue a different transaction; return a parse result that carries failure and have the Write handler stop on it.
Third Copilot review, summary-only findings, addressed in fdab92a (the NVS NUL thread is answered inline):
CANopen field parsing (canopen_app.hpp) — CanopenSession::parse_field(text, base, max) parses the whole field (no trailing junk, no sign, ERANGE overflow rejected) and enforces the destination width before anything is queued: node id 1..127 (decimal), SDO index ≤ 0xFFFF and sub-index ≤ 0xFF (hex), value within the selected u8 / u16 / u32 width (0x.. or decimal). A bad entry shows the reason in the SDO result label (node id: a toast) instead of being narrowed.
I2C byte list (i2c_scanner_app.hpp) — parse_bytes() now returns nullopt for the whole field on any malformed or > 0xFF token (01 ZZ, 100 are rejected, nothing is truncated or partially written); Write stops with a message in the result label. The single-field parsers already rejected malformed / out-of-range input in the previous round.
Builds (esp32s3, IDF v6.1, manager off): CI defaults OK, all-off OK, no new warnings; cppcheck (CI flags) clean on all three headers. Hardware-untested.
strtoul is used without an end pointer, so invalid input silently becomes a valid read length: an empty or nonnumeric value becomes 1, 2junk becomes 2, and -1 clamps to 64. Reject malformed and out-of-range input instead of issuing an unintended I2C transaction.
Check credential erase errors before reporting success
e1 and e2 are never checked, so either erase can fail while a successful commit(ec) makes forget() report success. The credentials can then remain in NVS and reconnect after reboot. Clear both values while checking every operation (or otherwise distinguish a missing key from a real erase failure).
Suppress reconnects while scanning during Wi-Fi connection
This only suppresses reconnect retries when is_connected() is already true. During initial association or immediately after Connect, is_connected() is false; WifiSta::scan() then performs a raw disconnect, and its DISCONNECTED handler retries esp_wifi_connect() because disconnecting_ was never set (components/wifi/include/wifi_sta.hpp:680-698). Scanning in that state can race a reconnect and fail or produce inconsistent results. Track the connecting state too and intentionally disconnect/suppress retries before scanning, then restore the prior connection intent afterward.
Fourth Copilot review, summary-only findings, addressed in 2daf1fb:
I2C read length (i2c_scanner_app.hpp) — the length field now goes through the same strict field parser (parse_field(text, base, min, max), decimal, 1..64): empty / non-numeric / 2junk / signed input is refused with a message in the result label and no transaction is issued (previously strtoul + clamp).
Unchecked NVS erasures (network_app.hpp Forget) — each erasure is checked through erase_key(): a key that was never stored counts as already forgotten (NvsHandle::erase() reports a missing key as a failure, so the key is probed first), any other failure is a failure; both results and the commit are folded into forget()'s return value, so the toast only claims success when both keys are gone from flash.
Scan during association (network_app.hpp) — a connecting intent flag is set on Connect and on auto-connect at start and cleared on got-ip, on retries exhausted (on_disconnected), on Disconnect and on Forget. Before a scan, if the station is connected or connecting, the task calls the intentional disconnect() (sets disconnecting_, suppressing the retry WifiSta::scan()'s raw esp_wifi_disconnect() would otherwise trigger), waits for the event, scans, then restores the intent with connect().
Builds (esp32s3, IDF v6.1, manager off): CI defaults OK, all-off OK, no new warnings; cppcheck (CI flags) clean on both headers. Hardware-untested.
The CANopen worker's Task::start() result is ignored. If task creation fails, polling never starts and every widget action keeps appending closures to queue with no consumer, causing silent unbounded memory growth. Handle startup failure by reporting it and preventing run() from accepting commands until a worker is running.
This issue also appears on line 230 of the same file.
Task::start() can fail, but the Scan button was already disabled and only the worker callback re-enables it. On a task-creation failure this window can never retry the scan. Restore the button/status and release the failed task when start() returns false.
Fifth Copilot review, summary-only findings, addressed in 08db444 (the NVS not-found thread is answered inline):
CANopen Task::start() ignored (canopen_app.hpp) — start() now returns the task's result; on failure the task is released, the session is marked not running, the state label says so and a level-3 toast is shown. run() refuses with a toast (instead of enqueueing for a task that never consumes) when the session is not running, and the command queue is bounded to 64 entries regardless (a full queue is also refused with a toast).
I2C Task::start() failure (i2c_scanner_app.hpp) — on failure the task is released, the Scan button is re-enabled, the status label is restored and an error toast is shown.
Builds (esp32s3, IDF v6.1, manager off): CI defaults OK, all-off OK, no new warnings; cppcheck (CI flags) clean on all three headers. Hardware-untested.
last_abort_code() describes the most recent SDO transaction and NMT sends do not clear it. After an SDO abort, an unrelated later NMT transmit failure therefore includes the stale abort code; only append it when the current failure is an SDO protocol error.
This issue also appears in the following locations of the same file:
line 255
line 467
Only report credentials erased after successful NVS operations
When either NVS erasure or commit fails, erased is false and the credentials may still reconnect after reboot, but this persistent status claims there is no saved network. Make the status conditional on erased so users know Forget did not complete.
This issue also appears in the following locations of the same file:
Sixth Copilot review, summary-only findings, addressed in 3deb1c1:
Stale SDO abort code on non-SDO failures (canopen_app.hpp) — fail() takes an explicit sdo flag and only appends last_abort_code() when the failing call was an SDO transaction. Audited every site: bus init and the NMT sends (which never touch the client's abort state) no longer append it; set mode, the controlword / statusword drive commands (Enable / Disable / Quick stop / Fault reset), Inject fault, target velocity and the SDO read / write row do.
Forget status hides an NVS failure (network_app.hpp) — forget() returns {erased, station_reset}; the persistent status only says "idle (no saved network)" when the erase + commit succeeded and otherwise "Forget failed: the credentials may still be saved", and the toast is error (NVS failure) / warning (station not reset) / ok accordingly.
Builds (esp32s3, IDF v6.1, manager off): CI defaults OK, all-off OK, no new warnings; cppcheck (CI flags) clean on both headers. Hardware-untested.
The Read path ignores the selected width and always supplies a 4-byte span. CanopenClient::sdo_upload() uses the caller's span size when a valid expedited response omits its size, so an un-sized u8/u16 response is displayed as four bytes instead of the selected type. Size the span from the width selector, which also preserves width-mismatch validation for size-indicated responses.
Preserve desired connectivity during intentional Wi-Fi reconfiguration
on_disconnected also runs for the intentional disconnect inside WifiSta::reconfigure(). When Connect switches from an already-connected AP, line 441 sets connecting, but this callback can clear it while the replacement connection is still associating. A Scan in that interval then sees both is_connected() == false and connecting == false, so it neither suppresses the driver's retry nor restores the connection. Track desired connectivity separately from transient association state, clearing it only for explicit Disconnect/Forget.
This initializer only supplies the classic ESP32-Ethernet-Kit control/clock pins, but the Kconfig and README also expose the group on ESP32-P4. P4 routes the six RMII data signals and requires RmiiConfig::data_pins (as in components/esp32-p4-function-ev-board/src/ethernet.cpp:25-38); leaving it empty produces a compiled but nonfunctional P4 interface. Add board-selectable P4 data pins or restrict this option/documentation to the classic ESP32 configuration.
Restore scanning state when scanner task creation fails
Handle Task::start() failure here. If task creation fails, no callback clears scanning, so Scan, Connect, Disconnect, and Forget remain disabled permanently and the label remains “scanning…”. Restore the state just as the I2C scanner does on its task-start failure.
Seventh Copilot review, summary-only findings, addressed in ac77997:
SDO Read always 4 bytes (canopen_app.hpp) — the upload span is now sized from the width selector (u8 / u16 / u32 → 1 / 2 / 4 bytes), so an expedited response that omits its size is reported at the selected width instead of as 4 bytes.
on_disconnected clears the intent during an AP switch (network_app.hpp) — the transient connecting flag is replaced by want_connected, the DESIRED connectivity: set by Connect and by the saved-credentials auto-connect, cleared only by Disconnect / Forget; neither driver callback touches it (retries exhausted only annotates the status). The scan suppresses / restores the station from that flag (or an established connection), so a scan during the association that follows reconfigure()'s own disconnect is handled.
ESP32-P4 RMII data pins (network_app.hpp, Kconfig, README) — a DESKTOP_EXAMPLE_ETHERNET_BOARD choice (default per target) selects the wiring: ETHERNET_KIT (ESP32-Ethernet-Kit, fixed data pins) or P4_FUNCTION_EV (ESP32-P4-Function-EV-Board: MDC 31 / MDIO 52 / reset 51 / clock 50 and data_pins TX_EN 49, TXD0 34, TXD1 35, CRS_DV 28, RXD0 29, RXD1 30, as in components/esp32-p4-function-ev-board). Both branches compile-checked on the S3 via EXTRA_CXXFLAGS (the P4 one in this round).
Wi-Fi scan Task::start() failure (network_app.hpp) — the task is released, scanning cleared, the buttons re-enabled, the label restored and an error toast shown (same as the I2C scanner).
Builds (esp32s3, IDF v6.1, manager off): CI defaults OK, all-off OK, Ethernet/P4-pins compile check OK, no new warnings; cppcheck (CI flags) clean on both headers. Hardware-untested.
ec is reused for four independent SDO reads, but CanopenClient only promises to set it on failure and does not clear it after a successful transaction. If the mode read fails while position/velocity succeed, those successful values are still discarded because ec retains the mode error. Clear ec before each subsequent read (or use a separate error code per field) so one unsupported/intermittent object does not suppress the remaining telemetry.
Forget while disconnected leaves stale Wi-Fi disconnect state
Forget can also run while the station is already disconnected. WifiSta::disconnect() then leaves its private disconnecting_ marker set because ESP-IDF emits no disconnected event; reconfigure with empty credentials does not clear it, so the next saved-network Connect loses its retry path on the first failure. The WifiSta lifecycle needs to clear this marker before future connects or make disconnect idempotent in the already-disconnected state.
This issue also appears in the following locations of the same file:
line 392
line 485
Persistent scan task leaves reopened window with stale results
Closing the window does not stop this persistent scan task. Since Widget updates become no-ops after their window closes, a window reopened before the scan completes snapshots the old rows at launch and never receives the completed rows; its timer only updates labels/buttons. Synchronize ap_list from the shared scan results when a scan completes (for example via a result generation in the refresher), or bind/cancel the worker with the window.
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
… 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…ts 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…re apps
- CANopen: a telemetry field whose SDO read failed shows "unavailable
(<error>)" 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…es; 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…ONNECTED 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
…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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
If the missing DISCONNECTED event truly never arrives, WifiSta::disconnecting_ remains set by the preceding successful disconnect(). Stopping and restarting the driver here does not clear that flag, so the next genuine connection loss is misclassified as intentional and skips all configured retries. Recovery must also reset/cancel WifiSta's pending intentional-disconnect state (for example via a WifiSta API or by rebuilding the station) before a new attempt starts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on
feat/desktop(PR A): the three Kconfig-gated hardware apps for the desktop example, per the plan's "PR B".Apps (
components/desktop/example/main/apps/)canopen_app.hpp) —espp::CanopenClient+espp::Ds402Driveon the bus Kconfig selects: the can_bridge example's simulated DS402 node (can_bridge::SimulatedCanBus, the default, no hardware) or the TWAI peripheral (using CanBus = ...exactly likecan_bridge_example.cpp). Window: node id + Apply (rebuilds bus/client/drive), NMT Start / Stop / Pre-operational / Reset node, NMT state (heartbeat callback) + DS402 state + statusword + mode-display labels, mode Select (PP / PV / PT / Homing), target-velocity Slider + Apply, Enable / Disable / Quick stop / Fault reset, position / velocity labels, an SDO row (index / sub / width / value + Read / Write + result label) and, on the simulated bus, Inject fault (writes 1 to 0x2000). Every bus transaction runs on the app's ownespp::Task(SDO calls block;bus.on_receive -> client.process_framenever does SDO); the widget handlers only queue commands and the task polls statusword / position / velocity / mode every 250 ms and updates the widgets from there. Errors ->notifylevel Error (with the SDO abort code when there is one). Teardown happens in the window'son_close(desktop task, no lock held) so the blocking task join never runs under the model mutex.i2c_scanner_app.hpp) —espp::I2c(new API,auto_init = false+init(ec); a failed init shows a hint label instead of the tools). Scan probes 0x01..0x7F on a one-shot task into a Table (hex / decimal / note), selecting a row fills the address field; addr / register / length / value TextBoxes with Read (read_at_register-> hex dump) and Write (writeof register + bytes).network_app.hpp) — Wi-Fi group (espp::WifiSta, constructed on first launch with the NVS credentialsdesktop/wifi_ssid+wifi_pass, idle when none): status / SSID / IP / RSSI / MAC labels on a 1 s timer, Scan on its own task (label "scanning…", AP list "SSID (RSSI, auth, ch)"), password TextBox (Password flag), Connect (selected AP + password, saved to NVS;reconfigure()with a fresh config so stale SSID / password bytes never leak) / Disconnect / Forget. Ethernet group (own gate,espp::Ethernet::RmiiConfigwith the ESP32-Ethernet-Kit pins): link / IP / MAC / speed labels on the same timer. The interfaces persist across windows (closing the window does not disconnect).Kconfig (
Desktop Example Configuration)DESKTOP_EXAMPLE_ENABLE_CANOPENDESKTOP_EXAMPLE_CANOPEN_BUSSIMULATEDSIMULATED/TWAIDESKTOP_EXAMPLE_CANOPEN_NODE_IDDESKTOP_EXAMPLE_CAN_TX_GPIO/_RX_GPIO/_BAUDRATEDESKTOP_EXAMPLE_ENABLE_I2CDESKTOP_EXAMPLE_I2C_PORT/_SDA_GPIO/_SCL_GPIO/_FREQ_HZDESKTOP_EXAMPLE_ENABLE_WIFISOC_WIFI_SUPPORTEDDESKTOP_EXAMPLE_ENABLE_ETHERNETSOC_EMAC_SUPPORTEDBuild wiring:
canopen twai i2c wifi ethernet cliadded to the example'sEXTRA_COMPONENT_DIRS/COMPONENTSandmainREQUIRES(unconditionally —REQUIREScannot depend on Kconfig; the options only decide which apps are registered).main/CMakeLists.txtaddscomponents/canopen/can_bridge_example/maintoINCLUDE_DIRSfor the simulated bus / node headers (promoting them into thecanopencomponent is a follow-up). READMEs: app list + Kconfig table in the example README (included by the docs), app mention in the component README.Builds (esp32s3, IDF v6.1,
IDF_COMPONENT_MANAGER=0, scratch build dirs)ENABLE_CANOPEN/I2C/WIFI/ETHERNET=n,LOG_CAPTURE=n)CANOPEN_BUS_TWAI=y)-DCONFIG_DESKTOP_EXAMPLE_ENABLE_ETHERNET=1viaEXTRA_CXXFLAGS, since the S3 has no EMAC)No warnings from the new code (
-Werror=missing-field-initializerssatisfied; only pre-existing deprecation / unused warnings inethernet.cpp).cppcheck --check-level=exhaustive --force --enable=all ...on the examplemaindir and on each app header with the config macros defined: clean.Submodules needed for the manager-off build in a fresh worktree:
components/format/detail/fmt external/esp-usb external/tinyusb components/cli/detail/cliandcomponents/littlefs(recursive).Notes
🤖 Generated with Claude Code
https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU