Pre-processor bug ?

[lrpirlet]

I am trying to port my https://github.com/lrpirlet/Marlin/tree/motorized_mesh_bed_leveling_for_LRP and to try it with the https://github.com/Sebastianv650/MarlinRC_LIN_ADVANCE... This for the background info...
BTW, @Sebastianv650, thanks for that very nice algorithm...

Anyway, a diff of my configuration.h from the default in RCBugfix shows

$ git diff
diff --git a/Marlin/Configuration.h b/Marlin/Configuration.h
index a79b38e..d68b6ee 100644
--- a/Marlin/Configuration.h
+++ b/Marlin/Configuration.h
@@ -1126,6 +1126,7 @@ const bool Z_MIN_PROBE_ENDSTOP_INVERTING = false; // set to true to invert the l
 // If unsure, leave commented / disabled
 //
 //#define NUM_SERVOS 3 // Servo index starts with 0 for M280 command
+#define NUM_SERVOS 1 //lrp

 // Servo Endstops
 //
@@ -1136,11 +1137,14 @@ const bool Z_MIN_PROBE_ENDSTOP_INVERTING = false; // set to true to invert the l
 //#define Y_ENDSTOP_SERVO_NR 2
 //#define Z_ENDSTOP_SERVO_NR 0
 //#define SERVO_ENDSTOP_ANGLES {{0,0}, {0,0}, {70,0}} // X,Y,Z Axis Extend and Retract angles
+#define Z_ENDSTOP_SERVO_NR 0
+#define SERVO_ENDSTOP_ANGLES {{0,0}, {0,0}, {70,0}} // X,Y,Z Axis Extend and Retract angles

 // Servo deactivation
 //
 // With this option servos are powered only during movement, then turned off to prevent jitter.
 //#define DEACTIVATE_SERVOS_AFTER_MOVE
+#define DEACTIVATE_SERVOS_AFTER_MOVE

 #if ENABLED(DEACTIVATE_SERVOS_AFTER_MOVE)
   // Delay (in microseconds) before turning the servo off. This depends on the servo speed.

I compile that in ARDUINO 1.6.8 (installed on my new computer, not an upgrade or update) and upload it to my printer...

G28... G28 start it's usual dance, the servo deploy, G28 goes down till the z-probe touch the bed twice, servo trys to stow... power loss on the hardware... I know, too much power drain by the servo, I need to give the servo enough z height so that it does not get blocked...

so, I jump in the code... and change the following that must have been written by @Roxy-3DPrintBoard ...

diff --git a/Marlin/Marlin_main.cpp b/Marlin/Marlin_main.cpp
index b3c302d..9809dbd 100644
--- a/Marlin/Marlin_main.cpp
+++ b/Marlin/Marlin_main.cpp
@@ -2161,19 +2161,26 @@ static void homeaxis(AxisEnum axis) {
     current_position[axis] = 0;
     sync_plan_position();

-    #if ENABLED(Z_PROBE_SLED)
-      #define _Z_SERVO_TEST       (axis != Z_AXIS)      // deploy Z, servo.move XY
-      #define _Z_PROBE_SUBTEST    false                 // Z will never be invoked
-      #define _Z_DEPLOY           (dock_sled(false))
-      #define _Z_STOW             (dock_sled(true))
-    #elif SERVO_LEVELING || ENABLED(FIX_MOUNTED_PROBE)
-      #define _Z_SERVO_TEST       (axis != Z_AXIS)      // servo.move XY
-      #define _Z_PROBE_SUBTEST    false                 // Z will never be invoked
-      #define _Z_DEPLOY           (deploy_z_probe())
-      #define _Z_STOW             (stow_z_probe())
-    #elif HAS_SERVO_ENDSTOPS
-      #define _Z_SERVO_TEST       true                  // servo.move X, Y, Z
-      #define _Z_PROBE_SUBTEST    (axis == Z_AXIS)      // Z is a probe
+    #if ENABLED(Z_PROBE_SLED)
+      #define _Z_SERVO_TEST       (axis != Z_AXIS)      // deploy Z, servo.move XY
+      #define _Z_PROBE_SUBTEST    false                 // Z will never be invoked
+      #define _Z_DEPLOY           (dock_sled(false))
+      #define _Z_STOW             (dock_sled(true))
+    #elif SERVO_LEVELING
+      #define _Z_SERVO_TEST       (axis != Z_AXIS)      // servo.move XY
+      #define _Z_PROBE_SUBTEST    false                 // Z will never be invoked
+      #define _Z_DEPLOY           (raise_z_for_servo(); deploy_z_probe()) // raise away from the bed before deploying
+      #define _Z_STOW             (raise_z_for_servo(); stow_z_probe())   // or stowing the servo
+    #elif ENABLED(FIX_MOUNTED_PROBE)
+      #define _Z_SERVO_TEST       (axis != Z_AXIS)      // servo.move XY
+      #define _Z_PROBE_SUBTEST    false                 // Z will never be invoked
+      #define _Z_DEPLOY           (deploy_z_probe())
+      #define _Z_STOW             (stow_z_probe())
+    #elif HAS_SERVO_ENDSTOPS
+      #define _Z_SERVO_TEST       true                  // servo.move X, Y, Z
+      #define _Z_PROBE_SUBTEST    (axis == Z_AXIS)      // Z is a probe
     #endif

     if (axis == Z_AXIS) {

That compiles without any problem, I download it to the printer... same behavior
oops...

So I scratch my hair, look around in the code... Yes this is normal, raise_z_for_servo(); needs AUTO_BED_LEVELING_FEATURE to be defined...
SO I will edit right and left to... wait a minute... stop... think... observe... read it all..

The servo should never have moved at all since deploy_z_probe()implies AUTO_BED_LEVELING_FEATURE

Sorry, this is too much for me... Do I face a preprocessor error??

Now, to avoid compiling that, would it be a good idea to put in sanitycheck.h something such as #error on defined (HAS_Z_ENDSTOP_SERVO) and disabled(AUTO_BED_LEVELING_FEATURE) (using the correct syntax that I have not figured yet)

@thinkyhead , you have always been supportive, what do you think? ( I want to learn). ( I am an old engineeer, but not a C or C++ programmer, I wrote mickey mouse program on PDP 11 and Vax if that tells you some and some Z80 asm code)

That said, I would really appreciate to see my name associate with the solution.. I spend quite a few hours frustration on this one... I will probably NOT touch my computer before June, and I feel we should NOT leave that like that.... Unless I missed something obvious that I should know.

[Roxy-3D]

the following that must have been written by @Roxy-3DPrintBoard

Nope! You will have to look at the commit logs to find who did make that code. But it wasn't me.

[lrpirlet]

Ooops @Roxy-3DPrintBoard, I did not mean to finger point...

I just imagined you were (I konw, I should have verified instead of remembering Richard telling me to close one of my 3 PR I wrote cause you were involved in the probe handling...
ref: https://github.com/MarlinFirmware/MarlinDev/pull/321#issuecomment-221579421)

You are one of those I learn a lot from, thanks to correct me.

[thinkyhead]

the following that must have been written by…

I can take credit for most of the "macro piles" in Marlin.

[thinkyhead]

So… Yes, the SERVO_LEVELING flag does depend on AUTO_BED_LEVELING_FEATURE, so it seems it shouldn't be applicable here in the homing routine. But I believe that for the most part Marlin doesn't wholly support a Z servo probe in any other case (you must use auto bed leveling if you are using a Z servo probe). In the future this might change, so we'll be able to use a Z servo probe for homing even without any bed leveling. For now this dependency isn't a big deal.

[thinkyhead]

The servo should never have moved at all since deploy_z_probe() implies AUTO_BED_LEVELING_FEATURE

To sort this out, it will help to enable DEBUG_LEVELING_FEATURE and re-flash the firmware. Then before you do any homing or leveling, give the command M111 S255 to enable all the extra logging features. Then when you do homing, Marlin will give some extra output that might point to the cause.

[Blue-Marlin]

In homeaxis() we have direct servo commands when HAS_SERVO_ENDSTOPS is defined. AUTO_BED_LEVELING_FEATURE is not involved. HAS_SERVO_ENDSTOPS has no z lift, what is likely the problem.

HAS_SERVO_ENDSTOPS and SERVO_LEVELING and all the other probe definitions should go away and be replaced by something like USE_PROBE_FOR_HOMING in homeaxis(). All differentiating between the different probes should be done in deploy_z_probe() and stow_z_probe(). (Do i sound like a broken record?)

[lrpirlet]

@thinkyhead Please have a look in the logs directory in my https://github.com/lrpirlet/Marlin/tree/servo_swing_without_raise_reset_arduino branch...

The logs directory shows the Arduino compile full verbose ( no mention around a macro expansion warning in Marlin_main.cpp)

Yet the repetier logs shows the servo deploy and retract in G28 (it should not)

The logs also contain a diff to facilitate comparison between the RCBugFix and my servo_swing_without_raise_reset_arduino branch

[thinkyhead]

Please have a look in the logs directory…

@lrpirlet I will take a look tomorrow. I have to get some sleep now!

(Do i sound like a broken record?)

@Blue-Marlin We all sound like broken records until someone makes a Pull Request.

[Blue-Marlin]

@thinkyhead
The change is too big for a RC.

[Blue-Marlin]

@lrpirlet
I'd try something like

@@ -2319,10 +2319,18 @@ static void homeaxis(AxisEnum axis) {
     #if HAS_SERVO_ENDSTOPS
       if (_Z_SERVO_TEST && servo_endstop_id[axis] >= 0) {
         #if ENABLED(DEBUG_LEVELING_FEATURE)
           if (DEBUGGING(LEVELING)) SERIAL_ECHOLNPGM("> SERVO_ENDSTOPS > Stow with servo.move()");
         #endif
+
+        #ifndef Z_RAISE_AFTER_PROBING
+          #define Z_RAISE_AFTER_PROBING 15 // your number
+        #endif
+        current_position[Z_AXIS] = Z_RAISE_AFTER_PROBING;
+        line_to_current_position();
+        stepper.synchronize();
+
         servo[servo_endstop_id[axis]].move(servo_endstop_angle[axis][1]);
         if (_Z_PROBE_SUBTEST) endstops.enable_z_probe(false);
       }
     #endif

[thinkyhead]

The change is too big for a RC.

That's okay. We've put up Pull Requests that are not going into 1.1.0 but are being held until 1.1.1. There won't be too many changes before 1.1.0 is released. So, feel free to submit any PR that addresses extant issues. It will give us extra time to do cleanup and testing, as well.

[lrpirlet]

@Blue-Marlin Thanks, I'll try that tonight, if I can spare one hour, and report..

[lrpirlet]

@Blue-Marlin Many thanks... I now realize that preprocessor is not at fault... I did not realize that the stow was issued from Inside the homeaxis..

This is what I did... I did add the feedrate as the Z axis went to a real slow move... about 1mm per minute or so (needless to say it was too long for me to wait)...

@@ -2326,6 +2321,15 @@ static void homeaxis(AxisEnum axis) {
         #if ENABLED(DEBUG_LEVELING_FEATURE)
           if (DEBUGGING(LEVELING)) SERIAL_ECHOLNPGM("> SERVO_ENDSTOPS > Stow with servo.move()");
         #endif
+
+        #ifndef Z_RAISE_AFTER_PROBING
+          #define Z_RAISE_AFTER_PROBING 5 // your number
+        #endif
+        current_position[Z_AXIS] = Z_RAISE_AFTER_PROBING;
+        feedrate = homing_feedrate[Z_AXIS];
+        line_to_current_position();
+        stepper.synchronize();
+
         servo[servo_endstop_id[axis]].move(servo_endstop_angle[axis][1]);
         if (_Z_PROBE_SUBTEST) endstops.enable_z_probe(false);
       }

More info available in https://github.com/lrpirlet/Marlin/tree/servo_swing_without_raise_reset_arduino logs directory.
Want me to write the PR?? or is that a work-around that will be replaced by a bigger change??

[Blue-Marlin]

@lrpirlet
I'd prefer when you'd make the PR and i could continue with the bigger solution. That will last some day or more. Until the bigger solution will be in Marlin, what can last month, i'd like to see your fix.

[lrpirlet]

@Blue-Marlin ok, I'll do it, be warned, it may take me a few days as I can't give all the time I like...

[jbrazio]

Thanks @lrpirlet & @Blue-Marlin.

[lrpirlet]

@thinkyhead @Blue-Marlin @jbrazio I did submit a PR in #3942

[Roxy-3D]

the following that must have been written by @Roxy-3DPrintBoard

Nope! You will have to look at the commit logs to find who did make that code. But it wasn't me.

Ooops @Roxy-3DPrintBoard, I did not mean to finger point...

I need to be more careful how I word things. I was not trying to avoid being blamed for something. I was trying not to take credit for all the work somebody else did.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.