gnea / grbl

An open source, embedded, high performance g-code-parser and CNC milling controller written in optimized C that will run on a straight Arduino
https://github.com/gnea/grbl/wiki
Other
4.07k stars 1.61k forks source link

Reset input behaviour #479

Closed zsoden closed 6 years ago

zsoden commented 6 years ago

While commissioning my new machine I accidentally had the e-stop wired incorrectly (it is wired into the A0 reset input), so that it was giving the opposite signal. The result of this was that the machine would behave like an emergency stop only when the feed hold or cycle start hardware buttons were pressed.

Reviewing the code I discovered that although the ISR that detects these buttons is only triggered on a change of state of one of the inputs, the ISR function then reads all of the states:

ISR(CONTROL_INT_vect)
{
  uint8_t pin = system_control_get_state();
  if (pin) {
    if (bit_istrue(pin,CONTROL_PIN_INDEX_RESET)) {
      mc_reset();
    } else if (bit_istrue(pin,CONTROL_PIN_INDEX_CYCLE_START)) {
      bit_true(sys_rt_exec_state, EXEC_CYCLE_START);
      } else if (bit_istrue(pin,CONTROL_PIN_INDEX_FEED_HOLD)) {
        bit_true(sys_rt_exec_state, EXEC_FEED_HOLD);
    }
  }
}

(I removed the #defines to improve readability)

This means that the controller will only detect a change of state of the reset input. Once the controller is reset, if the machine will continue to run even if the reset pin is still active.

I presume this is by design, but I would rather see the machine always check the reset pin state before commencing any motion. This would make the pin state based rather than transition based. I also wanted to document this behaviour in case anybody else was having the same problem as I was having.

cri-s commented 6 years ago

You don't have followed the whole source code. Inside system_control_get_state() there is the code that check the levels of the input pins. It is true that the ISR is called on every polarity change from last port access, but the mentioned function checks the levels of input pin, not the transitions and the code is written correctly, you'r missed to check the system_control_get_state. If you have inverted levels, you must set INVERT_CONTROL_PIN_MASK accordly.

Regards Chris

2018-06-20 10:04 GMT+02:00, zsoden notifications@github.com:

While commissioning my new machine I accidentally had the e-stop wired incorrectly (it is wired into the A0 reset input), so that it was giving the opposite signal. The result of this was that the machine would behave like an emergency stop only when the feed hold or cycle start hardware buttons were pressed.

Reviewing the code I discovered that although the ISR that detects these buttons is only triggered on a change of state of one of the inputs, the ISR function then reads all of the states:

ISR(CONTROL_INT_vect)
{
  uint8_t pin = system_control_get_state();
  if (pin) {
    if (bit_istrue(pin,CONTROL_PIN_INDEX_RESET)) {
      mc_reset();
    } else if (bit_istrue(pin,CONTROL_PIN_INDEX_CYCLE_START)) {
      bit_true(sys_rt_exec_state, EXEC_CYCLE_START);
      } else if (bit_istrue(pin,CONTROL_PIN_INDEX_FEED_HOLD)) {
        bit_true(sys_rt_exec_state, EXEC_FEED_HOLD);
    }
  }
}

(I removed the #defines to improve readability)

This means that the controller will only detect a change of state of the reset input. Once the controller is reset, if the machine will continue to run even if the reset pin is still active.

I presume this is by design, but I would rather see the machine always check the reset pin state before commencing any motion. This would make the pin state based rather than transition based. I also wanted to document this behaviour in case anybody else was having the same problem as I was having.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/gnea/grbl/issues/479

zsoden commented 6 years ago

Hi Chris,

Yes, I saw and understand the operation of system_control_get_state(), but as you said the check is only performed when a change of state of the pins is detected. So if the reset input is being used with an e-stop button grbl will only react upon the initial press. The machine should remain disabled until the e-stop is reset:

  1. Program start
  2. E-stop button pressed (and latched)
  3. Machine stop and enter alarm state
  4. Reset alarms ($X) <---- But e-stop button is not reset
  5. Program start
  6. Machine stop and enter alarm state
  7. Reset alarms ($X)
  8. Reset e-stop button
  9. Program start
  10. Program runs to completion

At the moment the system operates as follows:

  1. Program start
  2. E-stop button pressed (and latched)
  3. Machine stop and enter alarm state
  4. Reset alarms ($X) <---- But e-stop button is not reset
  5. Program start
  6. Program runs to completion

Hence why I feel that it would be prudent to be periodically performing the input pin state checking outside of the ISR.

Regards,

Zac

chamnit commented 6 years ago

The AVR only detects a pin change. It does not tell you which pin on the port that triggered the change or which direction. Without a software debouncing routine, which requires a spare timer, you can't poll the pin reliably. Instead of trying to fix the problem in software that can introduce bugs, I highly suggest you either reroute your safety door switch to cut power instead. You can always cut the trace from the safety door switch from A0 since it's wrong anyhow.

zsoden commented 6 years ago

I understand how the interrupt on change feature works but I appreciate you explaining why the decision was made to operate like this.

I have my e-stop button doing both - it cuts power to the drives and triggers the reset pin. I was simply expecting to be able to use the reset pin to indicate to the operator via the alarm state that the e-stop has been triggered. It's no problem, this was just a suggestion and as I said, wanting to document this behaviour (regardless of whether correct or not) for others who have the same issue I had.

cri-s commented 6 years ago

I hope you use externall pull-up or pull-downs for enable and step driver inputs in order the driver is enabled during reset and no steps are generated because of floating step pins. If step pins using optocouplers, no pull-up or pull-down is required for step pins.

As e-stop require shutdown of power, the above sequence you describe are not possible. On faster machines, e-stop need to go to stop input and after some timout (ne555 as example) the power should be cut (using two separate relay, not single one) and reset should be applied if controller don't have external reset supervisor ic.

For the problem you have described, the simplest solution is to route the enable pin from GRBL trought the e-stop switch to the stop input. This then ensure the behavior you are requesting. This is valid only for hobby machines or if e-stop with two contacts where the second contact cuts power.

Further E-Stop usually means regulamentory requirement that differ a lot based on the local regulations and the usage of grbl generally this topic is left off to the implementor that usually must produce some documentation attesting the risk classes ....

I know hobby machines have relaxed requirement, but safety is not optional for bigger machines.

2018-06-21 0:29 GMT+02:00, zsoden notifications@github.com:

I understand how the interrupt on change feature works but I appreciate you explaining why the decision was made to operate like this.

I have my e-stop button doing both - it cuts power to the drives and triggers the reset pin. I was simply expecting to be able to use the reset pin to indicate to the operator via the alarm state that the e-stop has been triggered. It's no problem, this was just a suggestion and as I said, wanting to document this behaviour (regardless of whether correct or not) for others who have the same issue I had.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/gnea/grbl/issues/479#issuecomment-398918077

zsoden commented 6 years ago

Thanks for the tips, Chris. Yes, I'm using optocouplers for the enable, step and direction pins of the stepper drivers.

You're correct that this depends primarily on the e-stop legislation. This is only a small machine being used in a commercial workshop and after assessing the requirements I decided to only have the e-stop depower the stepper drivers and spindle drive, not the control electronics. The e-stop button has two contacts: one cutting drive power and one sending a signal to the controller.

cri-s commented 6 years ago

I have not checked it on grbl 1.1, on grbl 0.9 it worked routing the enable pin to stop (reset) input using NO switch in combination with external pull-up or pull-down. This ensures that there are pin state changes and the e-stop input is always processed. Previously when i was talking about reset, i intendet the hardware reset pin, not the stop (grbl software reset input) pin. This for small machines. For larger one or where safety is bigger concert, the switch must be NC and you must run a square wave combined with charge pump circuit, named cnc watchdog circuit. This ensures that if there is a break in the E-stop wiring, the machine automatically detect it and halts the execution.

2018-06-21 1:55 GMT+02:00, zsoden notifications@github.com:

Thanks for the tips, Chris. Yes, I'm using optocouplers for the enable, step and direction pins of the stepper drivers.

You're correct that this depends primarily on the e-stop legislation. This is only a small machine being used in a commercial workshop and after assessing the requirements I decided to only have the e-stop depower the stepper drivers and spindle drive, not the control electronics. The e-stop button has two contacts: one cutting drive power and one sending a signal to the controller.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/gnea/grbl/issues/479#issuecomment-398933202

zsoden commented 6 years ago

Thanks Chris, that makes sense.

Indeed safety on larger systems becomes very complex. I recall a few years ago a colleague working on a large plant and having to deal with light curtains, safety relays and other similar systems. There were many "gotchas" in using such equipment!

For this machine I have already added a feature to GRBL so it's probably easier to add a state check for the reset input rather than reconfigure the machine wiring. I have added a feature for GRBL to tell the g-code sender that the cycle start button has been pressed if the machine is idle. The g-code sender will therefore begin sending the program based on a button press rather than selecting "run" in the g-code sender software. This will be better in a production environment where the machine is repeating the same operation.