improvement!: match bb's synchronous set_position changes - #86
Merged
Conversation
…ptions
Requires bb 0.30.
`BB.IK.DLS.Motion` delegates every function straight to `BB.Motion`, so
its `@spec`s and `@doc` examples only ever describe what bb returns — and bb
no longer returns what they described.
`move_to_multi/3` and `solve_multi/3` documented `{:error, failed_link,
reason, results}`. A solve failure is now a single
`%BB.Error.Kinematics.MultiFailed{}` carrying the failed link, the underlying
error and the results that preceded it, and an actuator's refusal arrives
unwrapped, because nothing about it is kinematic. `move_to/4` and `solve/4`
were declared `{:error, atom(), meta()}`, which bb has not returned for
several versions; dialyzer says nothing about a spec that is merely too
narrow, which is how it survived.
The four result types are now aliases for `BB.Motion`'s own rather than
restatements of them, so they cannot drift again independently.
`:delivery` documented a `:sync` mode that no longer exists — `:pubsub` is
synchronous now — and gains `:timeout`, which is the only way to bound a
wait that has become blocking. `BB.IK.DLS.Tracker` takes and forwards it
too: it runs a periodic loop, so a `:pubsub` command that outlives its
timeout takes the tracker down with it.
The floor moves to `~> 0.30` in this commit rather than separately, because
nothing here fails loudly against an older bb. `set_position/4` existed in
0.28 and 0.29 as a publish that discards options it does not recognise, so
`delivery: :direct` and `:timeout` would compile clean and silently take the
wrong transport, or silently wait forever. A removed function fails at
compile time; an ignored option fails in the workshop.
`Tracker.stop/1` documents that it returns the positions tracking arrived at,
and it read them with `BB.Robot.State.get_all_configurations/1`. That was a
fair proxy while `BB.Motion` wrote each solve into the robot's state. It no
longer does — state is written from `BB.Message.Sensor.JointState` messages
and from nothing else — so on a robot without position feedback `stop/1`
hands back the configuration the robot *booted* in, and on one with feedback
it hands back a mid-flight measurement of every joint rather than the
tracker's own answer.
The tracker now solves and sends as two steps, `BB.Motion.solve_only/4` then
`BB.Motion.send_positions/3`, and keeps what the solver returned.
`BB.Motion.move_to/4` cannot serve here: it does both in one call but does
not hand back the positions it solved.
Three things follow from the split. The per-tick telemetry span becomes
`[:bb, :motion, :solve]` plus `[:bb, :motion, :send_positions]` rather than
`[:bb, :motion, :move_to]`. An actuator's refusal no longer overwrites
`last_meta` with a synthesised `%{residual: nil, iterations: 0}` — the solve
succeeded, so its real meta is kept and the refusal is logged instead; that
path is only reachable under `:pubsub`, since `:direct` always answers `:ok`.
And the cached `BB.Robot.State` handle leaves the tracker's state, having
only ever been there to be read.
The tracker had no test coverage at all, so this adds the mock actuator and
the tracker test the sibling solver package already carries, including one
that fails against the old bookkeeping.
Every solve seeds from the robot's current configuration, and bb now writes that from `BB.Message.Sensor.JointState` messages and from nothing else. A tracker pointed at joints nothing reports on re-solves from the same frozen pose on every tick. It does not diverge — a solve is a function of its seed and its target, so a frozen seed still yields an absolute joint configuration that reaches the target. What it loses is the warm start: more iterations per tick, worse convergence near singularities, and, since `:step_size` caps how far the configuration moves per iteration, `:max_iterations` bounds how far the answer can travel from its seed at all. The one that bites is that the answer stops depending on the path taken to get there, which leaves the arm free to change solution branch between ticks — a redundant arm can be asked to swing between two equally valid postures inside a single tick period. The fix belongs to the robot rather than to this package — an encoder, a driver declaring `:position_feedback`, or `BB.Sensor.OpenLoopPositionEstimator` — and bb warns at compile time about a driven joint with none of the three, so it cannot go unnoticed. Keeping a private warm-start cache here instead would quietly reinstate the dead reckoning bb just removed, once per solver package. `solve_and_update/6` gains the matching caveat: writing a solved configuration into a *running* robot's state claims its joints have arrived somewhere they have only been asked to go. It remains right for a state of your own, stepped through a planned motion. The tracker's test robot now declares an estimator per joint, as a real open-loop arm would, rather than tripping the new compile-time warning. The README's motion and tracker examples were also missing the required `:source_link`, so they could not have run as written.
jimsynz
force-pushed
the
sync-set-position
branch
from
August 21, 2026 23:09
be46d27 to
7924f22
Compare
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.
Matches
bb_ik_dlsto the breaking changes released in bb 0.30.0(beam-bots/bb#235). Three separate things needed doing, in three commits.
The
bbfloor moves to~> 0.30, in the first commit rather than separately.That is load-bearing, not housekeeping:
set_position/4existed in 0.28 and0.29 as a publish that discards options it does not recognise, so the
delivery: :directand:timeoutthis PR documents and forwards would compileclean against an older bb and silently take the wrong transport, or silently
wait forever. A removed function fails at compile time; an ignored option fails
in the workshop.
1. Types and docs that bb's new return shapes made wrong
BB.IK.DLS.Motionis a thin wrapper: every one of its functions delegatesstraight to
BB.Motion, so its@specs and@docexamples only ever describewhatever bb returns. They described something bb no longer returns:
move_to_multi/3andsolve_multi/3documented{:error, failed_link, reason, results}. A solve failure is now{:error, %BB.Error.Kinematics.MultiFailed{failed_link:, error:, partial_results:}},and an actuator refusal comes back unwrapped as
{:error, error}— notwrapped in
MultiFailed, because nothing about it is kinematic.move_to/4andsolve/4were declared{:ok, meta()} | {:error, atom(), meta()}. bb returns{:error, error}with aBB.Errorstruct. Dialyzercan't catch a spec that is merely too narrow, which is why this survived.
:deliverydocumented a:syncmode that no longer exists —:pubsubissynchronous now.
The four result types are now aliases for
BB.Motion's own(
Motion.motion_result()and friends) rather than restatements, so they can'tdrift again on their own.
:timeoutis documented and passed through, sinceunder the now-blocking
:pubsubpath it is the only way to bound the wait.2.
Tracker.stop/1returned the wrong thingNot a doc fix — a real regression. The tracker recorded its
last_positionswith
BB.Robot.State.get_all_configurations/1. That used to be a reasonableproxy for "what the tracker last commanded", because
BB.Motionwrote thesolved positions into the state. It no longer does, so on a robot without
position feedback
stop/1returns the configuration the robot started in,and on a robot with feedback it returns a mid-flight measurement of every joint
rather than the tracker's own answer.
The tracker now solves and sends as two steps —
BB.Motion.solve_only/4thenBB.Motion.send_positions/3— and keeps the configuration the solver returned.BB.Motion.move_to/4can't be used for this because it doesn't hand back thepositions it solved. Consequences worth knowing:
[:bb, :motion, :move_to]to[:bb, :motion, :solve]plus[:bb, :motion, :send_positions].last_metawith a synthesised%{residual: nil, iterations: 0}. The solve succeeded; the refusal is loggedand the solve's real meta is kept. (Only reachable under
:pubsub—:directalways answers:ok.)BB.Robot.Statehandle is gone from the tracker's state; it wasonly there to be read.
This tracker had no test coverage at all, so the commit also adds the mock
actuator and tracker test the sibling FABRIK package already carries. The
stop/1test fails against the old bookkeeping — verified by reverting justthat expression and re-running.
3. Position feedback is now a documented prerequisite for tracking
Every solve seeds from the robot's current configuration, and bb now writes
that from
BB.Message.Sensor.JointStatemessages and from nothing else. Atracker pointed at joints that nothing reports on re-solves from the same
frozen pose on every tick.
I looked at whether that needs a code change here and concluded it doesn't. It
doesn't diverge or oscillate — a solve is a function of its seed and its
target, so a frozen seed still yields an absolute joint configuration that
reaches the target. What it loses is the warm start: more iterations per tick,
worse convergence near singularities, and — the one that actually bites —
freedom to change solution branch between ticks, because the answer stops
depending on the path taken to get there. A redundant arm can be asked to swing
between two equally valid postures inside one tick period. DLS has one extra
wrinkle:
:step_sizecaps how far the configuration moves per iteration, so:max_iterationsbounds how far the answer can travel from its seed at all,which a warm start would never come near.
I measured the branch-change effect on FABRIK rather than DLS (a three-revolute
planar arm, 0.25 m + 0.25 m + 0.15 m, following a 0.2 m circle straddling its
own base over 61 ticks: a largest single-tick joint step of 5.25 rad frozen
against 1.73 rad warm-started). It is quoted here rather than in the moduledoc
because it belongs to the arm and the solver it was taken on, and would rot the
moment either solver's defaults changed. The moduledoc describes the mechanism
instead.
The fix belongs to the robot, not to this package: an encoder, a driver
declaring
:position_feedbackthroughc:BB.Actuator.capabilities/1, orBB.Sensor.OpenLoopPositionEstimator. bb's newBB.Dsl.Verifiers.ValidatePositionFeedbackwarns at compile time about adriven joint with none of the three, so this can't go unnoticed the way it
would have before — which is what settled it for me. Re-adding a private
warm-start cache in each solver package would have quietly re-introduced the
dead reckoning bb just deliberately removed, in two places, divergently.
The prerequisite is documented in
BB.IK.DLS.Tracker's moduledoc,usage-rules.mdandAGENTS.md.solve_and_update/6gained the matchingcaveat: it's right for a state of your own from
BB.Robot.State.new/1, andwrong for a running robot's.
The README's motion and tracker examples were also missing the required
:source_linkand so could not have run as written; fixed while correctingtheir return shapes.
Verification
mix check --no-retryagainst hexbb 0.30.0— noBB_VERSION, exactly whatCI does — is green on all nine tools, from a clean baseline on
main.mix.lockpinsbb 0.30.0and nothing else.mix deps.update bbalso wantedto carry
localize,nxandreqalong; bb 0.30.0 asks only forlocalize ~> 1.0andnx ~> 0.10, both of which the existing pins already satisfy, andreqis not a bb dependency at all, so those three are restored to theirprevious versions and left for a dep sweep.