robot.move() in the fleet view drove every unassigned board at once#1
Merged
Conversation
Each robot the IDE exposed was keyed by TOPIC id. Every fresh board answers to `unassigned` and publishes to robots/unassigned/*, so the whole pool collapsed into one entry — and a robot.move() published to robots/unassigned/pwm with no target, which every unnamed board on the desk obeys. Day one of a class, IDE open, is exactly when boards are unnamed. A board has two ids: the topic id (its name, or `unassigned`) that the topic is built from, and the stable MAC-derived board id in sys.board (rover-b79c) that the firmware matches `target` against (rover_role.c compares target to s_id, the board field, not the topic). The topic GROUPS boards; target DISAMBIGUATES within a group. So in the fleet view (instructor/anonymous, robots/+/#) the handle is now the board id, sourced from the sys beacon, and every command carries target — the topic reaches the pool, target picks the one. A team login is untouched: it is scoped to one board on its own topic, so the topic id is already a unique handle and no target is needed. Non-sys channels (imu) on a shared pool topic carry no board id and are genuinely ambiguous — attributed only when one board owns the topic, dropped otherwise, rather than smeared across the pool. Headless-tested (mocked mqtt): team publishes to its topic with no target; two unassigned boards get distinct board-id handles and move() targets exactly one; imu attributes to a sole-owner topic and is dropped on the shared one. NOT tested through Blockly + MicroPython + real boards — that wants the bench. The student-facing id string is now the board id in fleet mode; blocks.js binds no ids and renderTelemetry only displays it, so the string is opaque end to end, but the multi-board flow should be exercised before this ships to a class. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Fixes a fleet-view safety issue where multiple unnamed boards sharing the robots/unassigned/* topic could be merged into one logical robot, causing robot.move() to publish an untargeted command that every unassigned board would obey. The update refactors the client-side identity model so fleet mode keys robots by stable sys.board and stamps target on commands when operating on a shared topic.
Changes:
- Add explicit “topic id vs board id” model and switch fleet view addressing to use
sys.boardas the handle. - In fleet mode, publish commands to the topic while adding
targetto disambiguate the intended board. - Avoid smearing ambiguous non-
systelemetry across multiple boards on the same shared topic by dropping it unless a sole owner can be determined.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, timeoutMs); | ||
| pendingLed.set(id, { resolve, timer }); | ||
| publish(id, "led", { method: "set_led", on, red, green, blue }); | ||
| pendingLed.set(replyKey, { resolve, timer }); |
Comment on lines
+86
to
+88
| const here = [...topicOf].filter(([, t]) => t === topicId).map(([h]) => h); | ||
| if (here.length !== 1) return; | ||
| handle = here[0]; |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The last of the day's fixes, and the one that isn't mechanical — chosen deliberately over the alternatives (firmware-refuses-untargeted, or document-and-leave).
The bug
Each robot the IDE exposed was keyed by topic id. Every fresh board answers to
unassignedand publishes torobots/unassigned/*, so the whole pool collapsed into one entry — androbot.move()published torobots/unassigned/pwmwith notarget, which every unnamed board on the desk obeys. Day one of a class, IDE open, is exactly when boards are unnamed.Why it needed more than a one-liner
A board has two ids:
unassigned) thatrobots/<id>/<channel>is built from;sys.board(rover-b79c) that the firmware matchestargetagainst —rover_role.ccomparestargettos_id, the board field, not the topic.The topic groups boards;
targetdisambiguates within a group. So "just send target" isn't enough — the IDE couldn't produce a per-board target because its data model had already merged the pool.The fix
robots/+/#): the handle is now the board id, sourced from the sys beacon, and every command carriestarget— the topic reaches the pool,targetpicks the one.targetis needed. This path is byte-for-byte the old behaviour.Testing
Headless (mocked mqtt), all passing: team publishes to its topic with no target; two unassigned boards get distinct board-id handles and
move()targets exactly one on the shared topic; imu attributes to a sole-owner topic and drops on the shared one.Not exercised through Blockly → MicroPython → real boards.
blocks.jsbinds no ids andrenderTelemetryonly displays the id, so the id string is opaque end to end — but the multi-board flow should be run at the bench before this ships to a class. Holding this PR for that, unlike the day's others.🤖 Generated with Claude Code