FT_MOTION : updating last_direction_bits for Core printer solves homing failure

[narno2202]

Description

Some users report homing failure with FT_MOTION enabled and a CoreXY printer. This PR just disable FT_MOTION in G28 when the printer is Core printer. Despite testing no reliable solution has been found, disabling FT_MOTION is just a workaround for now.

Requirements

Benefits

Solve #26683

Configurations

Related Issues

[narno2202]

Regarding MINIMUM_STEPPER_POST_DIR_DELAY, it's not a leftover (I forgot to mention it in the title), the value is based upon STEP input minimum low time from TMC datasheet : solves the buggy axis motion. The second one is clearly a leftover :)

[thinkyhead]

Homing depends on being able to distinguish that a move is occurring in a particular axis, not just with a particular stepper. For Marlin standard Core kinematics motion we set axis_did_move[X_AXIS_HEAD], axis_did_move[Y_AXIS_HEAD], and axis_did_move[Z_AXIS_HEAD].

Meanwhile, Stepper::ftMotion_stepper modifies both axis_did_move and last_direction_bits without consideration for Core kinematics and always overwrites the hx, hy, and hz elements with 0. This effectively breaks the direction checking for endstops, so they will only trigger in the …negative… direction.

The solution is to fill in the hx, hy, hz fields in axis_did_move and last_direction_bits somewhere, most likely in ftMotion_stepper. However, there isn't enough information in ftMotion.stepperCmdBuff to determine the direction of the axes based on the directions of the steppers. This is because axis_did_move is only set when there is a STEP, and core axes don't necessarily have to step together. Meanwhile, the last_direction_bits also don't tell us about the direction of an axis.

It should be possible to add more bit flags to ftMotion.stepperCmdBuff to simply indicate that an axis is generally moving in a particular direction in addition to the flags which indicate that an axis should STEP with a DIR. As it currently stands, the usage of last_direction_bits by FT Motion is mostly compatible. But axis_did_move is constantly toggling as an axis either has a STEP or doesn't have a STEP, which makes it hit-or-miss to discover whether an axis is moving. Under Marlin standard motion axis_did_move is set as long as there are steps pending in the current segment.

There might be other features apart from endstop checking that depend on axis_did_move and last_direction_bits, so for best compatibility we should specifically address Core kinematics in FT Motion and especially make sure that axis_did_move is true throughout the whole move.

[thinkyhead]

We could possibly set the move and direction flags for CORE1 and CORE2 as soon as the new planner block is fetched, assuming that FT Motion immediately starts processing. However, if FT Motion is pre-fetching and populating ftMotion.stepperCmdBuff "asynchronously" ahead of completing motion for the previous block then we will need to add more FT Motion command bits, perhaps named FT_BIT_DIR_C1/C2 and FT_BIT_STEP_C1/C2. I've only added some TODO comments at this point to label the relevant code points.

[narno2202]

@thinkyhead , as the directions are valid until the end of block processing, we could probably set the corresponding fields in last_direction_bits when the block is loaded and before his processing begins in void Stepper::ftMotion_blockQueueUpdate() (as it was the case in early ft_motion code but without the Core logic).

[thinkyhead]

…we could probably set the corresponding fields in last_direction_bits when the block is loaded and before his processing begins…

As long as there's no "pre-fetching" going on that should work. With standard Marlin motion the next block is always fetched after the previous one has completed its STEP pulses. It would be possible for FT Motion to fetch the next block and start adding STEP/DIR events to its own command queue as soon as there's space available in its buffer, even if the block hasn't completed processing yet —which could provide improved continuity— but I wasn't sure if it was already doing that.

[thinkyhead]

…the directions are valid until the end of block processing…

That being the case, do we really need to include DIR information in the FT Motion command buffer, or could the DIR pins also be set just once when the new block is fetched? I gather that including both the DIR and STEP in the same buffer potentially allows for less delay, but most stepper drivers require a delay in-between changing DIR and starting the next STEP anyway.

[narno2202]

After some testing, when homing, there are only 3 consecutive loaded blocks per axis :

1. Block for the first move to the origin
2. Block for bumping if enabled
3. Final block: last move to the origin
   So it could be sufficient to set hx, hx and hz in void Stepper::ftMotion_blockQueueUpdate()

[narno2202]

I am currently running last bugfix with this PR plus all the new Marlincollaborative code (with very minor changes) and an adapted FT_MOTION menu. Motions are fine, sensorless homing is working, BLTouch HS mode too, printing is OK with or without shaping.

[Mjankor]

Thanks narno. Was just passing through to see where this PR was up to. Got a CoreXY here that doesn't play nice with current input shaping.

[ulendoalex]

@thinkyhead @narno2202

I have been doing some testing to resolve endstops [homing] issues and am going to propose the following as a solution. This is a timer based method that sets the axis movement end time when the block gets loaded by considering the movement time and time it takes the block to get converted and propagate through FTM's buffers. It requires some changes to endstops.cpp but is otherwise fairly clean.

I also tried a method that watches the FTM stepper(), similar to how refreshAxisDidMove() used to work. However, it did not play nicely with endstop interrupts. In particular, for the case when a block is loaded with motion away from an endstop that is already activated, commands has not yet propagated to the buffer stepper() is using, and the initial endstop check fails as there is no indication the axis is moving. The endstop isn't checked again since it's invoked only on pin change interrupt, so the axis grinds. All this to say we ultimately need checks when the block is loaded, or need to force endstop polling (which is what an earlier version of FTM used to do).

The timer based method doesn't impact stepper() runtime at all, and works with the endstop interrupts feature. I pushed it here since I'm working on top of some other changes/features to FTM, but the delta for this commit should be useful for your consideration.

I have tested it working on a few printers:

Kinematics	Endstop switch type	Endstop monitor type
Cartesian	Sensorless XYZ	Polling
Cartesian	Sensorless XY, switch Z	Polling
Cartesian	Switch XY, BLTouch Z*	Pin Change Interrupt
CoreXY	Sensorless XYZ	Polling

*Tested HS_MODE as well.

[narno2202]

I update my fork and will test the changes.
@ulendoalex , can you please test this PR with the 2 files provided. I just had a flag whenhomeaxis()is called, send endstops.update() in stepper.cpp only when the flag is true. After homing, endstops are managed by the temperature isr (polling) : on my printer it's OK with this PR and with the mix (previous post).
stepper-motion.zip

[narno2202]

@ulendoalex , I have tested your changes and everything seems to work fine. I still believe that there's no need to use endstops.update() after homing if temperature ISR polling is enough. Waiting for @thinkyhead opinion.

[thinkyhead]

I just went ahead and cherry-picked e589e859d8de7da0ba8da01af2c188536625fd26 and adjusted the code. Do all the reversions and changes from that single commit apply? Let's retain the changes that make sense to get things stabilized and working and we can merge this for further optimization in additional PRs.

[narno2202]

I test it after adding the missing code, each axis move well in only in one direction, the opposite doesn't move at all. Previous code is working well for me as the code in #26848

[Mjankor]

Got this working on my sensorless CoreXY last night. Took a couple of "restore to defaults" to get everything behaving properly, and had to bump the buffer up ~2.5x to prevent misbehaviour on long, fast G0 moves.:

#define FTM_STEPPER_FS          75000
 #define FTM_STEPPERCMD_BUFF_SIZE 15000 //2.5x buffer size over default.

Still trying a few tweaks.

[thinkyhead]

I test it after adding the missing code, each axis move well in only in one direction

@narno2202 — If you have a patch, please add it. I figured some of those reverted sections were inappropriate. We just need to decide which ones to keep and which to discard. Once we have that part sorted out, maybe this will be good to merge (?) for wider testing.

[Mjankor]

A couple of queries.
The noise profile has changed, with a bit of a whine occuring on long, fast moves. Is this noise likely related to:
#define FTM_FS 1000 // (Hz) Frequency for trajectory generation. (Reciprocal of FTM_TS)

What are the ramifications to buffer size and printer behaviour if I change that number (and the corresponding FTM_TS)?

And what do I need to consider when changing these.
#define FTM_BW_SIZE 100 // Unified Window and Batch size with a ratio of 2
#define FTM_STEPS_PER_LOOP 60 // Number of stepper commands to generate each loop()
#define FTM_POINTS_PER_LOOP 100 // Number of trajectory points to generate each loop()

There is also an obvious maximum buffer size that on my machine (Octopus Pro 429) comes in at around:
#define FTM_STEPPER_FS 90000
#define FTM_STEPPERCMD_BUFF_SIZE 18000 //3x buffer size over default.

When I try those values I start seeing stuttering on moves. Is that simply running out of memory or something else?

[narno2202]

Due to some sad familial problem, i am not very focused on Marlin these past days and for some days in the future. I will continue to check the repository.
@Mjankor : you reach the limit of your motherboard with the last values.
In the new PR, buffer system mangement is a little bit different and seems to work well on ly test printer (not a core XY).

@thinkyhead , i don't have any patch for the described issue. I think that the new buffer management system is better than the current one. PR #26848 is a mix between last @ulendoalex code and the last working code of this PR. I'm still thinking that if Temperature ISR polling is enough to check endstops in case of disaster once the printer has homed (motions are in theory constrained within the bed size), so we can use endstops.update() only for homing and avoid unnecessary tests. You're the man with the answer :)

[thinkyhead]

I want to go ahead and merge our current work for further refinement. We still mark this "experimental," but it is reaching the point of being good enough for common usage. We'll continue to tune those configuration defaults as well and provide good defaults for our existing example machines. Thank you to all contributors for your attention and I hope that things are going better for everyone!