Closed lrpirlet closed 5 years ago
autonumous
commented
6 years ago you have ENDSTOP_NOISE_FILTER defined? possibly similar to the issue I posted, see thread https://github.com/MarlinFirmware/Marlin/issues/10986 (albeit bugfix-1.1.x, and bed moves the other way)
lrpirlet
commented
6 years ago @autonumous Thanks, with your input it works as expected…
It also means that the routine enabled by ENDSTOP_NOISE_FILTER is buggy...
This is the minimum modification needed to make it work as expected…
$ git diff 225b2546c
diff --git a/Marlin/Configuration.h b/Marlin/Configuration.h
index 33fb9df8e..55847ee25 100644
--- a/Marlin/Configuration.h
+++ b/Marlin/Configuration.h
@@ -576,7 +576,7 @@
* based on the Makerbot design, since they already include the 100nF capacitor.)
*/
//#define ENDSTOP_NOISE_FILTER lrp changed... it needs to put capacitor...
-#define ENDSTOP_NOISE_FILTER
+//#define ENDSTOP_NOISE_FILTER
//=============================================================================
//============================== Movement Settings ============================@thinkyhead My problem is gone, there is most probably an issue hidden with the setting of ENDSTOP_NOISE_FILTER...
I have not the skill (given the time I can spend) to debug this problem...
I have no problem testing…. If you want, I'll keep my branch open till closure of this bug report: https://github.com/lrpirlet/Marlin/tree/lrpv2
Just tell me.
ejtagle
commented
6 years ago @lrpirlet : ENDSTOP_NOISE_FILTER works as expected. It just waits until the endstop line stops changing and, only at that time, it uses the endstop value as the accepted (denoised) value. If disabling ENDSTOP_NOISE_FILTER makes it work, the problem is probably your endstop switch is broken or has a loose connection. So, it just gives an small pulse when the endstop is hit, instead of keeping the signal at its active state, as it should.
AnHardt
commented
6 years ago The other way around. If there is no second (slow) approach the endstop probably still reports triggered.
lrpirlet
commented
6 years ago Please disregard hardware failure, it worked fine before I went to bugfix-2...
It NOW works as expected after commenting //#define ENDSTOP_NOISE_FILTER
ejtagle
commented
6 years ago The previous implementation had a 2 sample noise filter enabled at all times. The new approach, if you comment out (ENDSTOP_NOISE_FILTER) has no filter at all, and, if you enable it, has a filter that waits 7 samples with the same value before accepting the new endstop state... Sampling is done every 1mS .. There is nothing wrong disabling the noise filter. In fact, that is what we encourage to do
autonumous
commented
6 years ago As previously mentioned in another thread (#10986), on my setup, I do see a change in Z behaviour when enabling ENDSTOP_NOISE_FILTER. Whether directly or indirectly, setting that option caused homing issues on the Z axis where the bed moved the wrong direction.
For me, it was easier to fix the issue that required me to use that option (by adding a capacitor to the Endstop) which was only introduced around May 21st
thinkyhead
commented
6 years ago @ejtagle — The endstop noise filter requires 7 samples before it trusts that the endstop state has changed, and this counting only occurs in endstops.update, which is only called when the endstops are set as enabled. Therefore, if the endstop triggers or un-triggers and then the axis changes direction, it will require 7 samples (only while endstops are enabled) before it realizes the endstop state has changed.
So perhaps the endstop filter should be in effect at a lower level —at all times— and not just count-down when endstops are enabled.
ejtagle
commented
6 years ago Teoretically, when you enable ENDSTOP_NOISE_FILTER then it should start calling update() from the temperature ISR as well.. At least, that was the way i implemented it
thinkyhead
commented
6 years ago Only endstops.poll is called from temperature.cpp, not endstops.update, so endstops.update is still only called when endstops.enabled is true.
ejtagle
commented
6 years ago endstops.poll() calls update() (otherwise, a horrible bug would be there... )
thinkyhead
commented
6 years ago But endstops.update is only called when endstops.enabled is true. If endstops are disabled in-between moves towards the endstops, they will appear to still be triggered at the start of the second move.
ejtagle
commented
6 years ago Humm.. Then we have a problem. Maybe it should always be called ?
ejtagle
commented
6 years ago Humm... good question. Yes, this seems incorrect, but the periodic polling must be done periodically...
thinkyhead
commented
6 years ago The best solution to avoid side-effects would seem to be testing the endstops at all times and maintaining the "validated state" as a lowest-level value.
ejtagle
commented
6 years ago I was thinking on the purpose of ENDSTOPS_ENABLED
ejtagle
commented
6 years ago Yes, the idea is to always poll... or at least, when the endstop_poll_count > 0
ejtagle
commented
6 years ago Please, correct me if i am wrong @thinkyhead : The purpose of ENDSTOPS_ENABLED is just to filter out the reporting of endstops being hit - If that is the case, then we can modify easily the endstops::update() function to keep polling and only report if endstops are enabled
thinkyhead
commented
6 years ago Correct. The ENDSTOPS_ENABLED test tells the stepper ISR that if an endstop or probe is triggered then the current move should abort.
lrpirlet
commented
6 years ago Do you want me to test anything?
ejtagle
commented
6 years ago Let me check your config file...
ejtagle
commented
6 years ago @lrpirlet : Could you try increasing Z_HOME_BUMP_MM to 5 , and enable the ENDSTOP_NOISE_FILTER ?
The default 2mm bump for Z and the default speed of 4mm/s should give 0.5seconds for the bump, and that is 500 endstop samples ... unless the endstop code is disabled while performing the sampling... But i think it is not...
lrpirlet
commented
6 years ago @ejtagle @thinkyhead
Please note that I had to edit Marlin/src/module/endstops.h to remove a duplicate declaration that made PlatformIO to stop on error…
git diff shows
diff --git a/Marlin/src/module/endstops.h b/Marlin/src/module/endstops.h
index 830b1515b..7b41977bf 100644
--- a/Marlin/src/module/endstops.h
+++ b/Marlin/src/module/endstops.h
@@ -73,7 +73,7 @@ class Endstops {
#if ENABLED(ENDSTOP_NOISE_FILTER)
static esbits_t validated_live_state;
- uint8_t Endstops::endstop_poll_count;
+ // uint8_t Endstops::endstop_poll_count;
static uint8_t endstop_poll_count; // Countdown from threshold for polling
#endifTested as per request with ENDSTOP_NOISE_FILTER enabled and Z_HOME_BUMP_MM set to 5…
SAME BEHAVIOUR : it fails to execute a second bump….
Also, a subsequent G30 fails as it does not touch the bed
lrpirlet
commented
6 years ago @thinkyhead @ejtagle Please note that I see the problem on ALL 3 AXIS: X, Y and Z...
ejtagle
commented
6 years ago @lrpirlet Could you try latest bugfix2.0.x with the following endstops.cpp file ?
ejtagle
commented
6 years ago @thinkyhead : I could be wrong here, but i think the already merged PR #11066 is not enough. If you read endstops::update(), you will notice the endstop state is ONLY read if steppers.axis_is_moving() returns true. That will only return true if the motors are moving, Something that will not happen when the move stops due to an endstop being hit. That causes all endstops, after 7 samples, to be reported as open...
lrpirlet
commented
6 years ago @ejtagle @thinkyhead
Thanks ejtagle, with the new endstops.cpp, it does work as expected…
Note that I set back the Z_HOME_BUMP_MM to 2 and it still continue working as expected… Note that G28, G30 and M48 now works fine…
I would have closed this entry… do you want me to raise a PR for the duplicated declaration in endstops.h? (see 4 entries above)
ejtagle
commented
6 years ago @lrpirlet: Donñt worry, i am raising a PR with both fixes (and an extra one to the Planner...) in a few hours... Thanks for confirming the fix works (i was not sure, as in my case the fix was not required)
thinkyhead
commented
6 years ago Seems we're still having some endstop issues. (e.g., #10986). Please test the latest code, doing multiple G28 commands, to see if homing is behaving the way it should 100% of the time.
lrpirlet
commented
6 years ago @thinkyhead I have rebased lrpv2 to current mainstream… (lrpv2 is now "hanging from" commit 8299ac121) I have run 12+ iteration of G28... NO PROBLEM at all (statistically that leaves a very low probability of an intermittent problem, yet still possible.) Please note that I do NOT use interrupt to detect endstop change, that I disable auto reporting of temperature and stop sending busy message while executing G28 (I have no added precision with interrupt but I risk my bed if interrupt is missed, I feel that Repetier is overdoing it by requesting a temperature reading every second, I see no use for those messages clutering the console)
Log is https://github.com/lrpirlet/Marlin/blob/lrpv2/Marlin/WhatIHaveDone.txt
sorry no time for text formating…. I need to drop now… tell me if you want more testing, I'll try my best.
thinkyhead
commented
6 years ago AUTO_REPORT_TEMPERATURES is a good option to quiet down the host. Repetier should notice that it's enabled and stop sending M105 commands. Then M155 can also be used to set a longer interval.
boelle
commented
5 years ago @lrpirlet has problem gone away in latest bugfix 2.0?
github-actions[bot]
commented
4 years ago 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.
Description
I decided to switch to the bugfix-2.0.x, and to use the Visual Studio Code + PlatformIO… I compiled the bugfix2.0.x with my config files (Mega 2560)… I did reinitialize the EEPROM…
Seems that G28 does NOT execute the "Home 2 Slow" move
Steps to Reproduce
Compile using my config files (see https://github.com/lrpirlet/Marlin/blob/lrpv2/Marlin/Configuration.h and https://github.com/lrpirlet/Marlin/blob/lrpv2/Marlin/Configuration_adv.h)
Expected behavior: [What you expect to happen] G28 does execute second bump...
Actual behavior: [What actually happens] G28 does touch the switch once, retract and forget to bump again...
Additional Information
Please find here the debug session… Note that the printer does NOT bump a second time after and that for X, Y and Z axis…
I may be doing something stupid, sorry if that is the case.
Log Output
``` 20:52:14.995 : Printer reset detected - initalizing 20:52:14.995 : start 20:52:14.995 : echo: External Reset 20:52:14.999 : Marlin bugfix-2.0.x 20:52:15.003 : echo: Last Updated: 2018-01-20 | Author: LRP 20:52:15.003 : echo:Compiled: Jun 17 2018 20:52:15.007 : echo: Free Memory: 2571 PlannerBufferBytes: 1488 20:52:15.031 : echo:V55 stored settings retrieved (655 bytes; crc 23719) 20:52:15.035 : Unified Bed Leveling System v1.01 inactive. 20:52:15.084 : Unified Bed Leveling initialized. 20:52:15.089 : UBL System reset() 20:52:15.093 : echo: G21 ; Units in mm (mm) 20:52:15.093 : echo: M149 C ; Units in Celsius 20:52:15.097 : echo:Filament settings: Disabled 20:52:15.097 : echo: M200 D3.00 20:52:15.101 : echo: M200 D0 20:52:15.101 : echo:Steps per unit: 20:52:15.105 : echo: M92 X80.00 Y80.00 Z4000.00 E402.00 20:52:15.109 : echo:Maximum feedrates (units/s): 20:52:15.113 : echo: M203 X300.00 Y300.00 Z3.00 E45.00 20:52:15.113 : echo:Maximum Acceleration (units/s2): 20:52:15.117 : echo: M201 X3000 Y3000 Z100 E10000 20:52:15.126 : echo:Acceleration (units/s2): P