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

Functionality for pulsed servo/endstops like BLTouch #5650

Closed
1stsetup opened this issue Jan 2, 2017 · 19 comments
Closed

Functionality for pulsed servo/endstops like BLTouch #5650

1stsetup opened this issue Jan 2, 2017 · 19 comments

Comments

@1stsetup
Copy link

1stsetup commented Jan 2, 2017

Hi this is just first of all an informational request. I recently installed a BLtouch sensor and it went into error state when I ran G28 for the first time and it did the second zaxis (bed), bump, move.

It looks like there is not specific code in Marlin to handle, what I call, pulsed endstops like the bltouch. Only code for switched endstops.
So my questions would be: Do want/need code which can handle pulsed type endstops? Or should the current code be able to handle this?
Or is the code buggy and does not detect endstop changes when steppers are moving below a certain speed?

This is what I see and can reproduce on my printer when I issue a G28 (or later G29).
X and Y axis home ok. The it does the Z homing.
So the first touch by the zaxis (bed) is ok, Fast run, it drops the ZAXIS (bed) and does the bump which moves the ZAXIS (bed) closer to the bltouch sensor but with a slower speed specified by "HOMING_BUMP_DIVISOR {2, 2, 4}". It hits the BLTouch probe but the bltouch drops the pin before the code is reached where the bltouch is stowed.

I got it working when I lowered the HOMING_BUMP_DIVISOR for the zaxis to 2 ({2, 2, 2}). I also tried three but it did not always work. Most of the times the bltouch goes into error state.

The bltouch sensor is not an regular endstop like mechanical switches, optical switches or inductive switches in that when once triggered it does not stay triggered. The bltouch when the pin is pushed and the bltouch firmware detects it the bltouch pulls up its pin. It brings its signal line up for 5ms and then drops the signal and its pin. When the pin hits anything in its way it will go into error state.

In the code (Marlin_main.cpp function do_homing_move) there is a part where it does a stow of the bltouch once the move is done/interrupted. But I think this part is hit to late when the bed is moving slow (high divisor).

  stepper.synchronize();

  #if HOMING_Z_WITH_PROBE && ENABLED(BLTOUCH)
    if (deploy_bltouch) set_bltouch_deployed(false);
  #endif

  endstops.hit_on_purpose();

I went over the code and added debugging lines at different places to see if the bltouch signal is detected ok and what happens. I have defined ENDSTOP_INTERRUPTS_FEATURE as it works on my RAMPS 1.4 but the problem is the same without this feature enabled.
I can see the signal interrupt is detected on the second slower move when the bed pushes the bltouch pin, e_hit == 2. I did not debug if the endstop state is stored ok. will do this tonight and report back.

I am thinking of multiple different solutions which I can try to implement but I would first like to know:
a: is someone already working on this?
b: if no one is working on it should we change the endstops.cpp functionality for the bltouch. So it stows the pin once the triggered?
c: The stowing can also be done in the stepper.cpp ISR function but I think this is not the right place.
d. something else.

We need to remember that the pin on the bltouch has a double function. It is a servo and a endstop in one. Only the servo is write only and the endstop pulses once when triggered.

Any help and info is appreciated. I can spent some time on implementing/designing code but I am still learning the internals of Marlin.

Regards Michel.

@1stsetup 1stsetup changed the title Functionality for pulsed endstops like BLTouch Functionality for pulsed servo/endstops like BLTouch Jan 2, 2017
@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jan 2, 2017

At least some of us are aware of the problem and it's roots (#5234 (comment) and following).

As far as i know no one is working on this.

Possible solutions:

  • Ensure the slow probing speed is above 3steps/5ms - for example in a sanity check for BLtouch. This is simple ans efficient, but might be a bit fast for DELTAs (~80steps/mm => ~7.5mm/s)
  • Dropping the debouncer would decrease the minimal speed but not eliminate it. (2steps/5ms).
  • When we are slow we have few steps/s -> have few stepper interrups/s -> much time in the main-loop. We could check endstops in idle() when stepping slow.
  • If we have endstop interrups we could start a timer in there - rechecking the BLtouch after ~2,5ms. (would need an other timer interrupt).
  • If we have endstop interrupps we could measure the time between rising an falling edge of the BLtouch pin. If about 5ms it was triggered by purpose.
  • ...

@1stsetup
Copy link
Author

