G33 minor change #6700
G33 minor change #6700
Conversation
LVD-AC
commented
May 12, 2017
LVD-AC
commented
- A and O parameters obsolete
- quick homing with home_delta()
- preventing premature iteration end
- "first probe" fail safe
- further named remaining tests
- renaming tests
-A and O parameters obsolete -quick homing with home_delta() -preventing premature end -"first probe" fail safe use the first probe result to prevent scraping the bed or floating to high above it -further named remaining tests - renaming tests
|
All unmerged developements from https://github.com/LVD-AC/Marlin-AC uptill 1.1.1a-AC of 16/05/2017 |
| // set the radius for the calibration probe points - max 0.8 * DELTA_PRINTABLE_RADIUS if DELTA_AUTO_CALIBRATION enabled | ||
| #define DELTA_CALIBRATION_RADIUS (DELTA_PRINTABLE_RADIUS - 17) // mm | ||
| // set the radius for the calibration probe points - max DELTA_PRINTABLE_RADIUS*0.869 if DELTA_AUTO_CALIBRATION enabled | ||
| #define DELTA_CALIBRATION_RADIUS (DELTA_PRINTABLE_RADIUS*0.869) // mm |
There was a problem hiding this comment.
extra parentheses are needed for safety
#define DELTA_CALIBRATION_RADIUS ((DELTA_PRINTABLE_RADIUS) * 0.869) // mmThere was a problem hiding this comment.
???
Like in
#define DELTA_PROBEABLE_RADIUS (DELTA_PRINTABLE_RADIUS - 15)
#define Z_PROBE_ALLEN_KEY_STOW_2_FEEDRATE (XY_PROBE_SPEED/10)
and all the other instances where a calculation is performed?
I rather keep it as is, it works flawless and it is consistent with all the rest of Marlin code.
There was a problem hiding this comment.
it is consistent with all the rest of Marlin code.
I've been patching up the use of preprocessor defines and wrapping them in parentheses throughout the code for many months. Let's continue to apply this safety procedure.
All preprocessor defines must be parenthesized if used like so:
(DEFINED_THING) * 2.0
(DEFINED_THING) / 2.0
SOMETHING - (DEFINED_THING)Basically, imagine that DEFINED_THING is equal to the string "3 + 10", and then imagine what will happen when 7 * DEFINED_THING is expanded. Clearly, the result 7 * 3 + 10 (31) is bad, whereas 7 * (3 + 10) (91) is correct.
Where are parentheses not needed? Usually, this is fine and we let it slide:
17 + DEFINED_THING...since that won't be a problem if DEFINED_THING is "3 + 10". However, we may tighten up on that as well, because if DEFINED_THING is equal to something like "flag ? 5 : 10" even the expansion of a simple addition will be incorrect.
17 + flag ? 5 : 10
flag ? 5 : 10 - 22(XY_PROBE_SPEED/10)
Should really be ((XY_PROBE_SPEED) / 10).
There was a problem hiding this comment.
Ok I'll change this in the delta section; but I'm not gonna browse through all the marlin code to change this 'error' in every occurrence.
So basically (DELTA_PRINTABLE_RADIUS) * 0.869 will do?
There was a problem hiding this comment.
Background: All options in Marlin may be set externally, via Makefile, shell environment, etc. Since we have no control over what values any configuration option will contain, we must wrap all options in parentheses when used in equations, comparisons, etc. Otherwise, a value like "X_MIN_POS + 10" may cause hard-to-find bugs.
The ability to override options from an external source is also the reason that we use ENABLED instead of defined for simple config options. It allows configuration options to be disabled by:
- being undefined (as we do in the configs), or
- by being set to
0orfalse(as aMakefilewould be required to do).
There was a problem hiding this comment.
I'm not gonna browse through all the marlin code to change this 'error' in every occurrence.
No worries. I consider that my job, anyway, since I am most intimate with the code.
There was a problem hiding this comment.
PS: I've learned from reading (and copying) the existing code and I've noticed that DEFINED_THING was never set to something like blablabla, but always to (blablabla), so this is not a critical mistake. But since Marlin is appently in a transitional state from putting the parenthesis's around blablabla to putting them around DEFINED_THING, I'll put them around both.
There was a problem hiding this comment.
Good luck with that; there are a lot of instances to be corrected
Marlin/Marlin_main.cpp
Outdated
| _7p_intermed_points = _7p_calibration && !_7p_half_circle; | ||
|
|
||
| if (!_1p_calibration) { // test if the outer radius is reachable | ||
| for (uint8_t axis = 1; axis < 13; axis += 1) { |
|
Anyhow, I will give this another sweep and merge it at some point later today. |
|
Or… in the morning… |
| @@ -1,4 +1,4 @@ | |||
| # Marlin 3D Printer Firmware | |||
| # Marlin 3D Printer Firmware | |||
There was a problem hiding this comment.
Be sure to use "UTF-8" encoding, not "UTF-8 with BOM" encoding. Otherwise the first line of text and source files will keep showing up as changed.
|
Merged… |
