feat(swift): support teach() teach panel for Swift-backed robots - #621
Open
petercorke wants to merge 2 commits into
Open
feat(swift): support teach() teach panel for Swift-backed robots#621petercorke wants to merge 2 commits into
petercorke wants to merge 2 commits into
Conversation
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 #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.
Follow-up to this same PR's teach panel: swift-sim renamed desc to label on all SwiftElement subclasses (jhavl/swift#135) while this was in flight -- desc still works via a deprecation shim, but update to the new name rather than carry the warning forward.
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.
Summary
robot.teach()only ever worked for PyPlot/PyPlot2 -- Swift's connector wrapper explicitly setsupports_teach=False, so anyhasgeometry=Truerobot (the default backend Swift resolves to, matchingplot()) raised a cleanTypeError.examples/teach_swift.pywas RTB's own hand-rolled template for what a Swift teach panel looks like, but predates Swift 2.0'sAssemblyHandlerefactor and still drove the robot via the now-deprecatedrobot.q[j] = valuedirect-mutation style.What's implemented
Swift._add_teach_panel()using the current idiomatic pattern (named sliders ->env.values-> one per-stephandle.callback, seeexamples/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 compactLabels, matching PyPlot's own sixfig.text()calls).Dependency on companion swift PRs -- CI will be red until these merge
This needs
swift-simwith:Label(compact=True), used by the pose readoutrun()handling, needed so closing the browser tab mid-teach ends the panel cleanly instead of an uncaughtTimeoutErrorRTB's CI already installs swift from
git+https://github.com/jhavl/swift.git@future(not PyPI, seeci.yml), so this will resolve on its own once both merge -- no CI config change needed here.Also fixed, both blocking without them
teach()'senv.launch("Teach " + self.name, limits=limits)passed the name positionally, landing on Swift's reallaunch(realtime=..., ...)rather than the PyPlotlaunch(name=..., ...)it was written against -- raisesValueErrorimmediately. Fixed toname=as a keyword.hold()(whatteach()'s sharedif block: env.hold()relies on to keep the panel open) only sleeps and polls for a disconnect -- it never callsstep(), so nothing would ever process a dragged slider._add_teach_panel()now runs its ownenv.run()loop instead whenblock=True, matching every other interactive Swift script's own step loop, and signals back toteach()so it skips the now-redundantenv.hold()call. That call wasn't just wasteful to leave in -- it could hang indefinitely (headless mode'shold()never reports "disconnected" at all; even non-headless there's a race aroundclose()andsocket.USERS), which is what the two swift PRs above close off.Other things worth knowing
teach()'s docstring now documents thatrobot.qholds the final taught pose onceteach()returns, true for every backend. PyPlot achieves this by mutatingrobot.qthroughout its own session; Swift's_add_teach_panel()writeshandle.qback once, at the point the session ends, sinceAssemblyHandledeliberately never mirrors it during the session itself (Robot/Shape "instance handle" redesign (animation-loop API) 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 ownteach()already has (desiderata.md), just accepted here rather than solved.rtb.models.Panda().teach()with no explicitbackend=now opens Swift by default (hasgeometry=Trueresolves there, matchingplot()'s existing default) rather than always falling back to PyPlot, since Swift now actually supports it -- a real, intentional behaviour change, not incidental.Test plan
tests/test_backend_capabilities.py:TestSwiftCapabilities'ssupports_teachassertions flippedFalse->Truetests/test_BaseRobot.py: 3 new tests -- explicitbackend="swift"smoke test, default-backend-resolves-to-swift regression test, and a test confirmingrobot.qis written back after the session ends (withSwift.runmocked to a no-op, sinceblock=True's real loop only exits on a real disconnect)test_BaseRobot.py+test_backend_capabilities.py)robot.qreflects the final pose afterward