Conversation
|
Is there a way to add scripts in tests/ to demonstrate that this new code is working? |
|
|
Planner 0/1 are unaffected, the TP works in Cartesian space and the RT module's kinematicsInverse/kinematicsForward symbols are resolved at load time as usual. Any user-written kinematics module works fine. Planner 2 has a problem: it needs a userspace reimplementation of each kinematics module's math (for Jacobian computation, joint-space limit enforcement, path sampling). Currently kinematics_params.h enumerates known modules, and kinematicsUserInit() hard-fails for anything not in the list, aborting trajectory init entirely. Three options to preserve compatibility with custom kinematics in Planner 2: Fallback to identity kins in userspace, Treat unknown modules as trivkins for the userspace layer. RT still uses the real module. Joint limit enforcement would be approximate but conservative. Downgrade to planner 0/1, If userspace kins init fails for an unknown module, automatically fall back to planner 1 (or 0) with a warning. Simplest fix, preserves "any kins works with any TP". Generic RT-userspace bridge, Add a KINS_TYPE_GENERIC that calls the RT module's forward/inverse via shared memory. Correct but slower, and requires a new communication channel. Which approach would you prefer? |
The new code is not really done yet, I'll work on tests after things are stable enough, G64 is still not implemented, rigid tapping is missing, adaptive feed not tested, a few more things... |
|
at the moment still hardening the Feed Override system, it is quite complex still have not squashed all possible ways things could go wrong, but getting closer each day... |
|
Preserving the generic plugable nature of kinematics across trajectory planners is a very nice feature and should be preserved if possible. |
Good point. The math is actually already shared, each kinematics module has a *_math.h header (e.g. 5axiskins_math.h, trtfuncs_math.h) with pure static inline forward/inverse functions and no RT dependencies. Both the RT module and the userspace lib call into these same headers. What differs is the glue code around the math. The RT side creates HAL pins with hal_pin_float_newf() and reads them by direct pointer dereference, while userspace walks HAL shmem by pin name string through hal_pin_reader. Init is hal_malloc() + switchkinsSetup() + EXPORT_SYMBOL() on the RT side vs calloc() + function pointer dispatch in userspace. Logging is rtapi_print() vs fprintf. Trying to unify these into a single .c would mean heavy #ifdef RTAPI scaffolding around everything except the math, which is already shared. The real obstacle for custom kinematics in planner 2 isn't math duplication, it's that kinematics_user.c needs to know the module exists at compile time (enum entry, function pointers, HAL pin names for refresh()). A user-written RT module has no matching userspace entry. A possible path: let custom modules optionally ship a mykins_userspace.so implementing a standard kins_userspace_init() API, which the planner dlopen()s at runtime. That would make planner 2 pluggable the same way RT kins already are, without enumerating every module. If that's too heavy, falling back to planner 1 for unknown modules is the simplest safe option. If you are able to conjure up a method to make this work, I'd be glad to implement it. |
That is the real problem. You have static enumeration instead of a dynamic plugable system. Why do you need enumeration? Your interface into the kinematics should be generic. The real question is, when the math is shared, what glue is required for userspace to be usable with the new planner and what is the glue code for realtime. The glue-code should be the same for each and every kinematics for the interface. Therefore, you only need to devise a way to compile the kinematics modules so they give you two resulting loadable modules, one for realtime and one for userspace.
That is how I think it is supposed to be, yes. Just like the realtime kinematics. You probably only need to be able to load and not to unload modules (the kinematics is set in a configuration file and cannot be changed at run-time).
Have a look at A similar strategy will also work for userspace kinematics. |
|
Would it be possible to pass an ID value to the kinematics modules and use that instead of the enum? Then users could configure / customize it . |
|
Deep into hardening feed override system, I'll have a better look at the kins probably tomorrow, thank you for the input, I'll try my best to make it work, I'm sure there is a possible approach. |
|
The last PR addresses all jerk spikes I was able to find, my testing method was running gcodes with small segments, at high feed rate, and having a script wildly swing the feed override, the feed override hand-off branching system is now basically bullet proof as far as I've tested, and I have tested it a lot. |
|
Implemented the dlopen plugin approach. Here's what changed: The kinematics_type_id_t enum and map_kinsname_to_type_id() are gone entirely. No IDs, no dispatch switch. The module name string is the identity — it maps directly to _userspace.so. Each kinematics module now ships a small plugin (50-150 lines) that exports one symbol: kins_userspace_setup(). The loader does dlopen(EMC2_HOME "/lib/kinematics/" name "_userspace.so"), calls setup, and the plugin sets its forward/inverse/refresh function pointers. Built-in and custom modules are loaded identically. The 17 built-in kinematics were extracted into self-contained .c files under plugins/. They reuse the existing *_math.h headers (pure math, no HAL deps) — same shared code that RT uses. The glue is minimal: read params from ctx->params, call the math function, done. If planner 2 is requested but the plugin .so doesn't exist (custom kins without a userspace plugin), it warns and falls back to planner 0 instead of aborting. Custom kins still work fine on planners 0/1 as before — they just won't get planner 2 until they add a _userspace.so. kinematics_user.c went from ~1500 lines to ~280. The shared memory struct changed (removed type_id field) |
|
Good to hear you implemented dlopen(). But I'm still not sure why you moved the actual forward/reverse kinematics calculation into a *_math.h header file. That seems to defeat the one source file and two glues. Header files are usually a very bad place for code. Header files are there as an interface layer. Sure, using "static inline" qualifiers makes them local, but that is, IMO, a very bad habit. What I had expected was:
Or is the *_math.h header a remnant from the previous code iteration? |
|
In C, code in header files is indeed not a common practice. If we don't want to abandon RTAI just yet, I think it is usually not possible to link one object file to a kernel module and to a normal program. IMO #inlcude-ing the code is not a bad idea in this case, also keeps the build system out of the loop. The sources to be included could be renamed to a different ending like |
Creating a .ko can be done from multiple .o objects, just like creating an .so can be created from multiple .o objects. There is no difference afaics. Only the userspace/kernelspace interface/glue layer is different, which is done in rtapi. I only propose to add some glue to differentiate between linking RT and non-RT kinematics modules. I don't think the non-RT kinematics can run in kernel space. @grandixximo must pitch in here to make that assessment whether the non-RT kinematics could ever be a kernel module.
I don't think we should be using this type of code inclusion at all. And for RTAI, it seems that development has stopped completely. I'm not sure it is worth the effort to keep it in very much longer. There is already a lot that does not work with RTAI anyway. |
|
I don't think they can be a kernel module, because they run on a userspace thread, but I'm no expert, and have not really explored this deeply yet. |
|
I have no problem if kernel-mode stuff is abandoned, but then it should be stated, and all that C / C++ schisma could be resolved / isn't needed in new code, so the whole split would be pointless. If we want to keep kernel support for now, linking one object into userspace and kernel objects is of course possible in theory, but it is asking for trouble. math stuff is handled differently for one, you can't just include <math.h> in kernel code, rtapi_math.h has conditional compilation depending on KERNEL or not. It may work to link stuff compiled against the "wrong" prototypes, but that is a hack at best. There may be other problems like LTO, autovectorization, calling conventions, frame pointer and in worst case it would break on "wrong" kernel configs. I don't think it's worth it just to get rid of a |
|
I had a better look at this. I considered the single .o approach, but the *_math.h pattern has advantages, for example in BUILD_SYS=normal (kernel), RT objects are compiled with -nostdinc and kernel includes, so the same .o can't serve both contexts. The math headers work for both build systems with zero #ifdef. They could be renamed to .inc if the .h extension bothers, but static inline functions in headers is the same pattern the Linux kernel uses extensively (list.h, rbtree.h, etc.), so unless the kernel also has bad habits, I think it's fine. |
|
I might be wrong about this, but I explored this for a while before settling on the shared header approach. The alternative would be splitting each *_math.h into a .h (prototypes) and .c (implementation), then compiling the .c twice with different flags and updating the link rules for every module. It's doable but adds significant Makefile complexity for the same result. If you'd prefer that approach I can implement it. |
|
If I understand it correctly, the loadable kinematics module is not going into the same process space for RT (rtai_app) and non-RT (milltask?). When your new TP cannot load into the kernel (RTAI) then we do not need to consider that option too seriously, just enough to bypass in compilation. In the case of uspace, why can't the same kinematics .so be loaded into two different processes and perform their specific function in the process' context? The motion controller links directly into the kinematics{Forward,Reverse} functions, which means that the kinematics .so must be loaded before the controller's .so to satisfy the dynamic linking process. If you also export appropriate functions for your non-RT process hook, then you could, in principle, load the same .so in both processes and have it perform the kinematics there too. Or am I missing something here? |
The RT .so (e.g., maxkins.so) does hal_init() + hal_pin_new() in rtapi_app_main(), and kinematicsForward() reads params directly from HAL pin pointers. Loading the same .so in a second process would either conflict on hal_init() or need runtime detection to skip it and read parameters differently. The separate userspace plugin avoids that, it reads HAL pin values through a read-only interface without registering as a HAL component. |
|
Afaik, only when you run But, you don't need to call rtapi_app_main() at all when you yourself do the dlopen(). A call to dlopen() will do nothing more than resolve the dynamic link dependencies. Adding RTLD_LOCAL will prevent exporting any symbols from the loaded .so and the only way to get to them is to use dlsym(). You don't even need worry or care about the kinematicsForward and kinematicsReverse symbols (functions). You can simply split the mathematics inside the kinematics source and implement and export, lets say, as an example, You can also prevent your functions from being exported in a kernel build simply by placing the definition and EXPORT_SYMBOL() invocations in a #ifndef __KERNEL__ conditional. More should not be required. |
|
You're right that dlopen() alone won't call rtapi_app_main(), confirmed. Both approaches work, so here's a comparison from a maintenance perspective: Current approach (math headers + separate plugins): Math extracted into *_math.h, RT modules and userspace plugins both include it Math stays in the .c file, nonrt_* functions exported alongside RT functions |
That is completely optional. You are allowed to export the non-RT functions and they will simply go unused and fill a marginal amount of space. No problem with that. As long as there are no dyn-link refs, but that is a naming question. You only need to make sure that it links, which could mean the requirement of a few stubs. Although, the code can be designed that no or only few stubs are required.
That I see as an advantage because the actual kinematics is in one file. You can reuse code more effectively. You do have to choose carefully what the interface does. You do not want to replicate code from higher layers in the modules.
That is a general issue in all of the components already because of the kernel/userspace boundary. The RT/non-RT boundary is easier to handle. Just make your code run as RT, then it should also run as non-RT. I can't imagine that your use of the kinematics calculations changes its actual behaviour in any meaningful way. Or does it? If not, then it should be a moot issue. The biggest advantage here is that the changeset should be easier to understand and people with their own kinematics component can add/change their code to work with the new way a bit easier. I guess a "how to migrate kinematics components" document would be required in any circumstance. |
|
Kernel modules are pretty restricted in what they can do, whereas in the userspace realtime thread nearly everything is allowed, including C++, exceptions, etc... Things with non-constant upper limit of runtime should be avoided though, and code should only access data and code that is locked and can't be evicted or paged out. Shared memory segment and stack is OK, dynamic memory probably not. The rtapi glue code memlocks all code, but probably not stuff that you dlopen somewhere in a module, so that should be checked. |
|
Agreed, I will go ahead with refactoring, thank you for the guidance.
The dlopen() of the RT .so happens in milltask (non-RT), not in the servo thread. |
|
refactored the Kinematics, much cleaner approach, thank you @BsAtHome for the guidance |
grandixximo
force-pushed
the
9d
branch
2 times, most recently
from
19663a5 to
3d04de5
Compare
Replace the old profile-valid + queue-depth gates with a single FINALIZED gate that checks optimization_state >= TC_PLAN_FINALIZED before allowing RT to activate any segment. - Forward pass only stamps FINALIZED when exit boundary conditions are known: EXACT segments (vf=0 always correct), segments with a successor in queue, or tail segments when queue is sealed. - SKIP path re-stamps FINALIZED after backward pass knocks state back to SMOOTHED. - queue_sealed flag in TP_STRUCT: set by tpFlushCompressor_9D at sync points (dwell, mode change, program end), cleared by tpAddLine_9D/tpAddCircle_9D when new motion arrives. Lets the optimizer finalize the tail segment immediately instead of waiting for a successor that will never come. - 200ms safety-net timeout for cases not covered by the seal (first segment after tool change, program start). - Cleanup: removed debug probes (GATE_DBG, ACTIVATE_DBG, QUEUE_DBG, OPT_DBG, SEAL_DBG, XING_DBG, FWD_VF_DBG), stale active-segment rewrite, pessimistic first-profile hack.
When a short segment (e.g. 0.030mm arc) completes within a split cycle's remaining time, the leftover time produced zero displacement, causing a 1-sample velocity dip visible as ~1.9M mm/s³ jerk spikes. Chain-split forwards leftover time to the next segment instead of losing it. After tpUpdateCycle detects nexttc completed within the split budget (profile duration <= remain_time), the loop marks it for removal and activates the next-next segment with the leftover time. Multi-pop removal in tpRunCycle ensures all chained segments are dequeued in the same servo cycle, preventing dead-cycle velocity dips. Also removes all investigation debug probes (JERK_DBG, SPLIT_DBG, JUNCTION_DBG, GAP_DBG, RESET_DBG, CHAIN_DBG, PATH_DBG, FWD_V0_DBG, FWD_SKIP_DBG).
Block segment consolidation when prev_tc has been trimmed by a blend. pmCartLineInit recomputes uVec from (trimmed_start → new_end), changing the line direction, while the attached blend still references the old uVec — causing 0.4-0.9° direction jumps at bezier→line split boundaries. Also add endpoint dκ/ds fields to Bezier9 for curvature-rate analysis.
G64/G61 mode changes called tpFlushCompressor_9D which sealed the queue, causing the optimizer to finalize the tail segment immediately with wrong boundary conditions (full length, vf=0) before the successor segment and its blend trimming arrived. RT would then activate the segment, accelerate to cruise speed, and when the blend later trimmed the segment to ~half its length, Ruckig had to emergency-brake in the tiny remaining distance — producing a jerk spike. Fix: add tpFlushCompressorNoSeal_9D that flushes the compressor without setting queue_sealed. Used for mode changes where new motion immediately follows. tpFlushCompressor_9D (with seal) is reserved for true sync points: dwell, M0/M1, program end.
…red by EXTRA_CFLAGS -I paths)
emc/ini/ does not have -Iemc/motion in its compile flags, so the flat "motion.h" include fails on ARM and clang builds.
… prefixes Flat includes of tc_types.h, tc.h, tp.h, tp_types.h and motion.h fail when motion_planning headers are included from directories without -Iemc/tp or -Iemc/motion in their compile flags (e.g. emc/ini/). Use "../tp/" and "motion/" qualified paths throughout, consistent with the upstream three-tier include ethos.
…n_types.h blendmath.h is in emc/tp/, motion_types.h is in emc/motion/ — both need qualified paths when compiled without -Iemc/tp or -Iemc/motion in scope.
Each module now exports nonrt_attach(shmem_base, offset, ops) instead of nonrt_kinematicsForward/Inverse. The planner calls nonrt_attach once at startup to get the shmem pointer and register forward/inverse ops. Modules with HAL-configurable params use a !haldata guard in the RT forward/inverse functions to detect the userspace context and read from shmem via KINS_SHMEM_READ. Modules with no configurable params (corexykins, rotatekins, scorbot-kins) register the RT functions directly. trivkins sets is_identity=1 and skips kinematics entirely. Also updates kinematics.adoc to describe the current architecture.
Add hal_struct_newf() / hal_struct_attach() / hal_struct_detach() to hal.h and hal_lib.c as a clean public API for naming opaque blobs in HAL shmem without exposing hal_priv.h internals. Migrate all 17 kinematics modules and kinematics_user.c: - remove #include "hal_priv.h" from every kins module - replace hal_malloc + hal_param_s32_newf(...uspace-params-offset...) + SHMOFF(ptr) with hal_struct_newf(comp_id, sizeof(kinematics_params_t), NULL, "<module>.params") + hal_struct_attach() - remove self_offset field from kinematics_params_t - simplify nonrt_attach signature from (char *shmem_base, int offset, nonrt_ops_t *ops) to (nonrt_ops_t *ops); each module calls hal_struct_attach() internally to obtain its uspace_params pointer - simplify kinematics_user.c: drop hal_pin_reader dependency for the offset lookup, call hal_struct_attach() directly in find_shmem_params() - update kinematics.adoc custom module guide to reflect new API
Replace the param-backed implementation with a proper struct namespace inside HAL shmem. hal_struct_newf now maintains its own linked list (struct_list_ptr / struct_free_ptr in hal_data_t), sorted by name, entirely separate from pins, signals, and parameters. hal_struct_attach increments attach_count; hal_struct_detach decrements it, making detach a meaningful operation. No hal_param_s32_new call anywhere in the new path. HAL_VER bumped to 0x11 for the hal_data_t layout change.
Four fixes from review: - Fix indentation: new functions used 4-space instead of tabs - Add comp->ready check in hal_struct_newf, consistent with hal_param_new and hal_pin_new - Check for duplicate name before shmalloc_up: the bump allocator has no free, so allocating then detecting a duplicate would silently leak shmem - hal_struct_detach now returns -EINVAL when attach_count is already 0 instead of silently ignoring the over-detach
- docs/man/man3/hal_struct_newf.3: full man page for hal_struct_newf, hal_struct_attach, hal_struct_detach with SYNOPSIS, ARGUMENTS, DESCRIPTION, RETURN VALUE, EXAMPLE, and SEE ALSO sections - hal_struct_attach.3, hal_struct_detach.3: .so redirects to main page - halcmd_commands.cc: add print_struct_info() and print_struct_names() following the print_param_info/print_thread_info pattern; wire into do_show_cmd (show struct, bare show, show all) and do_list_cmd - halcmd_completion.c: add "struct" to show_table and list_table
Replace the hand-written troff files in docs/man/man3/ with a proper AsciiDoc source at docs/src/man/man3/hal_struct_newf.3.adoc, matching the format used by all other HAL man pages. Register the new page in docs/po4a.cfg so the build generates the troff and translations from it.
Two root causes addressed: 1. Backward-pass missing kink constraint (motion_planning_9d.cc): The backward pass computed prev_tc's final_vel without applying tc->kink_vel (the junction kink at the prev_tc→tc boundary). Only prev_tc->kink_vel (its own entry kink) was applied, leaving the predecessor free to exit faster than the downstream junction allows. Add tc->kink_vel as Constraint 4 in tpComputeOptimalVelocity_9D so the predecessor's exit is always capped to the physical junction limit. 2. Stale-feed profile v0 mismatch at handoff (tp.c, tc_types.h): When feed override changes after a profile is written but before RT reaches that segment, profile.v[0] reflects the old feed while the actual junction velocity reflects the new feed. tpUpdateCycle samples the profile at t=0, snapping currentvel to the stale v0. Fix: stamp each profile with written_at_feed (the committed feed at write time). At split-cycle handoff, when the feed drift exceeds 5%, clamp nexttc->currentvel to the physical junction_vel and correct progress/position_base proportionally. Userspace re-converges a fresh profile within 1-2 cycles.
Mark shared blob pointers as volatile in the example code on both RT and userspace sides, with a note that volatile alone does not provide memory ordering guarantees and that atomics are needed for sequence-lock fields. Add hal_struct_attach/detach/newf.3 to docs/man/.gitignore so the asciidoc-generated man pages are not reported as untracked.
Provides macros (KINS_READ, COMP_KINS_BEGIN/END, COMP_KINS_NONRT_ATTACH) that let halcompile-based .comp kinematics modules work with the userspace trajectory planner without requiring conversion to hand-written .c files. Converts xyzab_tdr_kins.comp as reference example and adds documentation section to kinematics.adoc with the 5-step conversion recipe.
- Make motion_planning Submakefile explicit: only copy bezier9.h to include/ (blend_sizing.h uses ../tp/ paths that break in flat include/) - Align blend_sizing.h includes with bertho's convention (<> for system) - Remove orphaned tc_log.h from include/ (no source, no Submakefile) The header-sanity failure was caused by stale tp headers lingering in include/ from before bertho's commit 82f05bc that removed the tp→include copy rules. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6 participants
No description provided.