usbcnc / grbl

This is a port of GRBL 1.1 to STM32F103 target
https://github.com/usbcnc/grbl/wiki
Other
206 stars 73 forks source link

bug when using normally closed switch for hard limits #60

Open mstrens opened 5 years ago

mstrens commented 5 years ago

Hard limit switches are handled by interrupt.

By default GRBL uses normally open switches but the setup allows to invert the logic in order to use normally closed switches (with the parameter defined to invert limits).

Still the current version for stm32 has a bug in case of normally closed switches. Currently the interrupts are only triggered on the falling edge. This is wrong for a normally closed switch. It has to be triggered on the rising edge.

There is a pull request that propose to trigger on both edges but this generates another bug because interrupt is then triggered twice.

I thing that following change should be ok. In file limit.c, there is a function void limits_init() There is a line EXTI_InitStructure.EXTI_Trigger = EXTI_Trigger_Falling; //Trigger mode, can be a falling edge trigger EXTI_Trigger_Falling, the rising edge triggered EXTI_Trigger_Rising, or any level (rising edge and falling edge trigger EXTI_Trigger_Rising_Falling)

It should be replaced by if (bit_istrue(settings.flags, BITFLAG_INVERT_LIMIT_PINS )) { // for normally closed switches, we need to interrupt on the rising edge EXTI_InitStructure.EXTI_Trigger = EXTI_Trigger_Rising ; //Trigger mode, can be a falling edge trigger EXTI_Trigger_Falling, the rising edge triggered EXTI_Trigger_Rising, or any level (rising edge and falling edge trigger EXTI_Trigger_Rising_Falling) } else { EXTI_InitStructure.EXTI_Trigger = EXTI_Trigger_Falling; //Trigger mode, can be a falling edge trigger EXTI_Trigger_Falling, the rising edge triggered EXTI_Trigger_Rising, or any level (rising edge and falling edge trigger EXTI_Trigger_Rising_Falling) }

robomechs commented 5 years ago

I have to check it.

robomechs commented 5 years ago

Hmm. It is not very simple to get rid of the signal bounce. You have to use RC filter close to uC input. Without it you will have several falling and rising edges for each switching. I believe, that the safest option would be Rising_Falling. Only one difficulty for this option: after the limit switch is triggered (and after reset), retraction the carriage from it in the opposite direction will again cause an interrupt when the switch is released. But in most practical cases, the interrupt will be caused in any case (doesn't metter falling or rising edge) because of bouncing. What do you think?

mstrens commented 5 years ago

I will have to check again on my machine. I am quite new with all this. I am currently mainly writing a GRBL controller for STM32F103 in order to control the machine without a connection to a pc. Still I think that my solution works for my machine perhaps because:

robomechs commented 5 years ago

2018-11-20 21-57-49_22-02-06

mstrens commented 5 years ago

I even did not put the resistance R; I only put the capacitor C. When the switch is closed (normal state), I expect that the level remains low (even if there is some noise on the wires) because there is a very low resistance to ground. When the switch is open, there is a risk that noise on the wires arrives up to the mcu pin but this seems acceptable because the machine should already be stopped. If switch is open (limit reached) and I unlock the alarm, I think I can move the axis (e.g. with jog commands) without bouncing because the capacitor and inyternal pull up resistor will smooth any rising voltage just like in your schema with a capacitor. I will try to make tests to morrow

robomechs commented 5 years ago

I agree with you. I just want to say that in most cases users will not be add additional elements (as R&C).

I will try to make tests to morrow

I think it's not quite necessary. May be you are right about separate Falling and Rising edges for NC and NO limits. So it will be more useful only for releasing limit switch with jog commands for example (not manualy).

mstrens commented 5 years ago

FYI, I made some tests and it works fine. Here how I did it: I reach the limit switch; the machine stops; I unlock the alarm. I can then jog the machine in the opposite direction.