Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Marlin DIR Bits: 1=Forward, 0=Reverse #25791

Merged

Conversation

thinkyhead
Copy link
Member

Simpler FT_MOTION changes ahead of the more involved #25719 refactor.

  • Invert the DIR bits in FT_MOTION to match the Marlin standard (1 = Reverse/Negative).
  • Use (AXIS)_APPLY_DIR to support multi-axis, multi-extruder, and extruder variants.

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch 2 times, most recently from fc97827 to e5d82f6 Compare May 8, 2023 07:14
@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch 3 times, most recently from 6465d3f to 2137420 Compare May 8, 2023 07:34
@narno2202
Copy link
Contributor

In order to get a functional multistepper axis (some kind of obsession), (AXIS)_APPLY_DIR should be modified as in the fix proposed in #25635. After that, probably more people could test FT_MOTION and provide feedback.

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch from 2137420 to 82cef84 Compare May 8, 2023 09:22
@thinkyhead
Copy link
Member Author

In order to get a functional multistepper axis (some kind of obsession), (AXIS)_APPLY_DIR should be modified as in the fix proposed…

I don't think any changes to (AXIS)_APPLY_DIR are needed. Perhaps you didn't notice that INVERT_X2_VS_X_DIR is already covered by INVERT_DIR(X2_VS_X, v) and that (AXIS)_DIR_WRITE already applies the configured INVERT_(AXIS)_DIR.

@narno2202
Copy link
Contributor

Delete the fix, compile, flash and now all my axes are moving correctly (it's was not the case before). You were right :)

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch from 82cef84 to 608fcb0 Compare May 8, 2023 09:45
@narno2202
Copy link
Contributor

I need to change (AXIS)_STALL_SENSITIVITY for sensorless homing with FT_MOTION enabled. I think FT_MOTION should be disabled during homing to avoid this changes once the printer is correctly tuned without IS or FT_MOTION.

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch 5 times, most recently from a1b2696 to 76ab68b Compare May 8, 2023 10:33
@thinkyhead
Copy link
Member Author

thinkyhead commented May 8, 2023

FT_MOTION should be disabled during homing…

@narno2202 — In the completed version at #25719, presumably one should be able to run FT_MOTION always without any difference in behavior. We'll see! Do you know what it is that's currently missing either here or in that PR which makes sensorless homing untenable? One thing that is not yet accounted-for –but added in #25719– is updating count_position for all the other axes, not just Z.

Meanwhile, I just added a commit to this PR that reverses the meaning of direction_bits throughout the Marlin planner/stepper code because it's more consistent with the DIR state, where HIGH is used to indicate forward motion (by default), and so, for example we can now just do X_DIR_WRITE(last_direction_bits.x). Perhaps this can lead to some small compile optimizations.

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch 7 times, most recently from 2609bae to f796470 Compare May 8, 2023 11:52
@narno2202
Copy link
Contributor

Oups, i clone the bad repo. With f796470, with FT_MOTION enabled, axis motion OK but same problem with sensorless homing. With FT_MOTION disabled (M493 S0), Z axis goes the good way, X and Y axis are reversed.

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch 2 times, most recently from 1ce66a6 to d2128d7 Compare May 8, 2023 12:29
@thinkyhead
Copy link
Member Author

…unreliable sensorless homing…

In what way is it unreliable? No detection of the stall so it ends up grinding?

Solved with FT_MOTION disabled then reENABLED…

Is this done by sending M493? One thing that this does is to call fixed-time reset() which –as the name suggests– resets all the FT buffers. But it doesn't do anything specifically related to stallguard, so that's interesting. So why this makes the next homing motion work is still an open question.

My first instinct is to look at how FT Motion uses stepper.abort_current_block which is set by an endstop hit. We need to figure out if the endstop hit is being registered and whether the flag is actually being set. If that much happens then we can isolate the issue to certain parts of FT Motion. However if the endstop hit is not being registered then we need to look at how the sensorless endstops are being polled and also look at whether this affects switch-based endstops.

I've got a couple of test machines standing by. Neither one is using sensorless homing, but maybe the answer will become obvious in the process of testing.

@narno2202
Copy link
Contributor

Sensorless homing is never done with FT_MOTION enabled. When using M493 S0, sometime one axis home and the other not (not always the same). Most of the time it's look like too much sensitivity (endstop hit). I change the STALL_SENSITIVITY, same result but when it's homing ends-up with grinding. With the code in void homeaxis(const AxisEnum axis) homing works fine but i need to call fixed time reset() after fxdTiCtrl.cfg_mode = FTM_DEFAULT_MODE to get correct motion.

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch 3 times, most recently from ec13253 to 434a59d Compare May 9, 2023 19:06
@thinkyhead
Copy link
Member Author

Sensorless homing is never done with FT_MOTION enabled. When using M493 S0, sometime one axis home and the other not…

So, this sensorless homing problem only exists with this PR, and not with unmodified bugfix-2.1.x? And, only after using M493 S1? Or always? Or, are other steps required before sensorless homing breaks?

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch from 434a59d to 9927982 Compare May 9, 2023 20:57
@narno2202
Copy link
Contributor

narno2202 commented May 9, 2023

With unmodified bugfix-2.1.x no sensorless homing problem. With this PR and FT_MOTION enabled, sensorless homing always fail. But now with 9927982 and M493 S0 sensorless homing works fine (not the case before) but fail again after M493 S1.

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch 3 times, most recently from 8684b10 to 2d33e9d Compare May 15, 2023 05:04
@narno2202
Copy link
Contributor

i did some test this week end. With ulendo original, same failure with sensorless homing. the main difference is the noise during the various movement (less noisy). With 2d33e9d, always the same problem for sensorless homing and FT MOTION enabled. Maybe a link to the solution : when i do a move of an axis before homing (regardless of the length), sensorless homing is always successful for this axis.

@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch from 2d33e9d to 1cb4dac Compare May 16, 2023 00:30
@thinkyhead thinkyhead force-pushed the bf2_ft_motion_dir_PR branch from 1cb4dac to a7d91d3 Compare May 16, 2023 02:10
@thinkyhead
Copy link
Member Author

when i do a move of an axis before homing (regardless of the length), sensorless homing is always successful for this axis

That's not too weird, and almost what I would expect. There's probably something about the initial motion that is preparing some variables or stepper driver states that come into play during sensorless homing. It will surely become clearer after doing more debugging.

I'm tracking another issue with DIR state, maybe related to the recent move of DIR INVERT handling to a lower level. That bug can be reproduced by: homing, moving XY to the center, sending M502 (which reinitializes the stepper drivers), and then attempting to move X left (in my case). Instead of moving left it moves right! So I need to get that solved ASAP.

I've also been working on Fixed-Time Motion essentials, so I'll be merging this PR in its current state shortly, then following up tonight with FT Motion EEPROM support and a MarlinUI menu (within the Motion menu) to select FT Motion mode and set its parameters.

I'll then continue my debugging of the DIR issue while also testing the code in #25719 on a couple of my test machines, starting with an LPC1769-based TH3D EZBoard with TMC2208 (UART) in a CR-10S, the STM32-based Ender-3 V2, and a Chiron with an AVR Trigorilla board and TMC2209_STANDALONE drivers (i.e., only the DIR issue on that machine since FT Motion is most likely guaranteed to fail on AVR).

So we can move discussion over to that PR next, since it represents the final completion of FT Motion for up to 9 axes (but maybe not fully supporting DISTINCT_E_FACTORS just yet).

@thinkyhead thinkyhead merged commit 25ddde0 into MarlinFirmware:bugfix-2.1.x May 16, 2023
@thinkyhead thinkyhead deleted the bf2_ft_motion_dir_PR branch May 16, 2023 03:00
@thinkyhead thinkyhead changed the title FT_MOTION: Reverse DIR bits, Multi-Axis Change Marlin DIR Bits: 1=Forward, 0=Reverse May 16, 2023
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 15, 2023
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 18, 2023
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 18, 2023
tombrazier added a commit to tombrazier/Marlin that referenced this pull request Apr 5, 2024
…tion in Backlash::get_applied_steps() that the zero backlash steps applied state is forwards
tombrazier added a commit to tombrazier/Marlin that referenced this pull request Jun 25, 2024
…tion in Backlash::get_applied_steps() that the zero backlash steps applied state is forwards
thinkyhead pushed a commit that referenced this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants