Skip to content

RSDK-14149: MotoPlus firmware flashing (flash_firmware DoCommand + flash_on_start)#69

Open
JohnN193 wants to merge 11 commits into
viam-modules:mainfrom
JohnN193:motoplus-flasher
Open

RSDK-14149: MotoPlus firmware flashing (flash_firmware DoCommand + flash_on_start)#69
JohnN193 wants to merge 11 commits into
viam-modules:mainfrom
JohnN193:motoplus-flasher

Conversation

@JohnN193

@JohnN193 JohnN193 commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

RSDK-14149

What

Adds MotoPlus controller firmware flashing to the module — a Linux replacement for the Windows-only OnlineDownload.exe. Flashing is exposed as a flash_firmware DoCommand and an optional flash_on_start config attribute.

How

  • src/viam/lib/motoplus_flasher.{hpp,cpp}robot::MotoPlusFlasher, a Boost.Asio client for the MotoPlus DevTools line protocol over TCP/12000 (reverse-engineered and capture-verified): DELETENRB <name>, then a two-phase DOWNLOAD <name>,<size> (wait for DOWNLOAD Start, then stream the bytes). Reboot is encoded in the verb. rc=0x2010servos_on, rc=0x3902 → benign "nothing to remove".
  • yaskawa_arm: new config firmware_path / firmware_dest_name / flash_on_start. The flash_firmware DoCommand and the startup path share flash_firmware_(), which quiesces the FSM (disconnect()), deletes + downloads, then reconnects and polls to ready.
  • Servo power must be OFF before flashing; rc=0x2010 is surfaced with that instruction. flash_on_start is unconditional for now (reflashes + reboots every start); a version-gated skip is planned (see below).
  • README: new "Firmware Flashing" section + attribute rows.

Deferred (not in this PR)

Phase 2 = version-gated flashing (only flash when the controller is out of sync): a build_id on MSG_CAPABILITIES, firmware git-describe stamp + sidecar, and flipping flash_on_start to gated. Design is written up but intentionally not included here.

Test plan

  • make build — clean under -Werror -Wconversion -pedantic.
  • make test / motoplus_flasher_test — loopback fake asserts byte-identical DELETE/DOWNLOAD requests, the two-phase handshake, and rc classification. Passing.
  • The identical wire protocol was verified end-to-end against a real YRC1000 (update + fresh install) via the standalone reference tool.

Prerequisites (controller)

Servo power OFF + HOLD released; one-time Maintenance-mode MotoPlus Temp File init + S4C1084=1; firmware ≥ YAS4.12.

🤖 Generated with Claude Code

JohnN193 and others added 9 commits July 1, 2026 15:01
…tart)

Linux replacement for the Windows OnlineDownload.exe. MotoPlusFlasher speaks the
MotoPlus DevTools line protocol over TCP/12000 (DELETENRB, then two-phase DOWNLOAD;
reboot is encoded in the verb), reverse-engineered and capture-verified.

- src/viam/lib/motoplus_flasher.{hpp,cpp}: Boost.Asio flasher client. Two-phase
  DOWNLOAD (wait for 'DOWNLOAD Start' before streaming), rc mapping (0x2010 ->
  servos_on, 0x3902 -> benign 'nothing to remove').
- yaskawa_arm: firmware_path / firmware_dest_name / flash_on_start config; the
  flash_firmware DoCommand and startup path share flash_firmware_(), which quiesces
  the FSM (disconnect), deletes+downloads, then reconnects and polls to 'ready'.
- Servos must be OFF before flashing; rc=0x2010 is surfaced with that guidance.
  flash_on_start is unconditional for now (reflashes+reboots every start); a
  version-gated skip is planned.
- Unit test with a loopback fake: byte-identical DELETE/DOWNLOAD requests, two-phase
  handshake, and rc classification.
- README section + CMake/Makefile test wiring.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…multi-arm docs

- flash_firmware_ no longer blocks on reconnect: after a successful flash it restarts
  the FSM and returns immediately (returns a 'note' instead of reconnected/state), so
  config_mutex_ isn't held for the whole ~90s reboot and do_command can't hit the gRPC
  deadline. The FSM reconnects in the background.
- Bump the download timeout to 120s so a slow controller flash-write can't false-fail.
- README: document that firmware config is controller-scoped (set on one arm per host;
  a flash reboots all arms on that host), and correct the DoCommand blocking/response docs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- classify() takes msgs by const ref (performance-unnecessary-value-param)
- drop the redundant sock.close(ec) on connect failure; the local socket closes via
  RAII (bugprone-unused-return-value on the returned error_code)
- flash_firmware_ 'bytes' is const (misc-const-correctness)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A firmware flash (disconnect -> reboot -> reconnect) surfaced 'cannot push on closed
queue' on every FSM reconnect attempt. TcpRobotSocket can't be reused after disconnect
(its request queue is closed permanently), and establish_connections_ only rebuilt it
when it was still connected -- so a cleanly disconnected socket, or one left disconnected
by a failed attempt, was reused with a closed queue. Always start each attempt with a
fresh socket. Verified by fault_injection_test and controller_integration_test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With servos on, DELETE returns rc=0x2010 but the follow-up DOWNLOAD can return a
different code (observed rc=0x2140), so keying the servos-off guidance only off the
download outcome produced a generic, unhelpful error. Treat rc=0x2010 on DELETE as the
authoritative servos-on signal: return the servo-power-off guidance immediately and skip
the (doomed) 141KB upload. De-duplicated the message; ASCII hyphen instead of em-dash.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Boost.Asio takes an internal service-registry mutex while constructing a tcp::socket;
the analyzer wrongly treats it as held through the later blocking recv/send, flagging a
false positive on the synchronous flasher I/O. The diagnostic's primary location is in a
Boost header, so a NOLINT in our source can't suppress it. Disable the check (it's noisy
with any Asio synchronous socket use).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the 'Phase 2' references in the flash comments with a concrete TODO(RSDK-14150)
for the version-gated flashing follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The mp_online_download.py tool and motoplus-sdk notes were POC scaffolding, not part of
this repo. Keep just the reverse-engineered/capture-verified note against OnlineDownload.exe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread .clang-tidy
-readability-implicit-bool-conversion,
-readability-magic-numbers,
-readability-named-parameter,
-clang-analyzer-unix.BlockInCriticalSection,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-analyzer-unix.BlockInCriticalSection here is a false positive. Boost.Asio grabs an internal scoped_lock while constructing the tcp::socket and releases it at the end of construction; the later blocking recv is not under any lock. The checker — only promoted out of alpha in clang 19 (llvm#93815) — doesn't track locks released via destructor, so it treats that transient construction-time lock as still held and flags the blocking recv. See llvm#104241 and the "not checking destructors" false-positive report.

NOLINT can't suppress it — the diagnostic anchors in a Boost header, not our source — so the check is disabled in .clang-tidy. It only affects synchronous socket I/O, which nothing else in this repo uses (all other socket code is coroutine/async), so disabling it isn't hiding anything real elsewhere.

@@ -0,0 +1,194 @@
#include "motoplus_flasher.hpp"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used wireshark to see what we were sending to the control box and get a rough idea of what success/error cases we could run into.

// connected session, a cleanly disconnected one (e.g. after a firmware flash), or a socket
// left disconnected by a previous failed attempt (otherwise the retry pushes onto a closed
// queue -> "cannot push on closed queue").
if (tcp_socket_) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TcpRobotSocket can't be reused once disconnected (its request queue is closed permanently), so start every attempt with a fresh socket regardless of the prior state: a dead-but-connected session, a cleanly disconnected one (e.g. after a firmware flash), or a socket left disconnected by a previous failed attempt (otherwise the retry pushes onto a closed queue -> "cannot push on closed queue").

Comment on lines +17 to +28
/// Client for the Yaskawa MotoPlus DevTools "online download" protocol used by the Windows
/// OnlineDownload.exe: a plain-text line protocol over TCP (default port 12000). Each call is a
/// self-contained connect / send / read-response / close. Reverse-engineered and capture-verified
/// against the Windows OnlineDownload.exe.
///
/// Preconditions on the controller (else the flash is rejected): servo power OFF + HOLD released,
/// the MotoPlus Temporary File initialized (one-time, maintenance mode), S4C1084=1, firmware
/// >= YAS4.12. This class does not manage those; a failed DELETE/DOWNLOAD with rc=0x2010 surfaces
/// as FlashOutcome::servos_on so the caller can tell the operator to drop servo power.
///
/// Uses a private Boost.Asio io_context internally (the calls are synchronous, blocking admin
/// operations), so it does not share or drive the controller's io_context.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we make our own tcp connection to flash the firmware over to the control box. this does not touch the fsm.

the fsm will remain in a disconnected state while the control box is rebooting. Not sure if its worth it to be more explicit that the reflash might be the cause of the disconnected state?

JohnN193 and others added 2 commits July 9, 2026 12:23
Main now bundles the version-pinned firmware .out in the module tarball
(installed next to the kinematics). Resolve the firmware to flash via a
new resolve_firmware_path_() helper: use firmware_path if configured,
otherwise fall back to the bundled firmware if present. Both callers
(flash_firmware DoCommand and flash_on_start) route through it, so
firmware_path is no longer required to flash.

Update the README firmware-flashing section to document the bundled
firmware as the default and firmware_path as an override.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant