Paciente8159 / uCNC

µCNC - Universal CNC firmware for microcontrollers
https://github.com/Paciente8159/uCNC/wiki
GNU General Public License v3.0
279 stars 64 forks source link

When configured in 6 axis mode, there is a false-positive trigger of limit switch 'C' #602

Closed jarney closed 7 months ago

jarney commented 7 months ago

Describe the bug A clear and concise description of what the bug is.

What is your hardware Describe your hardware:

  1. What's your board Arduino Mega 2560
  2. Paste your "Grbl/µCNC settings"
    $0=33.333
    $1=0
    $2=0
    $3=0
    $4=0
    $5=255
    $6=0
    $7=0
    $10=1
    $11=0.200
    $12=0.002
    $13=0
    $20=0
    $21=1
    $22=1
    $23=0
    $24=10.000
    $25=50.000
    $26=250
    $27=2.000
    $30=1000
    $31=0
    $32=0
    $81=0.000
    $100=200.000
    $101=200.000
    $102=200.000
    $103=200.000
    $104=200.000
    $105=200.000
    $110=500.000
    $111=500.000
    $112=500.000
    $113=500.000
    $114=500.000
    $115=500.000
    $120=10.000
    $121=10.000
    $122=10.000
    $123=10.000
    $124=10.000
    $125=10.000
    $130=200.000
    $131=200.000
    $132=200.000
    $133=200.000
    $134=200.000
    $135=200.000
  3. If this is a custom compilation tell us what configurations you changed LIMIT_X, LIMIT_Y, LIMIT_Z, LIMIT_A, LIMIT_B, LIMITC all mapped to correct ports with LIMIT_PULLUP_ENABLE all enabled and $21=1 set to enable hard limits. Note also that $5=255 allowing all of the limit pin senses to be inverted.

What was the GCode running No GCode running. Alarm triggers on start-up forcing a re-start because of a (false) trigger of the LIMIT_C bit.

Screenshots N/A

Additional context I was able to trace this down to LIMITS_INV_MASK not correctly inverting the LIMIT_C bit because it was missing from the LIMITS_INV_MASK. Thus when the limit bit is '1', it does NOT invert resulting in a false report of a limit trigger.

        uint8_t inv = g_settings.limits_invert_mask; // This is $5
        uint8_t result = (value ^ (inv & LIMITS_INV_MASK));

Therefore, I propose to add LIMIT_C_MASK to the definition of LIMITS_INV_MASK as follows:

diff --git a/uCNC/src/cnc_hal_config_helper.h b/uCNC/src/cnc_hal_config_helper.h
index 8e6e6275..51c2a9cd 100644
--- a/uCNC/src/cnc_hal_config_helper.h
+++ b/uCNC/src/cnc_hal_config_helper.h
@@ -1822,7 +1822,7 @@ extern "C"
 #define LIMIT_Z2_INV_MASK 4
 #endif

-#define LIMITS_INV_MASK (LIMIT_X_INV_MASK | LIMIT_Y_INV_MASK | LIMIT_Z_INV_MASK | LIMIT_A_INV_MASK | LIMIT_B_INV_MASK | LIMIT_B_INV_MASK)
+#define LIMITS_INV_MASK (LIMIT_X_INV_MASK | LIMIT_Y_INV_MASK | LIMIT_Z_INV_MASK | LIMIT_A_INV_MASK | LIMIT_B_INV_MASK | LIMIT_B_INV_MASK | LIMIT_C_INV_MASK)
 #define LIMITS_DUAL_INV_MASK (LIMIT_X2_INV_MASK | LIMIT_Y2_INV_MASK | LIMIT_Z2_INV_MASK)

 #if (ASSERT_PIN(DIN0) && defined(DIN0_ISR))
Paciente8159 commented 7 months ago

Thanks for reporting this. Fell free to submit a PR.

If not, I will fix this tomorrow. Again thanks for spotting this.

jarney commented 7 months ago

I've just raised a PR. Thanks for providing the software, I'm having a blast setting it up. Fingers crossed, it goes smoothly :)

Paciente8159 commented 7 months ago

Merged the PR. Thanks