Skip to content

refactor(ik): joint limit helpers as module-level functions - #713

Open
petercorke wants to merge 1 commit into
mainfrom
refactor/ik-joint-limit-helpers
Open

petercorke wants to merge 1 commit into
mainfrom
refactor/ik-joint-limit-helpers

Conversation

@petercorke

Copy link
Copy Markdown
Owner

Summary

  • Move the joint limit normalisation and check out of IKSolver into module-level normalise_q(q, qlim, revolute) and within_limits(q, qlim) in roboticstoolbox/robot/IK.py. They take plain arrays instead of an ETS.
  • IKSolver._normalise_q and _check_jl become thin wrappers that map the jindex-indexed coordinates onto the ETS joints and call the helpers.
  • Purpose: let other IK implementations, e.g. the analytic IK being developed for DHRobot (which has qlim and joint types but no ETS), share the numerical solvers' coordinate convention (keep in-limit values, otherwise principal angle shifted by whole turns toward the limits) instead of duplicating it.

Behaviour

Unchanged, including that a NaN coordinate is not reported as outside its limits (same q < lo or q > hi semantics as before). No public API change: the new functions are not exported from the package.

Test plan

  • tests/test_IK.py and tests/test_ik_joint_limits.py pass unchanged (121 passed, 12 skipped)
  • New direct tests of the helpers: in-limit values preserved, shift into limits, principal angle preferred, input not modified, prismatic untouched, no-equivalent-angle rejected, exact limit endpoint after whole turns, inclusive limit check
  • Differential test, old methods from main vs the new wrappers, 7200 random cases over 6 jindex layouts (default, identity, reversed, permuted, sparse) and 3 limit sets: identical results
  • CI

🤖 Generated with Claude Code

Move the joint limit normalisation and check out of IKSolver into
normalise_q() and within_limits(), which take plain arrays (q, qlim, a
revolute mask) instead of an ETS.  IKSolver._normalise_q and _check_jl
become thin wrappers that map jindex-indexed coordinates onto the joints.

Behaviour is unchanged, including that NaN is not reported as outside its
limits.  This lets other IK implementations, for example analytic IK on a
DHRobot, share the numerical solvers' coordinate convention instead of
duplicating it.

Add direct tests of the helpers.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 7 high

Alerts:
⚠ 7 issues (≤ 0 issues of at least minor severity)

Results:
7 new issues

Category Results
Security 7 high

View in Codacy

🟢 Metrics 8 complexity · 0 duplication

Metric Results
Complexity 8
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@codecov

codecov Bot commented Oct 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (acc6842) to head (a0478ef).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/roboticstoolbox/robot/IK.py 0.00% 32 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #713   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        143     143           
  Lines      14269   14275    +6     
=====================================
- Misses     14269   14275    +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
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.

1 participant