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

No delay when resuming cycle with spindle off #605

Open
ademenev opened this issue Feb 11, 2019 · 28 comments
Open

No delay when resuming cycle with spindle off #605

ademenev opened this issue Feb 11, 2019 · 28 comments

Comments

@ademenev
Copy link

When Grbl is in Run state, sending '!' will switch it into Hol state. In this state it accepts 0x9E and stops the spindle. Now sending '~' turns the spindle on and resumes the cycle. The problem is that there is no delay between spindle power-on and start of motion. Instead, there should be a delay to allow spindle spin-up.

@carneeki
Copy link

carneeki commented Feb 11, 2019 via email

@ademenev
Copy link
Author

I am using 1.1g

Delay does work in the following scenario:

  1. Machine is in Run state, door is closed
  2. I send '!', machine enters Hold state
  3. I open the door, machine enters Door state, spindle stops
  4. I close the door and send '~'
  5. Spindle starts, motion resumes after 4 second delay

But in another scenario there is no delay

  1. Machine is in Run state, door is closed
  2. I send '!', machine enters Hold state
  3. Keeping the door closed, I send 0x9E. Spindle stops
  4. I send '~'
  5. Spindle starts, motion resumes immediately

In summary, when resuming from Door state, there is a delay, when resuming from Hold state, there is no delay, even if spindle was stopped

@barma1ey
Copy link

I am experiencing the same trouble. there is no spindle delay after it was toggled off in Hold state (default config)

carneeki added a commit to carneeki/grbl that referenced this issue Feb 12, 2019
@carneeki
Copy link

carneeki commented Feb 12, 2019 via email

@109JB
Copy link

109JB commented Feb 12, 2019

As far as I am concerned this is not an issue and the fact that the spindle starts automatically after only a feed hold is the issue. So what is actually an issue is that the spindle starts at all when the cycle-start is commanded.

On most industrial machines the feed hold and cycle start buttons start and stop the machine axis feeds only. The spindle can be turned off and on while in feed-hold, as can the coolant, but it is up to the operator to turn them back on when hitting cycle start. This is how every industrial machine I have ever worked on operated.

On an industrial machine the door is locked and cannot even be opened while the spindle is running, so I understand how Grbl has to make a logical concession for the safety door with a switch rather than a lock by turning the spindle off if the door is opened during cycle. However, I don't agree that cycle start should re-start the spindle automatically as default. It would be fine as a user configurable option (ie: config.h), but I do not feel that the cycle start should do anything more after a feed hold than restart axis motion to maintain commonality with industrial standards.

The way you open an industrial machine door and then resume the program is as follows:

1 Feed-hold
2. Coolant off
3. Spindle off (Unlocks door)
4. Open Door
5. Do whatever
6. Close door
7. Start spindle
8. Start coolant
9. Cycle-start

@ademenev
Copy link
Author

@109JB , yes, that is true. For industrial machines.

But how many such machines are are actually controlled by Grbl? The truth is that it is used mostly by hobbyists and small(ish) shops, and vast majority of machines do not have a door at all.

But I still should be able to pause my machine, stop the spindle, do whatever I need, and resume the cycle without breaking my tools.

@ademenev
Copy link
Author

My point is: there is no harm in a delay, but it is extremely helpful

@carneeki
Copy link

Partially agree @109JB .

One of the things I forgot to add in my first reply to @ademenev was to just issue 0x9E again which is supposed to toggle the spindle back on, and simply wait until spindle has spooled up before pressing ~.

I think we disagree about CS / resume though. I expect a resume to also restore the state prior to the hold, which includes the spindle state. I suspect this is what the intent was when the manual was written. By restoring the spindle state automatically, it won't really matter if you restore it manually in step 7 of the check list.
In the limited number of use cases I can think of where one would want to continue motion without the spindle restored, one can simply toggle the spindle override after the restore.

As ademenev said, there's no harm in a delay, but it is helpful. It simplifies your list. I don't run coolant, but I would expect resuming to also restore the coolant state if it was manually toggled off during a hold.

@ademenev
Copy link
Author

ademenev commented Feb 12, 2019 via email

@109JB
Copy link

109JB commented Feb 12, 2019

There are usually very good reasons they do things the way they do on industrial machines. Sometimes hobby machines can learn something. However that is why I said this should not be a default configuration but a user configurable option in config.h. To the extent possible, what you want to do on your machine should not be forced on me and what I want for my machine should not be forced on you. In most things Grbl is very good at this.

As far as breaking tools. Just restart the spindle before issuing a cycle start. You had to issue a spindle stop to stop it didn't you?

I agree that resume should be consistent but my vote would be to make it consistent with industrial operation. Also, it has been noted many times in the past that Grbl as much as possible follows how LinuxCNC operates. If this is still the case, Neither Feed hold or resume affect spindle or coolant in LinuxCNC.

@ademenev
Copy link
Author

So, what would work for most of us, I think is the following:

  • add a config option that will control automatic spindle restart after resume
  • make spindle restart consistent between door/no door configurations, and when enabled - there should be a delay

@carneeki
Copy link

carneeki commented Feb 12, 2019

Inverting the current default behaviour as you suggest is likely to be bad for existing installations where the current install base expects the spindle to spool up. However...

Adding a #define and some #ifndefs to config.h to leave spindle state untouched is trivial, flexible, and, permits the behaviour you would like, it just means when you next upgrade, you'll need to flip this yourself.

Perhaps when 1.2 comes out, this behaviour can be inverted to more closely reflect industrial machines, because as with major version numbers, one expects some significant changes, but not with an h to i revision (think like when D11 and D12 were swapped, the version numbers were bumped for ensuring compatibility) - here the spindle restore behaviour can remain consistent until a major version change, yet remains customisable before the version bump.

@Schildkroet
Copy link

The current behavior is fine! A hold doesn't stop the spindle and therefore doesn't need to wait for it after resume. Your problem is created by yourself and is no error of grbl.

@barma1ey
Copy link

I don't use feed hold often, if ever. So the issue is not a big deal for me. but the documentation clearly states that there must be a 4s delay and it sounds reasonable. Moreover, I think it would be nice to have $ variable to configure a delay time.

@ademenev
Copy link
Author

@Schildkroet , there was no problem created.

It is clearly stated that there should be a delay:

When motion restarts via cycle start, the last spindle state will be restored and wait 4.0 seconds (configurable) before resuming the tool path. This ensures the user doesn't forget to turn it back on.

And that is reasonable

@Schildkroet
Copy link

Schildkroet commented Feb 12, 2019

@ademenev
This is the case for the safety door. There it is correct. But not for a normal user hold.
Just add a dwell between spindle and resume. No code changes needed.
(Your changes are not reasonable and are wrong in terms of commercial machines)

@109JB
Copy link

109JB commented Feb 12, 2019

Actually I'm going to have to change from "my vote would be" to I vehemently oppose incorporating this request. At least incorporating it without a config.h option.

My reasoning is that the feed hold, holds the feed and is what you are resuming from. What you do during the hold is your responsibility to re-activate, or not on a resume.

Incorporating this feature can result in unexpected behavior, namely coolant restarting and spindle restarting. This is particularly true if a user is accustomed to industrial standards. Many user, myself included come from an industrial background, or may move on to an industrial setting. On top of that, we all know how finicky MCU's are with regard to switch inputs and noise errors. I have been fighting this issue on limits on a new machine build. Imagine how issuing a feed hold to check a cutter insert could have someone's hand on a tool, and an errant noise spike could start the spindle causing much more severe injury than simply axis feed resuming.

After thinking about this, I feel that it would be sacrificing safety for a tiny bit of convenience if incorporated without a config.h option for no-spindle and no-coolant on resume.

@ademenev
Copy link
Author

Actually I'm going to have to change from "my vote would be" to I vehemently oppose incorporating this request. At least incorporating it without a config.h option.

Anyway, at least documentation must be corrected.

After thinking about this, I feel that it would be sacrificing safety for a tiny bit of convenience if incorporated without a config.h option for no-spindle and no-coolant on resume.

I am totally fine with this. Make it consistent between door/no-door setups. Make it configurable, with whatever default behavior you think is most reasonable, and let the user decide what they need.

@SailWithChips
Copy link

Hello, I would like to say sorry if it's an stupid question or provably wrong place and sorry in advanced for my low understanding but I would like to ask here for instance working with GUI like "Universal Gcode Sender", how can I send a CMD_FEED_HOLD command ("!" or 0x83)?. Is the only way via some digital input (hardware?). I would like to be able to send a real time command to STOP the machine but I don't know how to do it via software - GUI.

@carneeki
Copy link

Could someone check to see if master...carneeki:master satisfies all parties?

@SailWithChips not quite the right place for this discussion - however no special hardware is needed. The GUI should issue realtime commands to do this for you. If you are writing a GUI in C, 0x83 is just the hexadecimal representation for a char to be sent.

char hold = '!'; // send a exclamation mark to feed hold
char cmd = 0x99; // set spindle to 100%

UGS is written in Java, so there might be a Unicode conversion required. Checkout here for more info on this. I recommend popping over to the UGS people for specifics there on implementing controls there. Incidentally, UGS already has Pause and Stop buttons in the interface already at the top.

@109JB
Copy link

109JB commented Feb 12, 2019

I can check it here in a bit.

@109JB
Copy link

109JB commented Feb 12, 2019

@carneeki

Could someone check to see if master...carneeki:master satisfies all parties?

The above code changes don't appear to work correctly in all cases. Here is what I see

With defines enabled to restore spindle and coolant on cycle start

SpindleOn - Hold - Resume    Works as expected with spindle remaining on the whole time
SpindleOff - Hold - Resume    Works as expected with spindle remaining off the whole time
SpindleOn - Hold - SpindleOff - SpindleOn - Resume   Works as expected
SpindleOn - Hold - SpindleOff - Resume   Does not work and resets Grbl upon sending "~"

With defines NOT enabled (commented out) to restore spindle and coolant on cycle start

SpindleOn - Hold - Resume    Works as expected with spindle remaining on the whole time
SpindleOff - Hold - Resume    Works as expected with spindle remaining off the whole time
SpindleOn - Hold - SpindleOff - SpindleOn - Resume   Works as expected
SpindleOn - Hold - SpindleOff - Resume   Does not work. Grbl remains in the hold state

@carneeki
Copy link

Thanks for the detailed test results!
I'll take a look in the morning - my guess is there is probably some combination of illegal states causing the reset. I'll also see if have a spare Arduino and some LEDs to knock together a test bench so the next commit should pass all test criteria you laid out.

@carneeki
Copy link

@109JB I was trying to recreate the second test case for the defines enabled (SpindleOff, Hold, Resume) when I noticed I had to use M5 rather than toggling the spindle during a motion.

It turns out this override is specifically blocked during motion (with a comment stating to permit the toggle only in hold).

Is this expected behaviour on machines you have used? I'm not sure I've ever had to toggle a spindle during motion before, and I can see reasons to both permit and block this toggle in idle, run and jog states.

@109JB
Copy link

109JB commented Feb 13, 2019

@carneeki

I can't say I have ever had an occasion to turn a spindle off during run mode. Usually if this is the case something bad has gone wrong and you are looking to stop everything and more likely to use an E-stop. I have had occasion to adjust rpm during run, which is covered by the spindle speed override commands. I think it is fine to have the toggle (0x9E) disabled during run.

To tell you the truth, I used the "Spindle" button on my gui when running the tests I performed. This button is programmed to use M3/M4/M5 as appropriate when in "Idle", and it uses (0x9E) when in "Hold". I also used the GUI's feed hold button which just sends a "!", and cycle start button which just sends "~". I didn't actually stream any code as grbl doesn't really know if code is streaming or just being typed a line at a time. For the SpindleOff-Hold_Resume test I simply typed a single line of code (M3S1000), and then M5 to make sure a S value was stored by Grbl. The I insured that the spindle was off, selected feed hold and then resume and made sure the spindle didn't start.

As for when it would make sense from my perspective to be able to use the spindle toggle I would say that limiting it to the Hold state is fine. It is needed for hold because g-code input is blocked during this state so M3/M4/M5 wouldn't get through. During Idle M3/4/5 works so that can be used there.

I can't really see a need to use the toggle anywhere else but a would like to hear you ideas of situations where it might be useful in other modes.

@109JB
Copy link

109JB commented Feb 13, 2019

Forgot to mention that on at least some of the machines I've used you could stop the spindle during run because the spindle speed override could be turned all the way down to 0% thereby stopping the spindle. I can't say for sure if the spindle buttons would work during run as I don't ever remember doing it.

@carneeki
Copy link

carneeki commented Feb 13, 2019 via email

@carneeki
Copy link

With defines enabled to restore spindle and coolant on cycle start

SpindleOn - Hold - SpindleOff - Resume   Does not work and resets Grbl upon sending "~"

The crash is caused by the delay I put in in the first commit. I had the delay in the wrong place, moving it results in the following test cases:

SpindleOn - Hold - Resume - PASS - Motion resumes
SpindleOff - Hold - Resume - PASS - Cannot toggle spindle, motion resumes
SpindleOn - Hold - SpindleOff - SpindleOn - Resume - PASS - Toggle works as expected.
SpindleOn - Hold - SpindleOff - Resume - PASS - Motion resumes after delay.

In the last case, motion was started after the desired four second delay.

Of interest, the spindle state cannot be changed during a feed hold if the spindle is M5. I cannot think of a case where this would be an issue, except perhaps if one were to want to do something dangerous (like a belt change on a CNC lathe / mill that has no VFD). To that end, it is probably best to leave this behaviour as is (and I suspect this is existing behaviour anyway).

FEED_HOLD_RESTORE disabled
SpindleOn - Hold - Resume - PASS - Motion resumes
SpindleOff - Hold - Resume - PASS - Cannot toggle spindle
SpindleOn - Hold - SpindleOff - SpindleOn - Resume - PASS - Motion resumes instantly.
SpindleOn - Hold - SpindleOff - Resume - FAIL - motion stays off until spindle toggled on manually, though perhaps this is desired? It forces the user to manually start the spindle.

I'm running into a bit of a mental wall with the complexity of protocol.c and I'm tempted to break the protocol_rt_exec methods into smaller methods to make it easier to understand. (notably the many nested if() and #ifdef statements).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants