refactor(robot): move fknm/frne C++ extensions under robot/#526
Merged
Conversation
Rename C extensions fknm→_fknm_c and frne→_frne_c (CMakeLists.txt, PyModuleDef names, PyInit_* functions). Add pure-Python facade modules fknm.py and frne.py that try to import the C extensions and provide Python fallbacks for the ETS evaluation functions when C is absent (Pyodide, CI without a C toolchain). Symbolic inputs are detected once in the facade; callers make unconditional calls with no try/except. - fknm.py: Python fallbacks for fkine, jacob0, jacobe, hessian0, hessiane; IK C-wrappers raise RuntimeError when C absent; ET_T raises TypeError to trigger existing fallback in ET.A(); Robot_link_T is a no-op (visualisation only) - frne.py: thin wrapper; init() returns None when C absent so rne() dispatches to rne_python() - ETS.py: removed all try/except blocks from eval/jacob0/jacobe/ hessian0/hessiane; now single-line facade calls with _data= and _n= kwargs for the Python path - DHRobot.py: rne() checks _rne_ob is None before dispatching to C - hessian0/hessiane also check _is_symbolic(J0/Je) not just _is_symbolic(q) - tests/test_fknm_fallback.py: 41 tests; context managers use new=_py_func pattern; timing tests run with C active Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- use a dict to map string name to int - name the parameter axis_type rather than joint_type
…e renames - Drop UserList in favour of MutableSequence; implement the five required abstract methods and delegate to self._data (plain list). - Add _dirties_fknm decorator; marks _fknm_stale on any mutation so the C handle is rebuilt lazily on first use rather than eagerly on every change. - _copy_to_cpp replaces the old _building context-manager pattern. - Add __repr__ and __eq__ to BaseETS (MutableSequence provides neither). - ETS/ETS2 insert() signature now matches MutableSequence: (index, value). append() is inherited and works via insert(len, value). - split() wraps each segment in self.__class__() instead of returning raw slices. - ETS/ETS2 __init__ raises TypeError for unrecognised arg types (was silent). - ET.__axis_to_number: use dict.get(axis, 0) to handle SE3 axis gracefully. - DHRobot: _rne_ob → _frne, _init_rne → _copy_to_cpp, _dynchanged → _frne_stale; lazy rebuild at top of rne() removes the old @_check_rne decorator. - Link/DHLink: _listen_dyn → _dirties_frne; wrapper renamed to match. - Update test_insert in test_ETS.py and test_ETS2.py for new insert API. 657/660 tests pass; 3 pre-existing URDF/xacro failures unrelated to this change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rewrites both C extension glue layers to use nanobind instead of the old
CPython API, eliminating manual reference counting and PyCapsule memory
management.
Key changes in fknm_nb.cpp:
- EigenRef4d (dynamic-stride Ref) for 4×4 matrix inputs (base, tool, Tep):
accepts C-contiguous, F-contiguous, or any-stride numpy arrays without
Python-side layout conversion
- EigenRefJd for 6×n Jacobian inputs: fixes hessian corruption when J0
comes from DHRobot.jacob0() (C-contiguous) vs ETS.jacob0() (F-contiguous)
- dcarray (nb::c_contig) for q vectors; nanobind enforces stride-1 access
- Stride fix: nanobind ndarray strides are in elements, not bytes; corrected
make_2d_F strides from {sizeof(double), n*sizeof(double)} to {1, n}
- ET_update: bool params (nanobind rejects Python bool where int expected)
Key changes in frne_nb.cpp:
- RobotObj wrapper with proper RAII destructor (no PyCapsule null-destructor)
Key changes in fknm.py facade:
- ET_init/ET_update catch TypeError for symbolic (object-dtype) inputs
- _to_q_array: np.ascontiguousarray for q (handles list/tuple/non-contiguous)
- _to_f_array: np.asarray for SE3 → ndarray (Eigen::Ref handles layout)
- _ik_args_se3: SE3 → ndarray conversion only (no contiguity enforcement)
Key change in DHRobot.rne():
- np.ascontiguousarray(q/qd/qdd) before frne loop; trajectory slices from
np.c_[...].T are non-contiguous, causing wrong results with stride-1 C code
All 558 non-GUI tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…import Complete Phase 3 cleanup: delete the legacy CPython-API sources (fknm.cpp, fknm.h, frne.c) superseded by the nanobind port, and relocate what remains to reflect that fknm/frne are robot kinematics/dynamics internals, not general package-level modules: - move fknm.py/frne.py facades from top-level roboticstoolbox/ into robot/ - rename+move the C++/C source dir core/ -> robot/cpp-extensions/ and update CMakeLists.txt paths (never a Python package, so this is build-config only; compiled _fknm_c/_frne_c still install to the top-level roboticstoolbox package) - flesh out robot/cpp-extensions/README.md: synopsis, how the lazy-serialization/@_dirties_fknm/@_dirties_frne pattern works, corrected column-major gotcha, updated file table Moving fknm.py under robot/ made tools/p_servo.py's module-level import of Angle_Axis force robot/__init__.py to run before tools/__init__.py finished initializing, since roboticstoolbox/ loads tools before robot and robot depends on tools at module scope. Fix: defer the import inside angle_axis() (lazy import), and have Dynamics.py/DHRobot.py pull rtb_get_param/rtb_set_param directly from roboticstoolbox.tools.params instead of the top-level package, decoupling from init order. Document the lazy-import workaround as tech debt (proper fix is moving p_servo.py into robot/ itself). 656 tests passing, 0 failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
TEMPORARY, for CI validation only — revert before merging. rtb-data hasn't been republished to PyPI since data files were added to main (publishing now would break existing installs pinned to a version that doesn't expect these changes). Points at the git subdirectory instead so CI can validate against current data without a real release. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…earlier GH-side update-branch merge)
This reverts commit 6dcfc62.
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
fknm.cpp,fknm.h,frne.c), superseded by the nanobind portfknm.py/frne.pyfacades from top-levelroboticstoolbox/intorobot/— they're robot-kinematics/dynamics internals, not general package-level modulescore/→robot/cpp-extensions/, updateCMakeLists.txtpaths (build-config only — never a Python package; compiled_fknm_c/_frne_cstill install to the top-levelroboticstoolboxpackage)fknm.pyunderrobot/madetools/p_servo.py's module-level import forcerobot/__init__.pyto run beforetools/__init__.pyfinished. Fixed by deferring the import insideangle_axis(), and havingDynamics.py/DHRobot.pypullrtb_get_param/rtb_set_paramdirectly fromroboticstoolbox.tools.paramsinstead of the top-level packagerobot/cpp-extensions/README.md: synopsis, lazy-serialization/@_dirties_fknm/@_dirties_frnepattern, corrected column-major note, updated file tabletech-debt.md(proper fix: movep_servo.pyintorobot/itself)Test plan
pip install -e . --no-build-isolationrebuilds cleanly against the newCMakeLists.txtpaths🤖 Generated with Claude Code