fix: run() lets a disconnect-during-step() crash out as a raw traceback - #132
Merged
petercorke merged 1 commit intoAug 21, 2026
Merged
Conversation
A disconnect noticed *during* step() (SwiftRoute.py's expect_message()/ producer racing wait_closed(), jhavl#130) raises TimeoutError directly out of step() -- run()'s own loop only ever expected a disconnect to surface via _check_disconnected()'s poll *between* steps, so this propagated straight out of run() as an uncaught exception instead of the same graceful "Swift browser tab closed." message/return every other disconnect path already gets. Not a hypothetical: closing the browser tab mid-session (e.g. an interactive robot.teach(backend="swift") built on run()) reliably hits this, every time, now that jhavl#130 made disconnect detection fast enough to land mid-step almost always rather than rarely.
4 tasks
petercorke
added a commit
to jbkahrs/robotics-toolbox-python-csc376
that referenced
this pull request
Aug 22, 2026
robot.teach() only ever worked for PyPlot/PyPlot2 -- Swift's connector
wrapper explicitly set supports_teach=False, so any hasgeometry=True
robot (the default backend Swift resolves to, matching plot()) raised
a clean TypeError. examples/teach_swift.py was RTB's own hand-rolled
template for what a Swift teach panel looks like, but predates Swift
2.0's AssemblyHandle refactor and still drove the robot via the now-
deprecated robot.q[j] = value direct-mutation style.
Implements Swift._add_teach_panel() using the current idiomatic
pattern (named sliders -> env.values -> one per-step handle.callback,
see examples/panda_ik_sliders.py): one slider per joint (degrees for
revolute, native units for prismatic -- teach_swift.py's always-
degrees conversion was a bug, not carried forward), plus a live
end-effector pose readout (6 compact Labels, matching PyPlot's own six
fig.text() calls -- needs swift-sim with Label(compact=True), jhavl/
swift#131).
Also fixes two things this surfaced, both blocking without it:
- teach()'s env.launch("Teach " + self.name, limits=limits) passed the
name positionally, which lands on Swift's real launch(realtime=...,
...) rather than PyPlot's launch(name=..., ...) it was written
against -- raises ValueError immediately. Fixed to name= as a keyword.
- Swift's hold() (what teach()'s shared `if block: env.hold()` relies
on to keep the panel open) only sleeps and polls for a disconnect --
it never calls step(), so nothing would ever process a dragged
slider. _add_teach_panel() now runs its own env.run() loop instead
when block=True, matching every other interactive Swift script's own
step() loop, and signals back to teach() so it skips the now-
redundant (and occasionally hang-prone -- see below) env.hold() call.
Needs swift-sim with the disconnect-during-step fix (jhavl/swift#132)
for a closed tab to end that loop gracefully rather than an uncaught
TimeoutError; needs petercorke#131 too, since headless mode's hold() never
reports "disconnected" at all -- teach()'s subsequent env.hold() call
would otherwise hang indefinitely, not just waste time.
teach()'s docstring now also documents that robot.q holds the final
taught pose once teach() returns, true for every backend -- PyPlot
achieves this by mutating robot.q throughout its own session; Swift's
_add_teach_panel() writes handle.q back once, at the point the session
ends, since AssemblyHandle deliberately never mirrors it during the
session itself (jhavl/swift#85). A one-time, deliberate exception to
"stateless robot model" for this specific single-owner interactive
session, not a general precedent -- the same tension PyPlot's own
teach() already has (desiderata.md), just accepted here rather than
solved.
Also: rtb.models.Panda().teach() with no explicit backend= now opens
Swift by default (hasgeometry=True resolves there, matching plot()'s
existing default) rather than always falling back to PyPlot, since
Swift now actually supports it -- a real, intentional behaviour change.
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
1 participant
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.
Summary
A disconnect noticed during
step()(SwiftRoute.py'sexpect_message()/producer racingwait_closed(), #130) raisesTimeoutErrordirectly out ofstep()--run()'s own loop only ever expected a disconnect to surface via_check_disconnected()'s poll between steps, so this propagated straight out ofrun()as an uncaught exception instead of the same graceful"Swift browser tab closed."message/return every other disconnect path already gets.Not a hypothetical -- closing the browser tab mid-session reliably hits this, every time, now that #130 made disconnect detection fast enough (milliseconds, not 15s) to land mid-step almost always rather than rarely. Found while building an interactive
robot.teach(backend="swift")panel for roboticstoolbox-python (built onrun()) -- see companion RTB work.Fix
Catch
TimeoutErroraround the loop'sstep()call inrun(), same as the existingexpiredbranch: print"Swift browser tab closed.",close(), return -- instead of letting it propagate.Test plan
test_run_exits_quietly_on_disconnect_during_step: mocksstep()to raiseTimeoutError, assertsrun()returns normally (not raises) and prints the graceful messageSwift.pyfix (keeping the test) and re-ran -- fails with the rawTimeoutErrorpropagating, as expectedspatialgeometryPolyline gap from a stale local install, not this change)