From 5dae16ad837aa9d6fd6274aa57ee87aade1158e5 Mon Sep 17 00:00:00 2001 From: Terje Io Date: Wed, 16 Jul 2025 10:58:31 +0200 Subject: [PATCH] "hardened" code related to ioports claiming and PWM2 spindle configuration. Added API call and expanded one. --- README.md | 2 +- changelog.md | 20 ++++++++++ crossbar.c | 18 +++++++++ crossbar.h | 2 +- grbl.h | 2 +- ioports.c | 1 + spindle_control.c | 100 +++++++++++++++++++++++----------------------- stepper.c | 15 +++++++ stepper.h | 1 + 9 files changed, 108 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index a507559..5968ce2 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ ## grblHAL ## -Latest build date is 20250706, see the [changelog](changelog.md) for details. +Latest build date is 20250716, see the [changelog](changelog.md) for details. > [!NOTE] > A settings reset will be performed on an update of builds prior to 20241208. Backup and restore of settings is recommended. diff --git a/changelog.md b/changelog.md index 7dfef4b..25a4815 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,25 @@ ## grblHAL changelog +Build 20250716 + +Core: + +* "hardened" code related to ioports claiming and PWM2 spindle configuration. Added API call and expanded one. + +Drivers: + +* iMXRT1062: removed stray code guard preventing initialisation of analog auxiliary outputs. Refs issue [#99](https://github.com/grblHAL/iMXRT1062/issues/99). + +* STM32F1xx, STM32F3xx, STM32F4xx, STM32F7xx: ensured stepper enable signals are set to disabled during startup. +> [!NOTE] +> The initial stepper enable signal state will be determined by the stepper driver enable input (pulled up or down) since the related MCU pins are briefly in a high-Z state during startup. + +Plugins: + +* Spindle, PWM: "hardened" and cleaned up code. + +--- + Build 20250706 Core: diff --git a/crossbar.c b/crossbar.c index 978b419..ba1b714 100644 --- a/crossbar.c +++ b/crossbar.c @@ -29,6 +29,22 @@ axes_signals_t xbar_fn_to_axismask (pin_function_t fn) switch(fn) { + case Output_StepperEnable: + mask.bits = AXES_BITMASK; + break; + + case Output_StepperEnableXY: + mask.x = mask.y = On; + break; + +#if N_AXIS > 3 + case Output_StepperEnableAB: + mask.a = On; +#if N_AXIS > 4 + mask.b = On; +#endif + break; +#endif case Input_LimitX: case Input_LimitX_Max: case Input_LimitX_2: @@ -98,6 +114,8 @@ axes_signals_t xbar_fn_to_axismask (pin_function_t fn) #endif default: + if(fn >= Output_StepperEnableX && fn <= Output_StepperEnableV) + mask.bits = (1 << (fn - Output_StepperEnableX)); break; } diff --git a/crossbar.h b/crossbar.h index de2437b..a80024d 100644 --- a/crossbar.h +++ b/crossbar.h @@ -178,9 +178,9 @@ typedef enum { Output_StepperEnableZ2 = Output_StepperEnableZ, Output_StepperEnableA, Output_StepperEnableB, + Output_StepperEnableC, Output_StepperEnableU, Output_StepperEnableV, - Output_StepperEnableC, Output_StepperEnableXY, Output_StepperEnableAB, Output_SpindleOn, diff --git a/grbl.h b/grbl.h index 7d246d3..b980f40 100644 --- a/grbl.h +++ b/grbl.h @@ -42,7 +42,7 @@ #else #define GRBL_VERSION "1.1f" #endif -#define GRBL_BUILD 20250705 +#define GRBL_BUILD 20250716 #define GRBL_URL "https://github.com/grblHAL" diff --git a/ioports.c b/ioports.c index 4a092ce..04c0164 100644 --- a/ioports.c +++ b/ioports.c @@ -581,6 +581,7 @@ static xbar_t *io_get_pin_info (io_port_type_t type, io_port_direction_t dir, ui if(is_match(io_port, type, dir, port)) { if((pin = io_port->hal.get_pin_info(dir, port - io_port->ports_id->cfg[dir].n_start))) { pin->ports_id = io_port->ports_id; + pin->cap.claimable = is_aux(cfg, pin->function); pin->mode.claimed = cfg->claimed.mask & (1UL << (pin->id + io_port->ports_id->cfg[dir].n_start)); } } diff --git a/spindle_control.c b/spindle_control.c index 7ca2917..53c6eaf 100644 --- a/spindle_control.c +++ b/spindle_control.c @@ -978,35 +978,54 @@ static uint32_t get_int (setting_id_t id) return value; } -static status_code_t set_dir_port (setting_id_t id, float value) +static float get_port (setting_id_t id) { - sp1_settings.port_dir = value < 0.0f ? 255 : (int8_t)value; + uint8_t port; - return Status_OK; -} + switch(id) { -static float get_dir_port (setting_id_t id) -{ - return sp1_settings.port_dir == 255 ? -1.0f : (float)sp1_settings.port_dir; -} + case Setting_Spindle_OnPort: + port = sp1_settings.port_on; + break; -static uint32_t get_pwm_port (setting_id_t id) -{ - return (uint32_t)sp1_settings.port_pwm; + case Setting_Spindle_DirPort: + port = sp1_settings.port_dir; + break; + + default: // Setting_Spindle_PWMPort: + port = sp1_settings.port_pwm; + break; + } + + return port == IOPORT_UNASSIGNED ? -1.0f : (float)port; } bool pwm_port_validate (xbar_t *properties, uint8_t port, void *data) { - return port == (uint8_t)((uint32_t)data); + return port == *(uint8_t *)data; } -static status_code_t set_pwm_port (setting_id_t id, uint_fast16_t int_value) +static status_code_t set_port (setting_id_t id, float value) { - bool ok; + bool ok = true; + uint8_t port = value < 0.0f ? IOPORT_UNASSIGNED : (uint8_t)value; - if((ok = (uint8_t)int_value == sp1_settings.port_pwm || - ioports_enumerate(Port_Analog, Port_Output, (pin_cap_t){ .pwm = On, .claimable = On }, pwm_port_validate, (void *)((uint32_t)int_value)))) - sp1_settings.port_pwm = (uint8_t)int_value; + switch(id) { + + case Setting_Spindle_OnPort: + sp1_settings.port_on = port; + break; + + case Setting_Spindle_DirPort: + sp1_settings.port_dir = port; + break; + + default: // Setting_Spindle_PWMPort: + if((ok = port == sp1_settings.port_pwm || + ioports_enumerate(Port_Analog, Port_Output, (pin_cap_t){ .claimable = On, .pwm = On }, pwm_port_validate, &port))) + sp1_settings.port_pwm = port; + break; + } return ok ? Status_OK : Status_SettingValueOutOfRange; } @@ -1027,10 +1046,10 @@ static bool has_ports (const setting_detail_t *setting, uint_fast16_t offset) } static const setting_detail_t spindle1_settings[] = { - { Setting_Spindle_OnPort, Group_AuxPorts, "PWM2 spindle on port", NULL, Format_Int8, "##0", "0", max_dport, Setting_NonCore, &sp1_settings.port_on, NULL, has_ports, { .reboot_required = On } }, - { Setting_Spindle_DirPort, Group_AuxPorts, "PWM2 spindle direction port", NULL, Format_Decimal, "-#0", "-1", max_dport, Setting_NonCoreFn, set_dir_port, get_dir_port, has_ports, { .reboot_required = On } }, + { Setting_Spindle_OnPort, Group_AuxPorts, "PWM2 spindle on port", NULL, Format_Decimal, "-#0", "0", max_dport, Setting_NonCoreFn, set_port, get_port, has_ports, { .reboot_required = On } }, + { Setting_Spindle_DirPort, Group_AuxPorts, "PWM2 spindle direction port", NULL, Format_Decimal, "-#0", "-1", max_dport, Setting_NonCoreFn, set_port, get_port, has_ports, { .reboot_required = On } }, { Setting_SpindleInvertMask1, Group_Spindle, "PWM2 spindle signals invert", NULL, Format_Bitfield, spindle_signals, NULL, NULL, Setting_IsExtendedFn, set_spindle_invert, get_int, NULL, { .reboot_required = On } }, - { Setting_Spindle_PWMPort, Group_AuxPorts, "PWM2 spindle PWM port", NULL, Format_Int8, "#0", "0", max_aport, Setting_NonCoreFn, set_pwm_port, get_pwm_port, has_ports, { .reboot_required = On } }, + { Setting_Spindle_PWMPort, Group_AuxPorts, "PWM2 spindle PWM port", NULL, Format_Decimal, "-#0", "0", max_aport, Setting_NonCoreFn, set_port, get_port, has_ports, { .reboot_required = On } }, { Setting_SpindlePWMOptions1, Group_Spindle, "PWM2 spindle options", NULL, Format_XBitfield, "Enable,RPM controls spindle enable signal,Disable laser mode capability", NULL, NULL, Setting_IsExtendedFn, set_pwm_options, get_int, has_pwm }, { Setting_RpmMax1, Group_Spindle, "PWM2 spindle min speed", "RPM", Format_Decimal, "#####0.000", NULL, NULL, Setting_IsLegacy, &sp1_settings.cfg.rpm_max, NULL, has_pwm }, { Setting_RpmMin1, Group_Spindle, "PWM2 spindle max speed", "RPM", Format_Decimal, "#####0.000", NULL, NULL, Setting_IsLegacy, &sp1_settings.cfg.rpm_min, NULL, has_pwm }, @@ -1052,7 +1071,6 @@ static const setting_detail_t spindle1_settings[] = { #endif }; -#ifndef NO_SETTINGS_DESCRIPTIONS static const setting_descr_t spindle1_settings_descr[] = { { Setting_Spindle_OnPort, "On/off aux port." }, { Setting_Spindle_DirPort, "Direction aux port, set to -1 if not required." }, @@ -1080,7 +1098,6 @@ static const setting_descr_t spindle1_settings_descr[] = { #endif #endif }; -#endif static void spindle1_settings_changed (settings_t *settings, settings_changed_flags_t changed) { @@ -1144,6 +1161,10 @@ static void spindle1_settings_restore (void) memcpy(&sp1_settings.cfg, &defaults, sizeof(spindle_pwm_settings_t)); + sp1_settings.port_pwm = ioport_find_free(Port_Analog, Port_Output, (pin_cap_t){.claimable = On, .pwm = On }, NULL); + sp1_settings.port_on = ioport_find_free(Port_Digital, Port_Output, (pin_cap_t){ .claimable = On }, NULL); + sp1_settings.port_dir = sp1_settings.port_on != IOPORT_UNASSIGNED && sp1_settings.port_on > 0 ? sp1_settings.port_on - 1 : IOPORT_UNASSIGNED; + hal.nvs.memcpy_to_nvs(nvs_address, (uint8_t *)&sp1_settings, sizeof(spindle1_pwm_settings_t), true); } @@ -1153,35 +1174,16 @@ static void spindle1_settings_load (void) spindle1_settings_restore(); } -static bool pwm_count (xbar_t *properties, uint8_t port, void *data) -{ - *((uint32_t *)data) += 1; - - sp1_settings.port_pwm = max(sp1_settings.port_pwm, port); - - return false; -} - -static bool check_pwm_ports (void) -{ - uint32_t n_pwm_out = 0; - - ioports_enumerate(Port_Analog, Port_Output, (pin_cap_t){ .pwm = On, .claimable = On }, pwm_count, &n_pwm_out); - - return n_pwm_out != 0; -} - spindle1_pwm_settings_t *spindle1_settings_add (bool claim_ports) { - uint8_t n_out; + uint8_t a_out = IOPORT_UNASSIGNED; - if((ports_ok = claim_ports && (n_out = ioports_available(Port_Digital, Port_Output)) && check_pwm_ports())) { + if((ports_ok = claim_ports && + ioports_available(Port_Digital, Port_Output) && + (a_out = ioport_find_free(Port_Analog, Port_Output, (pin_cap_t){ .claimable = On, .pwm = On }, NULL)) != IOPORT_UNASSIGNED)) { - sp1_settings.port_on = n_out - 1; - sp1_settings.port_dir = n_out > 1 ? n_out - 2 : 255; - - strcpy(max_aport, uitoa(sp1_settings.port_pwm)); - strcpy(max_dport, uitoa(n_out - 1)); + strcpy(max_aport, uitoa(a_out)); + strcpy(max_dport, uitoa(ioport_find_free(Port_Digital, Port_Output, (pin_cap_t){ .claimable = On }, NULL))); } return nvs_address == 0 && (!claim_ports || ports_ok) && (nvs_address = nvs_alloc(sizeof(spindle1_pwm_settings_t))) ? &sp1_settings : NULL; @@ -1193,10 +1195,8 @@ void spindle1_settings_register (spindle_cap_t cap, spindle1_settings_changed_pt .is_core = true, .settings = spindle1_settings, .n_settings = sizeof(spindle1_settings) / sizeof(setting_detail_t), - #ifndef NO_SETTINGS_DESCRIPTIONS .descriptions = spindle1_settings_descr, .n_descriptions = sizeof(spindle1_settings_descr) / sizeof(setting_descr_t), - #endif .load = spindle1_settings_load, .restore = spindle1_settings_restore, .save = spindle1_settings_save, @@ -1216,4 +1216,4 @@ void spindle1_settings_register (spindle_cap_t cap, spindle1_settings_changed_pt setting_remove_elements(Setting_SpindleInvertMask1, spindle_state.mask); } -#endif +#endif // N_SPINDLE > 1 diff --git a/stepper.c b/stepper.c index 218b2b9..2c841cd 100644 --- a/stepper.c +++ b/stepper.c @@ -1297,3 +1297,18 @@ offset_id_t st_get_offset_id (void) ? pl_block->offset_id : -1); } + +// Called by driver setup function to get initial enable signals state +// TODO: returns all disabled for now, should return enabled according to configuration if +// not using Trinamic drivers since Trinamic drivers are init'ed after driver setup? +axes_signals_t st_get_enable_out (void) +{ + axes_signals_t enable; + +// +// enable.mask = (settings.steppers.idle_lock_time == 255 ? AXES_BITMASK : settings.steppers.energize.mask) ^ settings.steppers.enable_invert.mask; + + enable.mask = settings.steppers.enable_invert.mask; + + return enable; +} diff --git a/stepper.h b/stepper.h index 3ca4488..e5ea71e 100644 --- a/stepper.h +++ b/stepper.h @@ -132,5 +132,6 @@ float st_get_realtime_rate (void); void stepper_driver_interrupt_handler (void); offset_id_t st_get_offset_id (void); +axes_signals_t st_get_enable_out (void); #endif