-
Notifications
You must be signed in to change notification settings - Fork 298
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
incorrect input pin state after glitch #470
Comments
Interesting. The latest development code (in the @giseburt Thoughts? |
Indeed, Justin, that was one reason for the refactor. That problem should be resolved in the edge-preview branch, along with other substantial improvements in GPIO.
|
Awesome. 😄 @lllars Any interest in compiling in the |
I hate to say I ran into this same problem a few months ago but didn't bother to address it. I simply manually touched the touch plate to the z-probe if I saw that it was out of sync (which got it back in sync) and then probed again. Looking at the change my concerns are that in->state is actually bouncing around following the actual state of the pin. gpio_read_input() and io_get_input() asynchronously read in->state and thus could get a "random" value depending on when called. I think this is probably benign for io_get_input, and it looks like gpio_read_input is only called when the machine is in a static state. So for current uses this change could be okay. However, the probing, homing, and subsequent actions will still miss reacting to very short pin transitions and fail to take appropriate action. This is probably no worse that what is happening today. I do like the simplicity of the fix. I had a more elaborate / complete fix in mind but didn't get to it. The plan I had was to start a retriggerable asynchronous timer for lockout_timer period of time whenever the pin state changes and once it expires call pin_changed() again to get the final resting state. This has the affect of stretching a pin transition for at least lockout_timer period of time, thus locking out other changes during that time. I've used this technique many times to debounce gpio pins on other projects. Unfortunately I'm not aware of any interrupt driven timer within the code so this wasn't straightforward to implement and I had to move on to my real but less fun job. |
I'm happy to test out edge-preview. Is there a summary anywhere of what changes it incorporates? @mhlong10 I was also thinking that checking the state of the pin after the lockout_timer expired would be a good solution. I just couldn't figure out an easy way to do that. |
I'm getting a lot of errors when trying to compile edge-preview. I'll post the output I'm seeing. Let me know if there's anything I can try on my end to fix things. I'm compiling with:
and the output is:
|
It's probably caused by something in the When I hit stuff like that, I generally first try to find a settings file that compiles ok or start with the default one ( So, try without giving any settings file on the command line, and see if that compiles successfully:
That should compile ok, and use the above mentioned default settings. 😄 If it does so, and the In theory. 😄 |
Thanks, good suggestion, but it didn't work. Output was identical.
|
Yep, you're right. Just tried it here, and same error. @giseburt Looks like |
Checking the build now.
Also re:
Unfortunately I'm not aware of any interrupt driven timer within the code so this wasn't straightforward to implement and I had to move on to my real but less fun job.
The homing and probing are all interrupt driven now. The debounce code was causing so much trouble because sometimes a probe would only briefly fire a pulse and the subset backoff would be fast enough that the next time we checked it wasn't triggered anymore.
See here for the handler: https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L132-L147 <https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L132-L147>
Here for the setup (registration for interrupt events): https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L361-L362 <https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L361-L362>
And here for the teardown (reregistration): https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L295-L296 <https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L295-L296>
The interrupt handling itself is in Motate, but in g2core it comes in here: https://github.com/synthetos/g2/blob/1e2d02c460e5f188f204edd455246c78eaf3e09e/g2core/gpio.h#L486-L525 <https://github.com/synthetos/g2/blob/1e2d02c460e5f188f204edd455246c78eaf3e09e/g2core/gpio.h#L486-L525>
There is a lockout timer, so multiple interrupts are not fired for the same pin in a brief period. Note that we're not glitch filtering, but instead ignoring anything after the initial event for a brief amount of time, trusting that the initial event was genuine.
If there are noise spikes on the lines causing glitches then you'll need to filter it in hardware. Luckily this is fairly simple.
…-Rob
On Jun 21, 2020, at 5:47 PM, Justin Clift ***@***.***> wrote:
Yep, you're right. Just tried it here, and same error.
@giseburt <https://github.com/giseburt> Looks like edge-preview has become busted. Something about HAS_HOBBY_SERVO_MOTOR maybe?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#470 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6HUPPOANVP7WSQMAFGFLRX2EW3ANCNFSM4OB4UY2A>.
|
Ok, that should be fixed in the latest commit.
I tested with compiling for: make CONFIG=MiniMillgShield
Which is defined here: https://github.com/synthetos/g2/blob/404106b75e95a3a12c981d2e1ff42ad626c3e015/g2core/boards.mk#L75-L81 <https://github.com/synthetos/g2/blob/404106b75e95a3a12c981d2e1ff42ad626c3e015/g2core/boards.mk#L75-L81>
…-Rob
On Jun 21, 2020, at 6:33 PM, Rob ***@***.***> wrote:
Checking the build now.
Also re:
> Unfortunately I'm not aware of any interrupt driven timer within the code so this wasn't straightforward to implement and I had to move on to my real but less fun job.
>
The homing and probing are all interrupt driven now. The debounce code was causing so much trouble because sometimes a probe would only briefly fire a pulse and the subset backoff would be fast enough that the next time we checked it wasn't triggered anymore.
See here for the handler: https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L132-L147 <https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L132-L147>
Here for the setup (registration for interrupt events): https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L361-L362 <https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L361-L362>
And here for the teardown (reregistration): https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L295-L296 <https://github.com/synthetos/g2/blob/d6ae0db69db2e5e6f02294a599f1318dd967817a/g2core/cycle_probing.cpp#L295-L296>
The interrupt handling itself is in Motate, but in g2core it comes in here: https://github.com/synthetos/g2/blob/1e2d02c460e5f188f204edd455246c78eaf3e09e/g2core/gpio.h#L486-L525 <https://github.com/synthetos/g2/blob/1e2d02c460e5f188f204edd455246c78eaf3e09e/g2core/gpio.h#L486-L525>
There is a lockout timer, so multiple interrupts are not fired for the same pin in a brief period. Note that we're not glitch filtering, but instead ignoring anything after the initial event for a brief amount of time, trusting that the initial event was genuine.
If there are noise spikes on the lines causing glitches then you'll need to filter it in hardware. Luckily this is fairly simple.
-Rob
> On Jun 21, 2020, at 5:47 PM, Justin Clift ***@***.*** ***@***.***>> wrote:
>
>
> Yep, you're right. Just tried it here, and same error.
>
> @giseburt <https://github.com/giseburt> Looks like edge-preview has become busted. Something about HAS_HOBBY_SERVO_MOTOR maybe?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <#470 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6HUPPOANVP7WSQMAFGFLRX2EW3ANCNFSM4OB4UY2A>.
>
|
I still have many many errors, even trying the same "make CONFIG=MiniMillgShield" line. The bit about HAS_HOBBY_SERVO_MOTOR is resolved, but I'm still seeing all the config.h, planner.h, and gpio.h errors. |
Adding I'm still stumped on the planner.h errors though. |
Just tried now, and for me it worked. @lllars Did you run a eg:
|
@justinclift Thanks, that worked. Note that I did still run into a couple errors involving settings_default.h when I compiled with It's probably too late for today, but I will try this out on my machine tomorrow. |
I've now tried out edge-preview on my machine and am happy to report that the input pin state remains correct following a glitch. I was able to home the machine without problems. |
Whoo Hooo! Sounds like good progress. 😄 |
Current HEAD didn't compile with gShield. Not with default settings nor MiniMillgShield. Took commit 490eb6c based on dates in this discussion and it did compile with MiniMill. Modified default settings with my machines parameters and taking missing stuff from MiniMill until it compiled. Tested probing with CNCjs + kreso autolevel. Single probing worked fine. Autolevel on the other hand crashes in different ways after 1 or 3 probes. Sometimes it just hangs with Using DUE. |
Linking some comments about the issue and recent changes here to help anyone googling themselves here. |
Did some digging but the source as a whole just looks too complicated to get a quick understanding what happens and where. I was planning to fix this by making sure input state is updated after lockout but no idea where I should be polling that / where's the callback or... This is a real bummer. I switched from a crappy Chinese controller to G2 in hope of getting auto leveling for PCB milling and kinda took it for granted that stuff like this surely works in G2 and thought a lot of people are using if for this. Reading that this isn't fixable with simple HW filtering, I guess my quick options are to go with GRBL or buy PlanetCNC controller + SW. |
I am using probing heavily as I use it at least once for every job. Bantam tools and ShopBot are also using it regularly, as they're the ones that collectively found most of the bugs I've mentioned recently. The only part of that that I don't use and can't say works well is CNCjs with an auto If you have a log file or something with a few notes about what's happening at what points along the log we can maybe help sort it out. |
@willmackey2nd That kreso autolevel extension for CNCjs looks nifty. If we can get it working decently well with g2, that'd be useful. 😄 |
@justinclift yeah it's simple and works and also only alternative for me for now AFAIK. I don't know any better alternatives to use with G2 besides CNCjs. Feel free to slap me in face and point me to a better gcode feeder. @giseburt Kreso AL works so that it sends the whole measurement task at once to CNCjs which then starts to execute the gcode. AL then just waits for probe status data, taking notes. Just means that AL itself shouldn't have anything todo with the motion control during probing. Single probing commands haven't failed once for me but grid probing with AL seems unreliable. I've done now ~30 tests and
I don't know if this helps in any way but here's the whole console capture from AL cycle, probing 40x40 area with 7.5mm between probes. This was the kind of fail where it actually told something in console. Somewhere in the middle it crashed Z, continued pushing down and then suddenly proceeded skipping several probe points. CNCjs Console log
|
I found a bug in the input pin debounce / lockout code. If an input pin changes, and then changes back before the lockout time has expired, then it's state is never updated to reflect that it changed back.
As an example, on my machine, I have hall effect limit switches which are normally high. I first power up g2core by connecting it to my PC. Then, when I power on the machine, the switch state very briefly goes low, only for a few ms, and then returns to high. g2core correctly detects that that the switches go low, but then never updates that they have gone back to high: $in continues to display the incorrect state. Because of this g2core then fails to detect the next time the switches go low, but it does then update to the correct state when the switch goes back to high. This bug causes my machine to crash when homing.
I've worked around this issue by changing pin_changed() in gpio.cpp. I moved the code to record the pin state change up, so that the state change is recorded even if the pin is locked out. The lockout check is still there and prevents any actions from occurring during lockout even though the state has changed.
I am on firmware version 0.99, build 101.3. Target board is an Arduino Due with no shield. This is a 3 axis mill with independent stepper drivers.
Here is my re-ordering of the relevant section of gpio.cpp:
The text was updated successfully, but these errors were encountered: