runners: uf2: accept and warn on --dev-id

[YumTaha]

Fixes #107128

The nRF Connect VS Code extension unconditionally appends -i <serial> to west flash for any detected Nordic device, regardless of which runner is selected. UF2BinaryRunner.capabilities() did not declare dev_id=True, causing the base-class guard in ZephyrBinaryRunner.create() to abort with a fatal error before any flashing occurred.

UF2 flashing is connectionless — the runner identifies the target drive via INFO_UF2.TXT (--board-id) and copies a file. do_create() only reads args.board_id. Declare dev_id=True so the argument is accepted, and emit a warning so users know it has no effect on target selection.

Changes

• scripts/west_commands/runners/uf2.py
  • capabilities(): add dev_id=True to RunnerCaps
  • __init__(): add dev_id parameter
  • do_create(): pass dev_id=args.dev_id through
  • do_run(): warn when --dev-id is provided and ignored

Testing

Tested on Windows 11, Seeed XIAO nRF52840, nRF Connect SDK v3.2.4, nRF Connect VS Code extension. west flash -i F3ADA8E6C5AA9120 previously aborted immediately; now completes with the expected warning:

WARNING: runners.uf2: --dev-id F3ADA8E6C5AA9120 is not used by
         the UF2 runner and will be ignored

[github-actions]

Hello @YumTaha, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

[marc-hb]

Great analysis and excellent commit message, thanks!

As the two bugs seem completely unrelated to each other, would you mind splitting this into two different commits? Single PR; it's small enough.

The nRF Connect VS Code extension unconditionally appends -i/--dev-id
to west flash for any detected Nordic device, regardless of runner.
UF2BinaryRunner.capabilities() did not declare dev_id=True, causing
the base-class guard to abort before any flashing occurred. [...]
Declare dev_id=True so the argument is accepted and silently ignored.

Silently discarding inputs is often puzzling and time-consuming and TBH this sounds like a (harmless) nRF connect bug. Is there a good place where you could add something like this?

if args.id: # see bug http:/.../107128
  self.logger.warning("Discarding spurious --dev-id {x}")

Or at least a self.logger.info()

[YumTaha]

Sure, splitting into two commits and adding a warning log for the discarded --dev-id; will update the PR shortly.

[pdgendt]

The nRF Connect VS Code extension unconditionally appends -i <serial> to
west flash for any detected Nordic device, regardless of which runner is
selected.

@carlescufi could other runners be impacted as well?

[marc-hb]

@carlescufi could other runners be impacted as well?

If a significant number of other runners are affected, then it would be a tedious and unreliable game of whack-a-mole to add a workaround in all of them - as opposed to making the VS Code extension smarter?

Or should Zephyr just give up and make it mandatory for all runners to accept (and possibly: ignore) --dev-id? Basically getting rid of the RunnerCaps(dev_id)... it is a very basic field after all.

[YumTaha]

Removing the RunnerCaps(dev_id) guard entirely seems cleaner than patching each runner individually, happy to update the PR in that direction if that's the preferred approach.

[marc-hb]

Up to you @YumTaha but... if you submit the unrelated copyfile() fix in a new, separate PR, now I bet it will be merged much faster?

EDIT:

• uf2 runner: shutil.copy() crashes with WinError 433 on Windows after successful flash #107165
• runners: uf2: use copyfile() to avoid copymode() crash on Windows #107166

[YumTaha]

Alright, I will make a new PR for the copyfile() bug, I will keep the --dev-id issue here for discussing until a decision is met.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[carlescufi]

Self-assigning this, will take a look

[kylebonnici]

I am not convinced this is the correct solution, The value for --dev-id F3ADA8E6C5AA9120 is generated from the selected device

Also the extension will have expectations that all the checked devices are flashed

IMO not using the value set in --dev-id will lead to some issues down the line. The nRF VS Code extension will operate with assumptions that the action was done to specific--dev-id and if the action is not done to the same device then unexpected behavior will follow.

also the nRF Connect may add other flags such as --erase not sure is this I also handled

[kylebonnici]

@carlescufi could other runners be impacted as well?

If a significant number of other runners are affected, then it would be a tedious and unreliable game of whack-a-mole to add a workaround in all of them - as opposed to making the VS Code extension smarter?

Or should Zephyr just give up and make it mandatory for all runners to accept (and possibly: ignore) --dev-id? Basically getting rid of the RunnerCaps(dev_id)... it is a very basic field after all.

Just looked at runners.yaml and as far as I can see it does not contain any information of the capabilities. If tools such the VS Code extension need to made smarted, the it need a way to query the runners capabilities to determine that dev_id is not supported

[marc-hb]

If tools such the VS Code extension need to made smarted, the it need a way to query the runners capabilities to determine that dev_id is not supported.

That's not what I meant when I wrote "smarter", sorry about the confusion. I meant: "better educated" and knowledgeable about which hardware types support multiple instances versus not.

But maybe the real issue is this, sorry I missed it until now:

UF2 flashing is "connectionless" — the runner identifies the target drive via INFO_UF2.TXT (--board-id) and copies a file.

I feel like higher-level users like VS Code should not care about such low-level implementation details. In other words, UF2 should accept --dev-id and internally convert it to whatever identification technique and details are required.

[kylebonnici]

I feel like higher-level users like VS Code should not care about such low-level implementation details. In other words, UF2 should accept --dev-id and internally convert it to whatever identification technique and details are required.

I agree fully with this!

[mbolivar]

I think this should not be merged as-is but won't block.

In my opinion, the "device ID" in this board context seems to be easy to handle: take the name of the partition you want to flash to, or something like that.

--dev-id is a runner capability meaning "I can take an additional argument that lets you pick one of multiple boards that are currently attached to the host for me to work on".

Looking at the source code for this runner, it seems like partitions[0] is the hard-coded assumption there, and the runner could instead check to see if the --dev-id argument is somehow "in the list" and use that instead of defaulting to whatever gets found first, no?