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

FT MOTION: add individual axis shaping and new buffer management #26848

Merged
merged 63 commits into from
Jul 15, 2024

Conversation

narno2202
Copy link
Contributor

@narno2202 narno2202 commented Mar 6, 2024

Description

@ulendoalex update the Marlincollaborative repository with a new buffer system management and the possibility to set shaping per axis. This PR adds individual axis shaping function, adapt the FTMotion menu accordingly and the new buffer managemnt system. This PR is a mix between #26720 last working code (homing works for coreXY as reported in anotther discussion, BLTOUCH HS Mode...) and @ulendoalex code with some minor changes. This PR suspercedes #26720 which needs more work to be fully functional.

  • FTMotion can be set on or off globally (only the motion system not the shapers).
  • Shaping can be set to any shaper (ZV, ZVD ...) or disabled on an individual axis basis (only for X and Y axis).
  • Buffer management system is left untouched

Requirements

Ideally, this PR should be merged before the other FT_MOTION PRs :

Configurations

Just enable FT_MOTION in Configuration_adv.h

Related Issues

None

@ulendoalex
Copy link
Contributor

@narno2202

Thanks for creating this, I intended to issue the pull request myself after more testing. I'm fairly confident in the changes, but please tag me if you encounter any issues.

@narno2202
Copy link
Contributor Author

This is not perhaps the best solution, but I was thinking of progressively incorporate your code in current Marlin bugfix after testing and debugging the branch in your repository.
BLTouch HS mode is broken and needs axis_did_move.reset() to work (it's in my PR in Marlincollaborative) and again if FTM_BATCH_SIZE is not equal to FTM_WINDOW_SIZE / 2 : motion is broken (this time for FTM_BATCH_SIZE < FTM_WINDOW_SIZE / 2)

@narno2202
Copy link
Contributor Author

@ulendoalex , in reset() function makeVector_batchIdx = TERN(FTM_UNIFIED_BWS, 0, _MIN(BATCH_SIDX_IN_WINDOW, FTM_BATCH_SIZE)); instead of makeVector_batchIdx = BATCH_SIDX_IN_WINDOW; solves the bug with asymetrical window and batch size (ratio not equal to 2).

@bmourit
Copy link

bmourit commented Mar 10, 2024

This throws a warning about a possible uninitialized variable in menu_motion.cpp line 461, 466 and 470. I am pretty sure it is safe to ignore, but maybe you want to avoid?

Adding a "default:" to the switch cases fixes this warning.

@narno2202
Copy link
Contributor Author

Strange, there is no switch .. case here. I do a compilation test for the simulator with the menu enabled and there is no warning.

@bmourit
Copy link

bmourit commented Mar 10, 2024

in motion_menu.cpp switches at line 414, 420 and 434.

@narno2202
Copy link
Contributor Author

Still no issue.

@bmourit
Copy link

bmourit commented Mar 10, 2024

VS Code is showing it in the console during a real build.

@narno2202
Copy link
Contributor Author

You're right, corrected

@bmourit
Copy link

bmourit commented Mar 10, 2024

Awesome! I'm excited to see this addition (once I figure my machine's issue out). Great work!

@ulendoalex
Copy link
Contributor

@ulendoalex , in reset() function makeVector_batchIdx = TERN(FTM_UNIFIED_BWS, 0, _MIN(BATCH_SIDX_IN_WINDOW, FTM_BATCH_SIZE)); instead of makeVector_batchIdx = BATCH_SIDX_IN_WINDOW; solves the bug with asymetrical window and batch size (ratio not equal to 2).

@narno2202
Thanks for this. I haven't gotten around to investigating this bug yet; hopefully this week.

FYI related to this PR, I did find some bugs and have updated our repository accordingly. You may wish to take a look at those changes to make sure you take them (or already fixed them).

@narno2202 narno2202 changed the title FT MOTION: add individual axis shaping FT MOTION: add individual axis shapingand new buffer management Mar 30, 2024
@sjasonsmith sjasonsmith changed the title FT MOTION: add individual axis shapingand new buffer management FT MOTION: add individual axis shaping and new buffer management Apr 7, 2024
@narno2202
Copy link
Contributor Author

narno2202 commented Jun 18, 2024

Looking at the code, I realize that markBlockStart was always false in ft-motion.cpp : never set to true when a new block is loaded (correctly set in current bugfix). Sensorless homing is fine, BLTouch HS mode is fine too. Waiting for feedback

@thisiskeithb
Copy link
Member

Also, these machines sound terrible with FT_MOTION enabled, so there's probably a lot of re-tuning required compared to standard Input Shaping apparently. 😄

Is it a bug or feature that motors/motions have a high-pitched whine with FT_MOTION enabled? I don’t have this problem with standard Input Shaping.

@thisiskeithb
Copy link
Member

Is it a bug or feature that motors/motions have a high-pitched whine with FT_MOTION enabled? I don’t have this problem with standard Input Shaping.

To answer my own question after re-reading 230119_marlin_fixed_time_ctrl.pdf:

Known Issues and Miscellaneous

  • The operation noise generated by the machine changes under this control method.

I'll see if I can write up a note for M493 - Fixed-Time Motion since I know I won't be the only one wondering if something was misconfigured.

I don't know about other drivers, but at least for UART & SPI-connected TMCs with various motors (stock Creality, LDO, Prusa - OEM'd by LDO, E3D, other generics), they sound like the current is cranked way too high & slightly crunchy.

@narno2202
Copy link
Contributor Author

@thisiskeithb , on my config, this noise was only present in the early development phase of FT_MOTION, now the sound is quite different but without cracking or anything else. Have you tried to increase the stepper command buffer size? Can you post an audio file?

@vovodroid
Copy link
Contributor

vovodroid commented Jun 22, 2024

I tried Remove obsolete config parameters commit

Z homing still fails.

There is indeed some new sound during movements.

Here is recording
FTM.zip

Strong 400 and 200 Hz signal is present with FTM enabled (w/o input shaping)

image

Oscillograms:

No FTM, holding state
image

FTM, holding state
image

Frequency is about 15-20% higher, voltage seems to be the same.

Motion oscillograms (video):
No FTM

no-ftm.mp4

FTM

ftm.mp4

@vovodroid
Copy link
Contributor

vovodroid commented Jun 22, 2024

It seems that FMT produces sharper spikes and tails:

No FMT
image

FMT
image

@narno2202
Copy link
Contributor Author

@vovodroid , could you comment line 3206 in planner.cpp if (TERN0(FT_MOTION, ftMotion.cfg.active)) synchronize() in this commit and give feeback us on your Z Axis homing failure.

@vovodroid
Copy link
Contributor

Unfortunately nothing changed, even when built with old IS disabled.

Would you like me to try with some logs added?

@vovodroid
Copy link
Contributor

I also tried with this line and latest bugfix merged (with #27179), the same failure.

@narno2202
Copy link
Contributor Author

Thanks @vovodroid . Have you tried to enable ENDSTOP_NOISE_THRESHOLD? I'm waiting for a BIqu Microprobe to do more testing. It should be of interest to have feedback with other probes (my BLTouch and 2 other similar probes work fine).

@vovodroid
Copy link
Contributor

Have you tried to enable ENDSTOP_NOISE_THRESHOLD?

I can't as #error "SENSORLESS_HOMING is incompatible with ENDSTOP_NOISE_THRESHOLD."

I'm waiting for a BIqu Microprobe

What could be different here is that BIqu Microprobe is connected to Z-stop connector, not Z-probe.

@narno2202
Copy link
Contributor Author

The BLTouch probe is also connected to the Z min endstop on my moterboard

@narno2202
Copy link
Contributor Author

Finally iI get a Biqu Microprobe V2. I can't get it working for Z homing in current bugfix without FT_MOTION enabled in Configuration_adv.h and in this PR with or without FT_MOTION. I look at @vovodroid configuration files, I test many configurations but It always fails.

@vovodroid
Copy link
Contributor

I can't get it working for Z homing in current bugfix without FT_MOTION

What are symptoms?

Here is relevant parameters:

//#define ENDSTOPPULLUPS
#if DISABLED(ENDSTOPPULLUPS)
  // Disable ENDSTOPPULLUPS to set pullups individually
  //#define ENDSTOPPULLUP_XMIN
  //#define ENDSTOPPULLUP_YMIN
  #define ENDSTOPPULLUP_ZMIN
........
#endif

//#define ENDSTOPPULLDOWNS

#define Z_MIN_ENDSTOP_HIT_STATE LOW
#define Z_MIN_PROBE_ENDSTOP_HIT_STATE LOW
#define BIQU_MICROPROBE_V2    // Triggers LOW
#define PROBE_ENABLE_DISABLE

I leave XY endstop in floating state, as I have sensorless homing. And #define Z_MIN_PROBE_ENDSTOP_HIT_STATE LOW probably doesn't matter, as probe connected to Z-stop connector.

@narno2202
Copy link
Contributor Author

I need to define #define ENDSTOPPULLUP_XMIN, #define ENDSTOPPULLUP_YMIN and MAX for X and Y axis as I use sensorless homing. I use the same configuration as you described. I have the same failure as in your previous posts.

@vovodroid
Copy link
Contributor

As it's reproducible I believe we can catch it with logs.

@thinkyhead
Copy link
Member

Let's merge this to get motion stuff settled, then address endstop and whining in a new PR.

@thinkyhead thinkyhead merged commit f0bc427 into MarlinFirmware:bugfix-2.1.x Jul 15, 2024
62 checks passed
@narno2202 narno2202 deleted the FT_MOTION_update branch July 16, 2024 05:14
thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Jul 23, 2024
@narno2202 narno2202 mentioned this pull request Aug 13, 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.

7 participants