1stsetup commented Jan 2, 2017

I got the timing problem fixed when using interrupts. (#define ENDSTOP_INTERRUPTS_FEATURE)

Following works on my machine: CoreXY, using RC branch updated a couple minutes ago.

diff --git a/Marlin/endstop_interrupts.h b/Marlin/endstop_interrupts.h
index 495a758..58d6d5a 100644
--- a/Marlin/endstop_interrupts.h
+++ b/Marlin/endstop_interrupts.h
@@ -75,6 +75,11 @@
 volatile uint8_t e_hit = 0; // Different from 0 when the endstops shall be tested in detail.
                             // Must be reset to 0 by the test function when the tests are finished.
 
+#if ENABLED(BLTOUCH)
+  volatile uint8_t bl_hit = 0; // Different from 0 when the endstops shall be tested in detail.
+                               // Must be reset to 0 by the test function when the tests are finished.
+#endif
+
 // Install Pin change interrupt for a pin. Can be called multiple times.
 void pciSetup(byte pin) {
   *digitalPinToPCMSK(pin) |= bit (digitalPinToPCMSKbit(pin));  // enable pin
@@ -87,6 +92,15 @@ FORCE_INLINE void endstop_ISR_worker( void ) {
   e_hit = 2; // Because the detection of a e-stop hit has a 1 step debouncer it has to be called at least twice.
 }
 
+#if ENABLED(BLTOUCH)
+  FORCE_INLINE void endstop_ISR_worker_bltouch( void ) {
+    bl_hit = 1;
+  }
+
+  // Use one Routine to handle bltouch
+  void endstop_ISR_bltouch(void) { endstop_ISR_worker_bltouch(); }
+#endif
+
 // Use one Routine to handle each group
 // One ISR for all EXT-Interrupts
 void endstop_ISR(void) { endstop_ISR_worker(); }
@@ -162,7 +176,11 @@ void setup_endstop_interrupts( void ) {
 
   #if HAS_Z_MIN
     #if (digitalPinToInterrupt(Z_MIN_PIN) != NOT_AN_INTERRUPT)
-      attachInterrupt(digitalPinToInterrupt(Z_MIN_PIN), endstop_ISR, CHANGE);
+      #if ENABLED(BLTOUCH)
+        attachInterrupt(digitalPinToInterrupt(Z_MIN_PIN), endstop_ISR_bltouch, RISING);
+      #else
+        attachInterrupt(digitalPinToInterrupt(Z_MIN_PIN), endstop_ISR, CHANGE);
+      #endif
     #else
       // Not all used endstop/probe -pins can raise interrupts. Please deactivate ENDSTOP_INTERRUPTS or change the pin configuration!
       static_assert(digitalPinToPCICR(Z_MIN_PIN) != NULL, "Z_MIN_PIN is not interrupt-capable");
@@ -192,7 +210,11 @@ void setup_endstop_interrupts( void ) {
 
   #if HAS_Z_MIN_PROBE_PIN
     #if (digitalPinToInterrupt(Z_MIN_PROBE_PIN) != NOT_AN_INTERRUPT)
-      attachInterrupt(digitalPinToInterrupt(Z_MIN_PROBE_PIN), endstop_ISR, CHANGE);
+      #if ENABLED(BLTOUCH)
+        attachInterrupt(digitalPinToInterrupt(Z_MIN_PROBE_PIN), endstop_ISR_bltouch, RISING);
+      #else
+        attachInterrupt(digitalPinToInterrupt(Z_MIN_PROBE_PIN), endstop_ISR, CHANGE);
+      #endif
     #else
       // Not all used endstop/probe -pins can raise interrupts. Please deactivate ENDSTOP_INTERRUPTS or change the pin configuration!
       static_assert(digitalPinToPCICR(Z_MIN_PROBE_PIN) != NULL, "Z_MIN_PROBE_PIN is not interrupt-capable");
diff --git a/Marlin/stepper.cpp b/Marlin/stepper.cpp
index 82289a5..ae53d3b 100644
--- a/Marlin/stepper.cpp
+++ b/Marlin/stepper.cpp
@@ -312,6 +312,9 @@ void Stepper::set_directions() {
 
 #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
   extern volatile uint8_t e_hit;
+  #if ENABLED(BLTOUCH)
+    extern volatile uint8_t bl_hit;
+  #endif
 #endif
 
 /**
@@ -413,13 +416,41 @@ void Stepper::isr() {
     #endif
     )
     #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
-      && e_hit
+      && (e_hit
+      #if ENABLED(BLTOUCH)
+        || bl_hit
+      #endif
+      )
     #endif
   ) {
     endstops.update();
 
     #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
-      e_hit--;
+      #if ENABLED(BLTOUCH)
+        if (bl_hit) {
+          #if ENABLED(DEBUG_LEVELING_FEATURE)
+            if (DEBUGGING(LEVELING)) {
+              SERIAL_ECHOPAIR("stepper.ISR() bl_hit(", bl_hit);
+              SERIAL_CHAR(')');
+              SERIAL_EOL;
+            }
+          #endif
+          if (motor_direction(Z_AXIS)) {
+            endstop_triggered(Z_AXIS);
+            bl_hit--;
+          }
+        }
+        else {
+          #if ENABLED(DEBUG_LEVELING_FEATURE)
+            if (DEBUGGING(LEVELING)) {
+              SERIAL_ECHOPAIR("e_hit(", e_hit);
+              SERIAL_CHAR(')');
+              SERIAL_EOL;
+            }
+          #endif
+          e_hit--;
+        }
+      #endif
     #endif
   }
 
@@ -1122,6 +1153,14 @@ void Stepper::endstop_triggered(AxisEnum axis) {
 
   #endif // !COREXY && !COREXZ && !COREYZ
 
+      #if ENABLED(DEBUG_LEVELING_FEATURE)
+        if (DEBUGGING(LEVELING)) {
+          SERIAL_ECHOPAIR("endstop_triggered(", axis);
+          SERIAL_CHAR(')');
+          SERIAL_EOL;
+        }
+      #endif
+
   kill_current_block();
 }
 

I will also comment in the other issue #5234 as with the above code change M48 works ok on my setup. I will ask people to test when possible.
bltouch_endstopfix.diff.txt

@Roxy-3D
Copy link
Member

Roxy-3D commented Jan 2, 2017

@1stsetup May I ask what type of board you checked this out on? I'm hoping it was a RAMPS v1.4 board because that is what I have on all of my printers.

Either way... I'll load it up and give it a check in the next few days!

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jan 2, 2017

     #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
-      e_hit--;
+      #if ENABLED(BLTOUCH)
+        if (bl_hit) {
+          #if ENABLED(DEBUG_LEVELING_FEATURE)
+            if (DEBUGGING(LEVELING)) {
+              SERIAL_ECHOPAIR("stepper.ISR() bl_hit(", bl_hit);
+              SERIAL_CHAR(')');
+              SERIAL_EOL;
+            }
+          #endif
+          if (motor_direction(Z_AXIS)) {
+            endstop_triggered(Z_AXIS);
+            bl_hit--;
+          }
+        }
+        else {
+          #if ENABLED(DEBUG_LEVELING_FEATURE)
+            if (DEBUGGING(LEVELING)) {
+              SERIAL_ECHOPAIR("e_hit(", e_hit);
+              SERIAL_CHAR(')');
+              SERIAL_EOL;
+            }
+          #endif
+          e_hit--;
+        }
+      #endif
     #endif

Will never decrease e_hit if BLTOUCH is not enabled.

      }
+   #else
+     e_hit--;
    #endif

If bltouch-pin is not on an extenal interrupt but on an Pin-Change_Interrupt this will not work. Means - for RAMPS on z-min and z-max only.

endstop_ISR_worker_bltouch() is used only once - so could replace its call.

So you removed all debouncing for BLtouch.
This will trigger a second time when the bl-signal ends?
This will trigger when bl-touch goes into or leaves error state.
This will trigger always when the signal changes its state - maybe a bit too often - we'll see.

@1stsetup
Copy link
Author

1stsetup commented Jan 3, 2017

@Blue-Marlin

Thank you for your feedback.

Means - for RAMPS on z-min and z-max only.

For now I only have a RAMPS to test on. So unable to develop and test for others boards. As I am somewhat of a newbie to the Marlin code I hope others can help for other setups.

About: cpp endstop_ISR_worker_bltouch() I created this part in the same way as the "old" existing code does it. I can change it into a single version.

I do not think the bltouch will show debounce behaviour. From what i read debounce happens mostly on mechanical contact switches Arduino debounce example, Whatis debounce description, Labbooks debounce description and the bltouch is a digitized switch. The ATTiny chip on the bltouch generates the signal and not the pin itself so there is already logic in between the pin being pushed and the signal line changing. I think/hope the debouncing is already handled in the bltouch firmware.

This will trigger a second time when the bl-signal ends?

How do you mean? I added the interrupt only when the signal line goes high (RISING) and not as other switches do when the signal changes (CHANGE).

This will trigger when bl-touch goes into or leaves error state.

I think only when goes into error state as the signal line goes high. When the bltouch goes into error state I think we want zaxis movement to stop!?
See previous comment on leaving.

I created a new diff which will show normal behaviour when BLTOUCH disabled. I had this part first in my code but cleaned and compressed a little bit to radically.
bltouch_endstopfixv2.diff.txt

@1stsetup
Copy link
Author

1stsetup commented Jan 3, 2017

@Roxy-3D : I have a RAMPS v1.4 board so I think it should work also for you.
Please use my latest version "bltouch_endstopfixv2.diff.txt".
The diff is against the RC branch.

@Roxy-3D
Copy link
Member

Roxy-3D commented Jan 3, 2017

I do not think the bltouch will show de-bounce behaviour.
...
I think/hope the de-bouncing is already handled in the bltouch firmware.

I can confirm the BL-Touch signal is 'de-bounced'. However, I also want to caution people that a new BL-Touch mode will be showing up soon. Very soon, by setting the servo angle at 60 degrees, you will be able to see the current state of the pin. It will not have a pulse anymore at that servo angle, and probably we are going to want to use that mode if available.

The point I'm trying to make is in the new mode, it is possible the pin is not de-bounced. There would still be an AVR Tiny in between the mechanical signal and the value that gets reported. But I'm not sure if they will build any hysteresis into the reported value.

PS. VERY NICE WORK @1stsetup !!!!

@1stsetup
Copy link
Author

1stsetup commented Jan 4, 2017

I also got it working now without interrupts.

Please test and report back if this works ok?
bltouch_endstopfixv3.diff.txt

@Roxy-3D
Copy link
Member

Roxy-3D commented Jan 4, 2017

@1stsetup I'm ready to test this. But this diff is dependent on some other changes. Can you spell out what else I need to load? Specifically, you have references to

ENDSTOP_INTERRUPTS_FEATURE

and that isn't defined any where in your code block. It would be good if you have the BL-Touch working without interrupts. But I wanted to test it both ways and I probably want my printer to use the interrupts.

What else besides your code do I need to load?

@1stsetup
Copy link
Author

1stsetup commented Jan 5, 2017

@Roxy-3D The ENDSTOP_INTERRUPTS_FEATURE is defined in the Configuration.h file.

Also enable the BLTouch in the normal way for branch RC in your Configuration.h file. I have the following:

//#define ENDSTOPPULLUPS

// Enable this feature if all enabled endstop pins are interrupt-capable.
// This will remove the need to poll the interrupt pins, saving many CPU cycles.
//#define ENDSTOP_INTERRUPTS_FEATURE

#define BLTOUCH
#define Z_PROBE_SPEED_SLOW (Z_PROBE_SPEED_FAST / 8) // Play with the divisor for changing slow speed. Used in M48
#define PROBE_DOUBLE_TOUCH
#define Z_MIN_PROBE_REPEATABILITY_TEST
#define DEBUG_LEVELING_FEATURE
#define Z_SAFE_HOMING
#define NUM_SERVOS 1

In file 'Configuration_adv.h':

#define Z_HOME_BUMP_MM 5  // I wanted a bit more distance. have not tested with default 2 mm value
#define HOMING_BUMP_DIVISOR {2, 2, 8} // Play with the last digit, zaxis divisor, for the changing slow speed. Used in G28

Plus the diff I posted and that should be it.
You toggle the interrupts in your Configuration.h file.

I have my bltouch connected to the RAMPS v1.4 in the advised way. So the three pin to servo 1 (SER1) and the two pin to GND and SIGNAL of ZMIN. (image)

@1stsetup
Copy link
Author

1stsetup commented Jan 5, 2017

See what I use have a look at my branches RCBugFix-bltouch-endstopdetect or bltouch-endstopdetect. It also contains the configuration for my printer.

@Roxy-3D
Copy link
Member

Roxy-3D commented Jan 5, 2017

@1stsetup Thank You for posting your settings. I'm going to start with those and then migrate to more reasonable settings for my printer.

@gcormier
Copy link
Contributor

gcormier commented Jan 6, 2017

@Roxy-3D You've obtained a BLTouch right?If not, I can mail one to you. Or any other primary Marlin developer. ( @thinkyhead ?)

@djuniversal
Copy link

Got a cartesian printer running rcbugfix. Just installed your bltouch branch and G29 worked on the second run! Finally! Thanks for your work. I got good M48 results too just got to play with the settings to see if I can run it a bit faster and I will report in.

@Roxy-3D
Copy link
Member

Roxy-3D commented Jan 8, 2017

You've obtained a BLTouch right?If not, I can mail one to you. Or any other primary Marlin developer.

Yes, both myself and ThinkyHead have BL-Touch probes to assist the debug effort. What I need is more time in the day. I don't suppose you can send me any of that?

@Bob-the-Kuhn
Copy link
Contributor

FYI - the last couple of days I've been playing with my BLTouch and a logic analyzer. There's a small debounce built into the endstop detect logic. It takes two stepper ISR runs before Marlin will declare an endstop as active. The result is you need three stepper ISR runs to happen within the 5mS BLTouch pulse to have a reliable system. This assumes the BLTouch itself is reliable.

On my machine I need a minimum feed rate of 121 mm/minute. I had my HOMING_FEEDRATE_Z set to 240 with a divisor of two for the double touch. That probably explains why my machine worked 99% of the time.

The formula for minimum feed rate is : 60 / ( DEFAULT_AXIS_STEPS_PER_UNIT * 2.5mS )

I was in the process of creating a test to verify this but the mount for my BLTouch broke. With any luck I'll be able to report results tomorrow.

@djuniversal
Copy link

OK. I have done some playing. I was being super conservative to try and get maximum reliability before I played with the speed but I was still getting failures. I read @Bob-the-Kuhn post above and decided to keep the homing speed up a bit because I have a fairly low Z axis step count (400). I used low Z clearance to get the speed going, 3mm is enough to clear the extended pin and a bit of slop.

I used @1stsetup config file initially and changed it as below

Configuration.h
// X and Y axis travel speed (mm/m) between probes
#define XY_PROBE_SPEED 8000
// Speed for the first approach when double-probing (with PROBE_DOUBLE_TOUCH)
#define Z_PROBE_SPEED_FAST HOMING_FEEDRATE_Z
// Speed for the "accurate" probe of each point
#define Z_PROBE_SPEED_SLOW (Z_PROBE_SPEED_FAST / 8) --> 4)

#define Z_CLEARANCE_DEPLOY_PROBE 10 -->5
// Z Clearance for Deploy/Stow
#define Z_CLEARANCE_BETWEEN_PROBES 10 -->3
// Z Clearance between probe points

Configuration_adv.h
#define Z_HOME_BUMP_MM 5 -->3
#define HOMING_BUMP_DIVISOR {2, 2, 8}

I was getting a failure every 4-5 G28 or G29 commands with @1stsetup setup. I have now done 20x G29 commands and 10x G28 > G29 and 3x M48 P50 without any fail alarms.

I get a good fast 16 point G29 execution and good repeat-ability. Hopefully that helps the conversation a bit. I am going to start printing now and see if I see good results in my first layers.

@djuniversal
Copy link

Good first layers on ABS and PLA. Been working like a charm 5 prints later

dot-bob added a commit to dot-bob/Marlin that referenced this issue Feb 1, 2017
- Improved display responsiveness.
- Changed default speed and acceleration settings to make them more conservative to prevent stepper skipping.
- Included a bug fix for BLTouch see: MarlinFirmware#5650
- Changed menu item selection from hollow box to inverted box.
- Added a confirm to stop print when printing from SD card.
- Fixed a bug in the stop print function by adding code to shutdown the hotend and heatbed.
- Changed the menu beep duration from 100ms to 10ms.
- Default to single bed probing when auto leveling with G29 (double touch is a waste of time see: https://goo.gl/fzHGji).
- Updated GCode scripts to keep print head in bounds (some users can't take the print head to a negitive location).
- Updated Conditionals_post.h so the #define XXXXXX_PROBE_BED_POSITION statements are no longer needed
@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants