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

Improved homing current management #26714

Merged
merged 28 commits into from
Aug 15, 2024

Conversation

sargonphin
Copy link
Contributor

@sargonphin sargonphin commented Jan 20, 2024

Description

This change improves how homing current is used, generalizes its use to other forms of probing and is flexible for future usages

Requirements

This change benefits to printers using Trinamic drivers and (optionally) bed probes

Benefits

  • Improves homing reliability by only applying homing current to the actively homed axis, taking special kinematics into considerations
  • Significantly improves StallGuard homing experience by allowing much higher XY_PROBE_FEEDRATE, thus allowing faster bed probing (leveling, alignment, ...) as well
  • Allows the specific application of homing current for any future feature that would benefit from lower motor current
  • Allows using the homing currents for probing (#define IMPROVE_PROBING_SAFETY in Configuration_adv.h) to reduce the risk of damage in the event of a bad sensor (broken, installed improperly, wrong pin, ...)
  • This PR also adds an extra SanityCheck to prevent using N_CURRENT_HOME values greater than normal N_CURRENT (where N is the axis name)

Testing

AT THE MOMENT, THIS REQUIRES TMC DRIVERS

  • In Configuration.h, DEBUG_LEVELING_FEATURE needs to be enabled
  • In Configuration_adv.h, N_CURRENT_HOME needs to be set below N_CURRENT for X, Y and Z axis (all motors)
  • If you have a bed probe, in Configuration_adv.h un-comment #define IMPROVE_PROBING_SAFETY
  • A host needs to be connected to the printer
  • Send M111 S247 to enable debug
  • Send G28 (to home all axis)
    -- Copy the terminal output in a text file labeled G28.txt
  • Optionally, and if enabled, send G34 (Z-Steppers Auto Alignment)
    -- Copy the terminal output to a text file labeled G34.txt
  • Optionally, and if enabled, send G29 (Bed Leveling)
    -- Copy the terminal output in a text file labeled G29.txt
  • Send the text files here, along with your printer characteristics (size, kinematics, type of probe, type of bed leveling, number of Z steppers)

Tested Kinematics (Homing)

  • Cartesian
  • CoreXY
  • MarkforgedXY
  • Delta
  • SCARA
  • TPARA
  • Polar
  • Articulated Arm
  • Foam Cutter

Tested Features (still WIP)

  • Bed Leveling 3-Point
  • Bed Leveling Linear
  • Bed Leveling Bilinear
  • Bed Leveling UBL
  • Bed Leveling Manual (Mesh)
  • Z-Stepper Alignment

Related issue

#26369
#26634

@sargonphin sargonphin changed the title [WIP} Improved homing current management [WIP] Improved homing current management Jan 20, 2024
@sargonphin sargonphin changed the title [WIP] Improved homing current management [Needs Testing] Improved homing current management Jan 23, 2024
@sargonphin sargonphin marked this pull request as ready for review January 25, 2024 02:20
@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 2 times, most recently from 9c65146 to 4f65466 Compare January 26, 2024 00:13
@thinkyhead
Copy link
Member

thinkyhead commented Jan 26, 2024

To augment the discussion, it should be noted that we don't reduce the current during homing just to make the steppers more easily slip (i.e., skip steps) when they hit an obstacle. We also adjust the current (in tandem with IMPROVE_HOMING_RELIABILITY reducing acceleration and jerk) so that when homing starts, the initial motion jerk and axis binding won't errantly trigger the stall (DIAG) signal.

I wouldn't necessarily recommend to users that they set their homing current to such a low value that it's easy to stop the motor(s) and grind the steppers, but only low enough that they don't get false-positives. That might correspond to a current that weakens the motor enough to make the motor slip when it tries to go below the glass bed instead of breaking the glass, but that isn't always the main consideration when it comes to homing/probing current.

thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Jan 26, 2024
MarlinFirmware/Marlin#26714

Co-Authored-By: sargonphin <85966195+sargonphin@users.noreply.github.com>
@sargonphin
Copy link
Contributor Author

Amazing rework of my sloppy unskilled coding there! Thanks a lot :D

Indeed the idea started with sensorless homing but it eventually skewed towards preventing damage in case of a bad sensor. I used to rely on this feature a lot, albeit limited to homing, to protect the buildplate when we worked on the toolhead and shifted stuff around. In fact, I did crash the bed in the nozzle a couple times by forgetting to plug the sensor at all XD

The hopes behind this idea was to give incentive to the users to reduce their homing current a little bit, at some point I even wanted to suggest using a factor (for instance X_CURRENT_HOME == X_CURRENT * 0.75) but it broke compatibility with trusty old preprocessor which I discovered, cannot perform such math during preprocessing. Oh well!

I will test tomorrow if everything is still working correctly ^_^ I did a quick compile with a minimum config for my printer and it successfully finished

@sargonphin
Copy link
Contributor Author

OK just tested and behavior is the same on my printer. Seems like it's ready to merge, although while reading the code, I noticed a small issue with quick_home

If the printer is a Core XY or Markforged, it will call the homing current for both axis which will lock the current to homing, since in the function it already sets it to both X and Y at the same time for either axis

@sargonphin
Copy link
Contributor Author

I fine-tuned the sanity checks to check for all possible configuration of X, Y and Z (multi-stepper per axis) and added a sanity check when homing current is not defined when sensorless homing/probing is enabled

@sargonphin sargonphin changed the title [Needs Testing] Improved homing current management Improved homing current management Feb 7, 2024
@sargonphin
Copy link
Contributor Author

Any update regarding the merging of this PR? It has been implemented on my printers for a while now and couldn't find any issues with it so far ^_^

@thinkyhead
Copy link
Member

A lot of other stuff has been distracting, but this is still slated for merge before the release. I'm just going to merge in the latest code now, clean it up, and look at that accidental situation where we set the current twice on core axes. Should be good to go shortly…!

@thinkyhead thinkyhead force-pushed the DEV_homing_current branch 4 times, most recently from e158740 to 1e5e3c9 Compare August 15, 2024 21:46
@thinkyhead thinkyhead merged commit 2aa2e54 into MarlinFirmware:bugfix-2.1.x Aug 15, 2024
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants