-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
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
Z_RAISE_BEFORE_HOMING -> MIN_Z_HEIGHT_FOR_HOMING #3107
Z_RAISE_BEFORE_HOMING -> MIN_Z_HEIGHT_FOR_HOMING #3107
Conversation
|
||
// Raise Z before homing any other axes and z is not already high enough (never lower z) | ||
if (current_position[Z_AXIS] <= MIN_Z_HEIGHT_FOR_HOMING) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clever, the planner may already have an estimation where the z-axis position could be at.
This looks good to me. I'm always worried with such changes that it will work on some configurations but cause others to behave strangely. How confident are we that this is effective for all configurations? This feature partly mirrors the "simplified clearance options" that I was working on before. (See MarlinFirmware/MarlinDev#366.) In that PR I used the name Once this is updated I will merge it for testing. Seems we'll need to make another PR for |
Names are always difficult to me. Z_PROBE_CLEARANCE is also not very precise. It could be misunderstood as the space between the bed and a already deployed probe. MIN_HEIGHT_FOR_PROBE_HANDLING? I'm quite confident it will work for all systems but DELTAs. But DELTAs have their on homing and they never can home towards a z-min-endstop |
I vote in favor of
What can be misunderstood is, are we speaking about the clearance from the bed to the nozzle or from the bed to the probe ? |
@jbrazio We are talking about is a save height to operate the xy-moves, regardless of nozzle or probe. See bed-screws/clamps as standing obstacles and the probe as hanging one. |
rebased corrected spelling changed to #elif for error Still with MIN_Z_HEIGHT_FOR_HOMING but con be done with (folder wide) search-replace when we have a better name.
cd68646
to
fd5414c
Compare
Rebased Still with MIN_Z_HEIGHT_FOR_HOMING but can be done with (folder wide) search-replace when we have a better name. |
Copy, paste, increase one number or add your new name for this define with a 1.
|
The purpose of increasing Z is to make sure it won't hit something while XY homes. It could already be too low, the bed may not be level, there could be a print on the bed. MIN_Z_FOR_AUTOMATED_XY_MOVES & MIN_Z_FOR_XY_MOVES are not relative. We want It for homing only. If u want it for other moves it can be setup in Repeteir or whatever. I can't change G28. MIN_HEIGHT_FOR_OBSTACLES & Z_RAISE_FOR_OBSTACLES only implies 1 problem MIN_Z_HEIGHT_FOR_HOMING adds HEIGHT which is not necessary and may add confusion Z_RAISE_BEFORE_HOMING uses RAISE which may be LOWER for some printers - like mine |
@ruggb |
If it is for something other than homing, then it is being done AFTER homing on a clean bed. How about 2 separate variables |
Also, wouldn't adding that change the specs of G28 and G29. |
Z_RAISE_BEFORE_HOMING -> MIN_Z_HEIGHT_FOR_HOMING
Please roll this for |
@thinkyhead Since all the work done there, is done by direct commits and not by PRs and the PRs are piling up.
Ommmmmmm. Ommmmmmm. It doesn't help. I'm still angry. |
It's no big deal, we'll compare them along the way. I'm trying to remember now —since no one has pointed out otherwise— if Anyway, I take it you don't like the new file layout...? I have been preserving backward compatibility for Marlin 1.1 so it can build on older toolsets, but it is very tempting to use subfolders now that Arduino 1.6.7 allows it. Even if Marlin isn't packaged in library form, I have no problem making Marlin 1.2 dependent on Arduino 1.6.7 and up. Good news is I believe Dev has gone back to a normal PR-based development model, with no more abuse of direct commits. Everything should now be vetted and more considered. Perhaps some developers will start to float back in. |
The current model gives you a lot of additional manual work, you are the one making sure RCBugFix is in sync with dev, and of course that lags behind because all the bleeding edge users are using RCBugFix to have all the new features. Nevertheless IMHO the code base between the two has diverged and the development branch seems more like a tech preview than a dev branch, in the other hand RCBugFix seems to be the dev branch. |
@jbrazio that is exactly what @AnHardt has warned us all the time for. In a defined and strict project environment, releases will work but in a loose open source development environment without a project release manager things start to diverge. @thinkyhead and @AnHardt are acting as our default project owners and cleaning it up (while issues keeps flowing in). I'm very greatful for the huge amount of work they put into it on a daily basis, without that work Marlin would be dead by now. |
@paulusjacobus I'm a true believer on the bazaar model without well defined goals to be reached. But I also believe that this type of model works better using a rolling release development cycle where there is one repository which gets fine tuned and polished into a version release, afterwards a new dev cycle starts going in (features changes whatever), then polishing starts until another release.. iterate. Well we're going really off topic here. :-) |
No worries, I'm always mesmerized by the fact that we get something like On 11 March 2016 at 11:51, João Brázio notifications@github.com wrote:
|
When you merge changes it closes the PR automatically.
Post hoc ergo propter hoc? I see nothing in this PR that could affect |
Replace Z_RAISE_BEFORE_HOMING with MIN_Z_HEIGHT_FOR_HOMING
This was made as a fix for #3001. @jbrazio was faster.
Nevertheless i'll take the opportunity to simplify z-raising in G28 a bit.
Make raising z work when AUTO_BED_LEVELING_FEATURE is disabled.
Raise for homing every axis, but only once and only if not already high enough.
Clean out some extra complications when Z_SAFE_HOMING is defined.