From 7e40bf24025b7e4870b996dff0ed532eeaf0a386 Mon Sep 17 00:00:00 2001 From: Terje Io Date: Wed, 20 Nov 2024 19:03:27 +0100 Subject: [PATCH] A bit of refactoring related to spindles as a first step for a coming settings structure revision, "hardened" some code. --- README.md | 4 +-- changelog.md | 31 ++++++++++++++++++++-- gcode.c | 12 +++++---- grbl.h | 2 +- grbllib.c | 28 +++++++++++++++++--- ngc_params.c | 2 +- nuts_bolts.c | 2 +- pin_bits_masks.h | 2 +- report.c | 18 +++++++++---- settings.c | 67 ++++++++++++++++++++++++++++------------------- spindle_control.c | 7 ++--- spindle_control.h | 2 +- system.c | 2 +- 13 files changed, 125 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 8a02074..a10efd0 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ It has been written to complement grblHAL and has features such as proper keyboa --- -Latest build date is 20241113, see the [changelog](changelog.md) for details. +Latest build date is 20241120, see the [changelog](changelog.md) for details. __NOTE:__ Build 20240222 has moved the probe input to the ioPorts pool of inputs and will be allocated from it when configured. The change is major and _potentially dangerous_, it may damage your probe, so please _verify correct operation_ after installing this, or later, builds. @@ -94,4 +94,4 @@ G/M-codes not supported by [legacy Grbl](https://github.com/gnea/grbl/wiki) are Some [plugins](https://github.com/grblHAL/plugins) implements additional M-codes. --- -20240523 +20241120 diff --git a/changelog.md b/changelog.md index e630e7b..05bf266 100644 --- a/changelog.md +++ b/changelog.md @@ -1,14 +1,41 @@ ## grblHAL changelog +Build 20241120 + +Core: + +* A bit of refactoring related to spindles as a first step for a coming settings structure revision, "hardened" some code. + +Drivers: + +* Some: added support for ["assorted small plugins"](https://github.com/grblHAL/Plugins_misc/), mainly for drivers available in the Web Builder. + +* STM32F1xx: moved board specific code to _biard_ directory. + +* STM32F4xx: simplified configuration of Trinamic low-level interfaces, fixed typos in second RGB LED channel code +and added workaround for RGB LEDs connected to debug pin beeing turned on at boot. + +Plugins: + +* Motors: refactored M-code handling and added support for Marlin style `M569` and `M919` commands. Some minor bug fixes. + +* Trinamic: fixed "bug" in TMC2660 current handling, extended API. + +* Spindle: changed settings handling when multiple spindles are configured. Fixed bug in `M104P` M-code. +__NOTE:__ an automatic update of settings `$511`-`$513` will be attempted, this may fail so please check them after upgrading. + +--- + + Build 20241116 Core: * Added support for named o-sub/o-call to flow control, for calling gcode subroutines stored on SD card or in littlefs. Sub name matches filename with extension _.macro_. -* Added core event for handling special gcode comments used by expressions: `DEBUG`, `PRINT` and `ABORT`. These can now be extended by or new added to by plugins. +* Added core event for handling special gcode comments used by expressions: `DEBUG`, `PRINT` and `ABORT`. These can now be extended or new added to by plugins. -* Added overrideable default $-setting values for second PWM spindle to _grbl/config.c_. +* Added overridable default $-setting values for second PWM spindle to _grbl/config.c_. Plugins: diff --git a/gcode.c b/gcode.c index 35f36b3..e557b00 100644 --- a/gcode.c +++ b/gcode.c @@ -940,7 +940,11 @@ status_code_t gc_execute_block (char *block) if((letter < 'A' && letter != '$') || letter > 'Z') FAIL(Status_ExpectedCommandLetter); // [Expected word letter] - if(!read_float(block, &char_counter, &value)) { + if(letter == 'O') { + value = NAN; + if((status = read_uint(block, &char_counter, &int_value)) != Status_OK) + FAIL(status); + } else if(!read_float(block, &char_counter, &value)) { if(user_mcode == UserMCode_NoValueWords) // Valueless parameters allowed for user defined M-codes. value = NAN; // Parameter validation deferred to implementation. else @@ -956,9 +960,7 @@ status_code_t gc_execute_block (char *block) // a good enough compromise and catch most all non-integer errors. To make it compliant, // we would simply need to change the mantissa to int16, but this add compiled flash space. // Maybe update this later. - if(isnan(value)) - mantissa = 0; - else { + if(!isnanf(value)) { int_value = (uint32_t)truncf(value); mantissa = (uint_fast16_t)roundf(100.0f * (value - int_value)); } @@ -1462,7 +1464,7 @@ status_code_t gc_execute_block (char *block) if (mantissa > 0) FAIL(Status_GcodeCommandValueNotInteger); word_bit.parameter.o = On; - gc_block.values.o = isnan(value) ? 0xFFFFFFFF : int_value; + gc_block.values.o = int_value; break; case 'P': // NOTE: For certain commands, P value must be an integer, but none of these commands are supported. diff --git a/grbl.h b/grbl.h index 53a2126..c846921 100644 --- a/grbl.h +++ b/grbl.h @@ -42,7 +42,7 @@ #else #define GRBL_VERSION "1.1f" #endif -#define GRBL_BUILD 20241116 +#define GRBL_BUILD 20241120 #define GRBL_URL "https://github.com/grblHAL" diff --git a/grbllib.c b/grbllib.c index 3270127..4ac139b 100644 --- a/grbllib.c +++ b/grbllib.c @@ -279,7 +279,27 @@ int grbl_enter (void) if(driver.ok == 0xFF) driver.setup = hal.driver_setup(&settings); - if((driver.spindle = spindle_select(settings.spindle.flags.type))) { + uint8_t n_spindle = spindle_get_count(); +#if N_SPINDLE < 2 + spindle_id_t spindle_id = 0; +#else + spindle_id_t spindle_id = setting_get_int_value(setting_get_details(Setting_SpindleType, NULL), 0); +#endif + +// Sanity checks + if(spindle_id >= n_spindle) { + + spindle_ptrs_t *spindle = spindle_get_hal(0, SpindleHAL_Raw); + + spindle_id = 0; + settings.spindle.flags.type = spindle ? spindle->id : 0; // TODO: change to ref_id in next settings revision + } + + if(settings.offset_lock.encoder_spindle >= n_spindle) + settings.offset_lock.encoder_spindle = settings.spindle.flags.type; +// + + if((driver.spindle = spindle_select(spindle_id))) { spindle_ptrs_t *spindle = spindle_get(0); driver.spindle = spindle->get_pwm == NULL || spindle->update_pwm != NULL; } else @@ -318,7 +338,7 @@ int grbl_enter (void) setting_remove_elements(Setting_FSOptions, fs_options.mask); } - // Grbl initialization loop upon power-up or a system abort. For the latter, all processes + // Initialization loop upon power-up or a system abort. For the latter, all processes // will return to this loop to be cleanly re-initialized. while(looping) { @@ -346,7 +366,7 @@ int grbl_enter (void) flush_override_buffers(); - // Reset Grbl primary systems. + // Reset primary systems. hal.stream.reset_read_buffer(); // Clear input stream buffer gc_init(); // Set g-code parser to default state hal.limits.enable(settings.limits.flags.hard_enabled, (axes_signals_t){0}); @@ -373,7 +393,7 @@ int grbl_enter (void) if(hal.driver_cap.mpg_mode) protocol_enqueue_realtime_command(sys.mpg_mode ? CMD_STATUS_REPORT_ALL : CMD_STATUS_REPORT); - // Start Grbl main loop. Processes program inputs and executes them. + // Start main loop. Processes program inputs and executes them. if(!(looping = protocol_main_loop())) looping = hal.driver_release == NULL || hal.driver_release(); diff --git a/ngc_params.c b/ngc_params.c index ca57fb9..1dc03d1 100644 --- a/ngc_params.c +++ b/ngc_params.c @@ -827,7 +827,7 @@ char *ngc_string_param_get (ngc_string_id_t id) break; } while((sp = sp->next)); - return sp->value; + return sp ? sp->value : NULL; } bool ngc_string_param_exists (ngc_string_id_t id) diff --git a/nuts_bolts.c b/nuts_bolts.c index d32ce75..87f1414 100644 --- a/nuts_bolts.c +++ b/nuts_bolts.c @@ -306,7 +306,7 @@ bool read_float (char *line, uint_fast8_t *char_counter, float *float_ptr) // Returns true if float value is a whole number (integer) bool isintf (float value) { - return value != NAN && fabsf(value - truncf(value)) < 0.001f; + return !isnanf(value) && fabsf(value - truncf(value)) < 0.001f; } // Non-blocking delay function used for general operation and suspend features. diff --git a/pin_bits_masks.h b/pin_bits_masks.h index 6be6918..cd6ac50 100644 --- a/pin_bits_masks.h +++ b/pin_bits_masks.h @@ -137,7 +137,7 @@ #endif #endif -// Optional control signals, assigned to auxillary input pins +// Optional control signals, assigned to auxiliary input pins #ifndef MOTOR_FAULT_BIT #if defined(MOTOR_FAULT_PIN) && !MOTOR_FAULT_ENABLE diff --git a/report.c b/report.c index f8e9135..0ca2ec8 100644 --- a/report.c +++ b/report.c @@ -1120,12 +1120,16 @@ void report_echo_line_received (char *line) hal.stream.write("]" ASCII_EOL); } -#if N_SYS_SPINDLE == 1 && N_SPINDLE > 1 +#if N_SYS_SPINDLE == 1 && N_SPINDLE > 1 -static void report_spindle_num (spindle_info_t *spindle, void *data) +static bool report_spindle_num (spindle_info_t *spindle, void *data) { - if(spindle->id == *((spindle_id_t *)data)) + bool ok; + + if((ok = spindle->id == *((spindle_id_t *)data))) hal.stream.write_all(appendbuf(2, "|S:", uitoa((uint32_t)spindle->num))); + + return ok; } #endif @@ -2488,7 +2492,7 @@ status_code_t report_time (void) return ok ? Status_OK : Status_InvalidStatement; } -static void report_spindle (spindle_info_t *spindle, void *data) +static bool report_spindle (spindle_info_t *spindle, void *data) { if(data) { char *caps = buf; @@ -2550,6 +2554,8 @@ static void report_spindle (spindle_info_t *spindle, void *data) } hal.stream.write(ASCII_EOL); } + + return false; } #if N_SPINDLE > 1 @@ -2560,9 +2566,11 @@ typedef struct { spindle_info_t *spindles; } spindle_rdata_t; -static void get_spindles (spindle_info_t *spindle, void *data) +static bool get_spindles (spindle_info_t *spindle, void *data) { memcpy(&((spindle_rdata_t *)data)->spindles[((spindle_rdata_t *)data)->idx++], spindle, sizeof(spindle_info_t)); + + return false; } static int cmp_spindles (const void *a, const void *b) diff --git a/settings.c b/settings.c index e7550c4..122fa00 100644 --- a/settings.c +++ b/settings.c @@ -379,7 +379,7 @@ static status_code_t set_control_invert (setting_id_t id, uint_fast16_t int_valu static status_code_t set_spindle_invert (setting_id_t id, uint_fast16_t int_value); static status_code_t set_pwm_mode (setting_id_t id, uint_fast16_t int_value); static status_code_t set_pwm_options (setting_id_t id, uint_fast16_t int_value); -static status_code_t set_spindle_type (setting_id_t id, uint_fast16_t int_value); +static status_code_t set_default_spindle (setting_id_t id, uint_fast16_t int_value); static status_code_t set_encoder_spindle (setting_id_t id, uint_fast16_t int_value); static status_code_t set_control_disable_pullup (setting_id_t id, uint_fast16_t int_value); static status_code_t set_probe_disable_pullup (setting_id_t id, uint_fast16_t int_value); @@ -630,7 +630,7 @@ PROGMEM static const setting_detail_t setting_detail[] = { { Setting_DoorCoolantOnDelay, Group_SafetyDoor, "Coolant on delay", "s", Format_Decimal, "#0.0", "0.5", "20", Setting_IsExtended, &settings.safety_door.coolant_on_delay, NULL, is_setting_available }, #endif { Setting_SpindleOnDelay, Group_Spindle, "Spindle on delay", "s", Format_Decimal, "#0.0", "0.5", "20", Setting_IsExtended, &settings.safety_door.spindle_on_delay, NULL, is_setting_available }, - { Setting_SpindleType, Group_Spindle, "Default spindle", NULL, Format_RadioButtons, spindle_types, NULL, NULL, Setting_IsExtendedFn, set_spindle_type, get_int, is_setting_available }, + { Setting_SpindleType, Group_Spindle, "Default spindle", NULL, Format_RadioButtons, spindle_types, NULL, NULL, Setting_IsExtendedFn, set_default_spindle, get_int, is_setting_available }, { Setting_PlannerBlocks, Group_General, "Planner buffer blocks", NULL, Format_Int16, "####0", "30", "1000", Setting_IsExtended, &settings.planner_buffer_blocks, NULL, NULL, { .reboot_required = On } }, { Setting_AutoReportInterval, Group_General, "Autoreport interval", "ms", Format_Int16, "###0", "100", "1000", Setting_IsExtendedFn, set_report_interval, get_int, NULL, { .reboot_required = On, .allow_null = On } }, // { Setting_TimeZoneOffset, Group_General, "Timezone offset", NULL, Format_Decimal, "-#0.00", "0", "12", Setting_IsExtended, &settings.timezone, NULL, NULL }, @@ -678,7 +678,7 @@ PROGMEM static const setting_descr_t setting_descr[] = { "If Run substatus is enabled it may be used for simple probe protection.\\n\\n" "NOTE: Parser state will be sent separately after the status report and only on changes." }, - { Setting_JunctionDeviation, "Sets how fast Grbl travels through consecutive motions. Lower value slows it down." }, + { Setting_JunctionDeviation, "Sets how fast grblHAL travels through consecutive motions. Lower value slows it down." }, { Setting_ArcTolerance, "Sets the G2 and G3 arc tracing accuracy based on radial error. Beware: A very small value may effect performance." }, { Setting_ReportInches, "Enables inch units when returning any position and rate value that is not a settings value." }, { Setting_ControlInvertMask, "Inverts the control signals (active low).\\n" @@ -754,7 +754,7 @@ PROGMEM static const setting_descr_t setting_descr[] = { #endif { Setting_SleepEnable, "Enable sleep mode." }, { Setting_HoldActions, "Actions taken during feed hold and on resume from feed hold." }, - { Setting_ForceInitAlarm, "Starts Grbl in alarm mode after a cold reset." }, + { Setting_ForceInitAlarm, "Start in alarm mode after a cold reset." }, { Setting_ProbingFeedOverride, "Allow feed override during probing." }, #if ENABLE_SPINDLE_LINEARIZATION { Setting_LinearSpindlePiece1, "Comma separated list of values: RPM_MIN, RPM_LINE_A1, RPM_LINE_B1, set to blank to disable." }, @@ -1064,30 +1064,45 @@ static status_code_t set_pwm_options (setting_id_t id, uint_fast16_t int_value) return Status_OK; } -static status_code_t set_spindle_type (setting_id_t id, uint_fast16_t int_value) +typedef struct { + uint8_t ref_id; + spindle_id_t spindle_id; +} spindle_map_t; + +static bool get_spindle_ref_id (spindle_info_t *spindle, void *map) { - if(spindle_get_count() < 2) - return Status_SettingDisabled; - else if(int_value >= spindle_get_count()) - return Status_SettingValueOutOfRange; + bool ok; - settings.spindle.flags.type = int_value; + if((ok = spindle->id == ((spindle_map_t *)map)->spindle_id)) + ((spindle_map_t *)map)->ref_id = spindle->ref_id; - spindle_select(settings.spindle.flags.type); + return ok; +} - return Status_OK; +static status_code_t set_default_spindle (setting_id_t id, uint_fast16_t int_value) +{ + status_code_t status; + spindle_map_t spindle = { .spindle_id = int_value }; + + if((status = spindle_enumerate_spindles(get_spindle_ref_id, &spindle) ? Status_OK : Status_SettingValueOutOfRange) == Status_OK) { + + settings.spindle.flags.type = spindle.spindle_id; // TODO: change to ref_id in next settings revision + + spindle_select(spindle.spindle_id); + } + + return status; } static status_code_t set_encoder_spindle (setting_id_t id, uint_fast16_t int_value) { - if(spindle_get_count() < 2) - return Status_SettingDisabled; - else if(int_value >= spindle_get_count()) - return Status_SettingValueOutOfRange; + status_code_t status; + spindle_map_t spindle = { .spindle_id = int_value }; - settings.offset_lock.encoder_spindle = int_value; + if((status = spindle_enumerate_spindles(get_spindle_ref_id, &spindle) ? Status_OK : Status_SettingValueOutOfRange) == Status_OK) + settings.offset_lock.encoder_spindle = spindle.spindle_id; // TODO: change to ref_id in next settings revision - return Status_OK; + return status; } static status_code_t set_spindle_invert (setting_id_t id, uint_fast16_t int_value) @@ -1801,7 +1816,7 @@ static uint32_t get_int (setting_id_t id) break; case Setting_SpindleType: - value = settings.spindle.flags.type; + value = settings.spindle.flags.type; // TODO: change to ref_id in next settings revision break; case Setting_AutoReportInterval: @@ -2370,10 +2385,14 @@ void settings_write_global (void) #if N_SPINDLE > 1 -static void get_default_spindle (spindle_info_t *spindle, void *data) +static bool get_default_spindle (spindle_info_t *spindle, void *data) { - if(spindle->ref_id == (uint8_t)((uint32_t)data)) + bool ok; + + if((ok = spindle->ref_id == (uint8_t)((uint32_t)data))) settings.spindle.flags.type = spindle->id; + + return ok; } #endif @@ -3158,10 +3177,4 @@ void settings_init (void) } while((details = details->next)); setting_details.on_changed = hal.settings_changed; - - // Sanity checks for spindle configuration - if(settings.spindle.flags.type >= spindle_get_count()) - settings.spindle.flags.type = 0; - if(settings.offset_lock.encoder_spindle >= spindle_get_count()) - settings.offset_lock.encoder_spindle = 0; } diff --git a/spindle_control.c b/spindle_control.c index a78beb5..44777f5 100644 --- a/spindle_control.c +++ b/spindle_control.c @@ -302,7 +302,7 @@ static spindle_num_t spindle_get_num (spindle_id_t spindle_id) do { idx--; if((setting = setting_get_details(idx == 0 ? Setting_SpindleType : (setting_id_t)(Setting_SpindleEnable0 + idx), NULL))) { - if(setting_get_int_value(setting, 0) == spindle_id) + if(setting_get_int_value(setting, 0) - (idx == 0 ? 0 : 1) == spindle_id) spindle_num = idx; } } while(idx && spindle_num == -1); @@ -377,10 +377,11 @@ bool spindle_enumerate_spindles (spindle_enumerate_callback_ptr callback, void * spindle.hal = spindle.enabled && sys_spindle[spindle.num].hal.id == spindle.id ? &sys_spindle[spindle.num].hal : &spindles[idx].hal; spindle.is_current = spindle.enabled && sys_spindle[0].hal.id == idx; - callback(&spindle, data); + if(callback(&spindle, data)) + return true; } - return true; + return false; } // The following calls uses logical spindle numbers pointing into the sys_spindle array diff --git a/spindle_control.h b/spindle_control.h index d3e2c5f..aa3ba40 100644 --- a/spindle_control.h +++ b/spindle_control.h @@ -334,7 +334,7 @@ typedef struct { /*! \brief Pointer to callback function called by spindle_enumerate_spindles(). \param spindle prointer to a \a spindle_info_t struct. */ -typedef void (*spindle_enumerate_callback_ptr)(spindle_info_t *spindle, void *data); +typedef bool (*spindle_enumerate_callback_ptr)(spindle_info_t *spindle, void *data); void spindle_set_override (spindle_ptrs_t *spindle, override_t speed_override); diff --git a/system.c b/system.c index c8366b3..24d8afa 100644 --- a/system.c +++ b/system.c @@ -789,7 +789,7 @@ const char *help_pins (const char *cmd) const char *help_pin_state (const char *cmd) { - return hal.port.get_pin_info ? "output auxillary pin states" : NULL; + return hal.port.get_pin_info ? "output auxiliary pin states" : NULL; } #endif