Skip to content

fixed RobotBase teach update of vellipse and fellipse - #594

Open
jbkahrs wants to merge 2 commits into
petercorke:mainfrom
jbkahrs:fix-teach-ellipse-update
Open

fixed RobotBase teach update of vellipse and fellipse#594
jbkahrs wants to merge 2 commits into
petercorke:mainfrom
jbkahrs:fix-teach-ellipse-update

Conversation

@jbkahrs

@jbkahrs jbkahrs commented Aug 13, 2026

Copy link
Copy Markdown

Problem

When robot.teach(..., vellipse=True) is used, moving a joint slider updates the robot visualization but leaves the velocity ellipse at its initial configuration.

The teach-panel callback already attempts to update a stored velocity ellipse, but the ellipse created by teach() is not assigned to that backend reference and the callback cannot update the ellipse when the joint sliders change.

Changes

  • Store the velocity ellipse created by teach() as the teach-panel ellipse.
  • Store the force ellipse similarly when fellipse=True.

Rationale

The plotting backend already contains the update mechanism for teach-panel ellipses. Assigning the objects when they are created connects that mechanism to the ellipses displayed by teach().

Related issues

No related issue found.

@petercorke

Copy link
Copy Markdown
Owner

Thanks for tracking down the teach-panel ellipse update bug — the fix for wiring up env._teach_vellipse/env._teach_fellipse makes sense.

One thing I noticed in the diff: the vellipse() call also drops scale=0.5:

vell = self.vellipse(q, centre="ee", scale=0.5, add=False)

vell = self.vellipse(q, centre="ee", add=False)

vellipse()'s default scale is 0.1, so this shrinks the ellipsoid drawn in teach() by 5x. Was that intentional (e.g. 0.5 looked too large once updates were actually working), or a stray edit that slipped in while making the other change? Happy to merge either way, just want to make sure it's not accidental.

@petercorke
petercorke changed the base branch from future to main August 16, 2026 23:50
petercorke added a commit that referenced this pull request Aug 17, 2026
future was 162 commits behind main, 0 ahead, last touched 2026-06-14 --
a leftover from before RTB used a proper PR-based workflow, no longer
part of how anything actually merges (confirmed by MVTB and bdsim,
Peter's other actively-maintained repos, which have already dropped
their own future branches entirely -- RTB was the outlier still
carrying it).

Third-party repos (swift, spatialgeometry) still use their own future
branch as the live PR-merge target, set by their own owners -- not
touched here, unrelated to RTB's now-dead one.

- ci.yml: drop future from push/pull_request trigger branches
- pull_request_template.md: stop telling contributors to target future
- README.md: fix a stray image URL pointing at future/docs/figs/... --
  would have 404'd once the branch was actually deleted

PR #594 (open, targeting future) retargeted to main separately before
this merges, so it isn't orphaned by the branch deletion.
@petercorke
petercorke force-pushed the fix-teach-ellipse-update branch from 534fda6 to 3a5f05d Compare August 17, 2026 11:54
@jbkahrs

jbkahrs commented Aug 19, 2026

Copy link
Copy Markdown
Author

Indeed, dropped the scale as it appeared to big in many cases (e.g. Panda or Puma560).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants