End-stop ISR

I'm slightly surprised the end-stops aren't checked all the time, what happens if the firmware gets itself into a knot or the motors lose steps and the firmware thinks the head is somewhere that is not and drives the head into the side of the printer/cnc-mill etc.. - its potentially going to do damage.

Is there a reason the end-stops are only checked during homing?

I've got marlin running on a couple of 3d printers and i've ported it to running a larger cnc machine, to get the larger machine running for development purposes, but the motors (220v 5-phase jobs) have got that much torque that its likely to do some damage if something goes wrong, hence my question.

I am thinking of adding an additional pinOnChange ISR() to my copy of marlin so if any of the end-stops get triggered then it can just stall, not sure what the general thoughts on this would be though.

Thanks in advance

Matt

[Netoperz]

I think that it is connected with this issue:

#5096

I've also noticed that my endstops are ignored, check my comments in that 5096 issue it is long so i will not put it ones again here.

[Nocturnal42]

Is ENDSTOPS_ALWAYS_ON_DEFAULT enabled in your Configuration_adv.h?

Ah i'd not seen that functionality - thanks :)

[Nocturnal42]

I've done the exact same thing several times recently :)

I just had a quick eyeball of the source, i guess as soon as the end-stop is hit, the stepper stops and the motion-block gets emptied so the machine will just abort the current g-code line?

[thinkyhead]

the stepper stops and the motion-block gets emptied

Yes. Not the "current GCode line" but the current movement. (A single GCode line may invoke several movements.) This behavior is used by homing and leveling.

And with ABORT_ON_ENDSTOP_HIT_FEATURE_ENABLED, it aborts the entire print job when SD printing.

[alexborro]

The endstop check routine takes several precious cpu cycles to be executed. You cannot perform such check and fast movements at the same time.

Unfortunately the few I/O with toggle interrupt usually is not mapped to endstop pins.

So it is a dead end. If you need a fast printer, you cannot poll the endstops all the time.

[thinkyhead]

True. Currently the stepper ISR calls endstops.update to get the current state of the endstop pins. This allows it to stop within only a few steps of hitting an endstop, whereas if we polled less frequently (for example in the temperature ISR, which runs at ~977Hz) then things like bed probing might be less accurate. It would be interesting to see whether such a loss of probing accuracy would be significant, since probing is usually done at a pretty slow feed-rate.

[Blue-Marlin]

According to #4026 at least 2/3 of our users (the RAMPS users) could profit from a solution not polling the pins but using interrupts or/and pin change interrupts to find the changing state of the endstop pins.
RAMPS has all 6 endstop pins on pins that can cause interrupts (4 direct and 2 on the same PCI)
Sadly this is not the case for all RAMPS based boards.
The RAMBOs have all 3 of their endsto pins on a common PCI.
All other boars i investigated have only 2 or less usable pins.
All ARM processor based boars have interrupts at all pins.

My current plan is simple enough to give it a try.

If for all used endstop pins a interrupt is available
  set up one common ISR-routine
  just setting a flag ('something has happened') to 2

In the stepper ISR 
  If for all used endstop pins a interrupt is available
    If endstop testing is on
      if 'something has happened'
        use the old endstop test routine and decrease the flag by one
        // because of the one step debouching it needs two calls.
      else
        skip the old endstop testing
  else
     use the old endstop testing

[Netoperz]

I'm switching my project also to ARM, Arduino clone with arm costs 2x price of arduino duo r3, but it is let's say 10$ vs 20$ in most pricey situation when buying from retailer on not much cheap shop.

In case of stable printing and good performance i think there is no need to work with solutions that keep the "old" "slow" boards fast, because switching to faster board is not a problem for owner that have build a printer by him self especially for Arduino user where changing board from ARM to Mega and back is just swapping the board. And if we consider that that potential user can sell "old board, the upgrade costs a 7-10$ and makes the device more stable.

So I would say we should think about performance in current hardware. And not checking the endstops may cause hardware disaster. Just like i've had.

But it is just free opinion.

[thinkyhead]

RAMPS has all 6 endstop pins on pins that can cause interrupts

@Blue-Marlin And, I presume we have an easy way of seeing for any given endstop pin whether it has an interrupt. Either by checking the fastio.h defines, or by using the "pin description" table from one of the Arduino headers…?

[Blue-Marlin]

Indeed.
For the EXT-interrupts it's easy. pins_arduino.h provides a macro (digitalPinToInterrupt(p)) we can check with the preprocessor.
But with the PCIs i'm struggling a bit.
From:

#if digitalPinToPCICR(2) == NULL
  #error "Not a PCI-pin"
#endif

i get

error: operator '&' has no left operand
                                   (((p) >= 62) && ((p) <= 69)) ? (&PCICR) : ((uint8_t *)0) )

                                                                   ^

Likely the preprocessor can't handle adresses - or at least I can't handle adresses in the preprocessor.
@thinkyhead
Has the 'Macro Master' an idea?

I could handle that in C at runtime. But that would produce a lot of unused code.

Cutout from pins_Arduino.h for the MEGA

// A majority of the pins are NOT PCINTs, SO BE WARNED (i.e. you cannot use them as receive pins)
// Only pins available for RECEIVE (TRANSMIT can be on any pin):
// (I've deliberately left out pin mapping to the Hardware USARTs - seems senseless to me)
// Pins: 10, 11, 12, 13,  50, 51, 52, 53,  62, 63, 64, 65, 66, 67, 68, 69

#define digitalPinToPCICR(p)    ( (((p) >= 10) && ((p) <= 13)) || \
                                  (((p) >= 50) && ((p) <= 53)) || \
                                  (((p) >= 62) && ((p) <= 69)) ? (&PCICR) : ((uint8_t *)0) )

#define digitalPinToPCICRbit(p) ( (((p) >= 10) && ((p) <= 13)) || (((p) >= 50) && ((p) <= 53)) ? 0 : \
                                ( (((p) >= 62) && ((p) <= 69)) ? 2 : \
                                0 ) )

#define digitalPinToPCMSK(p)    ( (((p) >= 10) && ((p) <= 13)) || (((p) >= 50) && ((p) <= 53)) ? (&PCMSK0) : \
                                ( (((p) >= 62) && ((p) <= 69)) ? (&PCMSK2) : \
                                ((uint8_t *)0) ) )

#define digitalPinToPCMSKbit(p) ( (((p) >= 10) && ((p) <= 13)) ? ((p) - 6) : \
                                ( ((p) == 50) ? 3 : \
                                ( ((p) == 51) ? 2 : \
                                ( ((p) == 52) ? 1 : \
                                ( ((p) == 53) ? 0 : \
                                ( (((p) >= 62) && ((p) <= 69)) ? ((p) - 62) : \
                                0 ) ) ) ) ) )

#define digitalPinToInterrupt(p) ((p) == 2 ? 0 : ((p) == 3 ? 1 : ((p) >= 18 && (p) <= 21 ? 23 - (p) : NOT_AN_INTERRUPT)))

digitalPinToPCICRbit(p) and digitalPinToPCMSKbit(p) don't work as a check because 0 is in legal results. digitalPinToPCMSK(p) has the same pointer problem.

[thinkyhead]

Has the 'Macro Master' an idea?

I believe we have to use static_assert instead of #if for dealing with addresses.

static_assert(digitalPinToPCICR(2), "Not a PCI-pin");