From 06f442643a115ab3acd6722b4afc1fcee8d9d7be Mon Sep 17 00:00:00 2001 From: Terje Io Date: Wed, 28 Feb 2024 17:36:17 +0100 Subject: [PATCH] Tentative fix for lathe CSS/feed per rev modes (G96/G95), ref discussion #450. Fixed incorrect reporting of feed rate modes in $G response. Added some ioPorts call wrappers, full support for reconfiguring output auxillary pins for PWM output. --- changelog.md | 22 ++++++++++++ crossbar.h | 5 +++ gcode.c | 17 ++++++---- grbl.h | 2 +- ioports.c | 95 +++++++++++++++++++++++++++++----------------------- ioports.h | 2 ++ planner.c | 18 +++++----- planner.h | 13 +++---- report.c | 6 ++-- settings.c | 29 +++++++++------- 10 files changed, 132 insertions(+), 77 deletions(-) diff --git a/changelog.md b/changelog.md index a956b3e..9076d46 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,27 @@ ## grblHAL changelog +Build 20240228 + +Core: + +* Tentative fix for lathe CSS/feed per rev modes \(G96/G95\), ref discussion [#450](https://github.com/grblHAL/core/discussions/450). + +* Fixed incorrect reporting of feed rate modes in `$G` response. + +* Added some ioPorts call wrappers, full support for reconfiguring output auxillary pins for PWM output. + +Drivers: + +* STM32F1xx, ST32F4xx, STM32F7xx: fix for incorrect status returned from ioPort configuration call. + +* STM32F7xx: added full support for reconfiguring capable auxillary output pins for PWM. + +Plugins: + +* WebUI: added update that did not make it in the previous one... + +--- + Build 20240226 Core: diff --git a/crossbar.h b/crossbar.h index e7259ed..24af322 100644 --- a/crossbar.h +++ b/crossbar.h @@ -81,6 +81,7 @@ typedef enum { Input_Aux5, Input_Aux6, Input_Aux7, + Input_AuxMax = Input_Aux7, Input_Analog_Aux0, Input_Analog_Aux1, Input_Analog_Aux2, @@ -89,6 +90,7 @@ typedef enum { Input_Analog_Aux5, Input_Analog_Aux6, Input_Analog_Aux7, + Input_Analog_AuxMax = Input_Analog_Aux7, // Output pins Output_StepX, Outputs = Output_StepX, @@ -147,6 +149,7 @@ typedef enum { Output_Aux5, Output_Aux6, Output_Aux7, + Output_AuxMax = Output_Aux7, Output_Analog_Aux0, Output_Analog_Aux1, Output_Analog_Aux2, @@ -155,6 +158,7 @@ typedef enum { Output_Analog_Aux5, Output_Analog_Aux6, Output_Analog_Aux7, + Output_Analog_AuxMax = Output_Analog_Aux7, Output_LED, Output_LED_R, Output_LED_G, @@ -537,6 +541,7 @@ typedef struct { typedef struct { bool inverted; bool open_drain; + bool pwm; } gpio_out_config_t; //! /a cfg_data argument to /a xbar_config_ptr for PWM pins diff --git a/gcode.c b/gcode.c index 8a8c651..bfd01b4 100644 --- a/gcode.c +++ b/gcode.c @@ -7,18 +7,18 @@ Copyright (c) 2011-2016 Sungeun K. Jeon for Gnea Research LLC Copyright (c) 2009-2011 Simen Svale Skogsrud - Grbl is free software: you can redistribute it and/or modify + grblHAL is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. - Grbl is distributed in the hope that it will be useful, + grblHAL is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License - along with Grbl. If not, see . + along with grblHAL. If not, see . */ #include @@ -3013,7 +3013,7 @@ status_code_t gc_execute_block (char *block) gc_state.spindle.css->tool_offset = gc_get_offset(gc_state.spindle.css->axis); float pos = gc_state.position[gc_state.spindle.css->axis] - gc_state.spindle.css->tool_offset; gc_block.values.s = pos <= 0.0f ? gc_state.spindle.css->max_rpm : min(gc_state.spindle.css->max_rpm, gc_state.spindle.css->surface_speed / (pos * (float)(2.0f * M_PI))); - gc_parser_flags.spindle_force_sync = On; +//?? gc_parser_flags.spindle_force_sync = On; } else { if(gc_state.spindle.css) { gc_state.spindle.css = NULL; @@ -3413,7 +3413,8 @@ status_code_t gc_execute_block (char *block) case MotionMode_Linear: if(gc_state.modal.feed_mode == FeedMode_UnitsPerRev) { - plan_data.spindle.state.synchronized = On; + plan_data.condition.units_per_rev = On; + plan_data.spindle.state.synchronized = settings.mode != Mode_Lathe || gc_block.values.xyz[Z_AXIS] != gc_state.position[Z_AXIS]; //?? gc_state.distance_per_rev = plan_data.feed_rate; // check initial feed rate - fail if zero? } @@ -3427,7 +3428,9 @@ status_code_t gc_execute_block (char *block) case MotionMode_CwArc: case MotionMode_CcwArc: - // fail if spindle synchronized motion? + if(gc_state.modal.feed_mode == FeedMode_UnitsPerRev) + plan_data.condition.units_per_rev = plan_data.spindle.state.synchronized = On; + mc_arc(gc_block.values.xyz, &plan_data, gc_state.position, gc_block.values.ijk, gc_block.values.r, plane, gc_parser_flags.arc_is_clockwise ? -gc_block.arc_turns : gc_block.arc_turns); break; diff --git a/grbl.h b/grbl.h index a693f98..7dcc059 100644 --- a/grbl.h +++ b/grbl.h @@ -42,7 +42,7 @@ #else #define GRBL_VERSION "1.1f" #endif -#define GRBL_BUILD 20240226 +#define GRBL_BUILD 20240228 #define GRBL_URL "https://github.com/grblHAL" diff --git a/ioports.c b/ioports.c index 2068800..b6b3bae 100644 --- a/ioports.c +++ b/ioports.c @@ -148,10 +148,24 @@ void ioport_assign_function (aux_ctrl_t *aux_ctrl, pin_function_t *function) if((input = hal.port.get_pin_info(Port_Digital, Port_Input, aux_ctrl->aux_port))) { *function = aux_ctrl->function; + digital.inx.mask &= ~(1 << input->id); hal.signals_cap.mask |= aux_ctrl->cap.mask; if(aux_ctrl->function == Input_Probe || xbar_fn_to_signals_mask(aux_ctrl->function).mask) - setting_remove_elements(Settings_IoPort_InvertIn, digital.inx.mask & ~(1 << input->id)); + setting_remove_elements(Settings_IoPort_InvertIn, digital.inx.mask); + } +} + +void ioport_assign_out_function (aux_ctrl_out_t *aux_ctrl, pin_function_t *function) +{ + xbar_t *output; + + if((output = hal.port.get_pin_info(Port_Digital, Port_Output, aux_ctrl->aux_port))) { + + *function = aux_ctrl->function; + digital.outx.mask &= ~(1 << output->id); + + setting_remove_elements(Settings_IoPort_InvertOut, digital.outx.mask); } } @@ -180,24 +194,18 @@ bool ioports_enumerate (io_port_type_t type, io_port_direction_t dir, pin_cap_t bool ioport_analog_out_config (uint8_t port, pwm_config_t *config) { - bool ok; xbar_t *pin; + bool ok = (pin = hal.port.get_pin_info(Port_Analog, Port_Output, port)) && pin->config; - if((ok = (pin = hal.port.get_pin_info(Port_Analog, Port_Output, port)) && pin->config)) - pin->config(pin, config, false); - - return ok; + return ok && pin->config(pin, config, false); } bool ioport_digital_in_config (uint8_t port, gpio_in_config_t *config) { - bool ok; xbar_t *pin; + bool ok = (pin = hal.port.get_pin_info(Port_Digital, Port_Input, port)) && pin->config; - if((ok = (pin = hal.port.get_pin_info(Port_Digital, Port_Input, port)) && pin->config)) - pin->config(pin, config, false); - - return ok; + return ok && pin->config(pin, config, false); } bool ioport_enable_irq (uint8_t port, pin_irq_mode_t irq_mode, ioport_interrupt_callback_ptr handler) @@ -207,13 +215,18 @@ bool ioport_enable_irq (uint8_t port, pin_irq_mode_t irq_mode, ioport_interrupt_ bool ioport_digital_out_config (uint8_t port, gpio_out_config_t *config) { - bool ok; xbar_t *pin; + bool ok = (pin = hal.port.get_pin_info(Port_Digital, Port_Output, port)) && pin->config && !(pin->mode.pwm || pin->mode.servo_pwm); - if((ok = (pin = hal.port.get_pin_info(Port_Digital, Port_Output, port)) && pin->config)) - pin->config(pin, config, false); + return ok && pin->config(pin, config, false); +} - return ok; +bool ioport_digital_pwm_config (uint8_t port, pwm_config_t *config) +{ + xbar_t *pin; + bool ok = (pin = hal.port.get_pin_info(Port_Digital, Port_Output, port)) && pin->config && pin->mode.claimed && pin->cap.pwm; + + return ok && pin->config(pin, config, false); } /* experimental code follows */ @@ -394,39 +407,38 @@ uint_fast16_t ioports_compute_pwm_value (ioports_pwm_t *pwm_data, float value) return pwm_value; } -static void sync_input_settings (pin_function_t function, gpio_in_config_t *config) +void ioport_save_input_settings (xbar_t *xbar, gpio_in_config_t *config) { - if(function == Input_Probe) + if(digital.inx.mask & (1 << xbar->id)) { + if(config->inverted) + settings.ioport.invert_in.mask |= (1 << xbar->id); + else + settings.ioport.invert_in.mask &= ~(1 << xbar->id); + } + + if(xbar->function == Input_Probe) settings.probe.invert_probe_pin = config->inverted; - else if(function < Input_Probe) { + else if(xbar->function < Input_Probe) { control_signals_t ctrl; - if((ctrl = xbar_fn_to_signals_mask(function)).mask) { + if((ctrl = xbar_fn_to_signals_mask(xbar->function)).mask) { if(config->inverted) settings.control_invert.mask |= ctrl.mask; else settings.control_invert.mask &= ~ctrl.mask; } } -} - -void ioport_save_input_settings (xbar_t *xbar, gpio_in_config_t *config) -{ - if(config->inverted) - settings.ioport.invert_in.mask |= (1 << xbar->id); - else - settings.ioport.invert_in.mask &= ~(1 << xbar->id); - - sync_input_settings(xbar->function, config); settings_write_global(); } void ioport_save_output_settings (xbar_t *xbar, gpio_out_config_t *config) { - if(config->inverted) - settings.ioport.invert_out.mask |= (1 << xbar->id); - else - settings.ioport.invert_out.mask &= ~(1 << xbar->id); + if(digital.outx.mask & (1 << xbar->id)) { + if(config->inverted) + settings.ioport.invert_out.mask |= (1 << xbar->id); + else + settings.ioport.invert_out.mask &= ~(1 << xbar->id); + } settings_write_global(); } @@ -439,12 +451,12 @@ static bool is_setting_available (const setting_detail_t *setting) case Settings_IoPort_InvertIn: case Settings_IoPort_Pullup_Disable: - available = digital.in.ports && digital.in.ports->n_ports > 0; + available = digital.in.ports && digital.inx.mask; break; case Settings_IoPort_InvertOut: case Settings_IoPort_OD_Enable: - available = digital.out.ports && digital.out.ports->n_ports > 0; + available = digital.out.ports && digital.outx.mask; break; default: @@ -476,7 +488,6 @@ static status_code_t aux_set_value (setting_id_t id, uint_fast16_t value) config.pull_mode = (pull_mode_t)xbar->mode.pull_mode; config.inverted = !!(change.mask & (1 << xbar->id)); xbar->config(xbar, &config, false); - sync_input_settings(xbar->function, &config); } } port++; @@ -504,7 +515,6 @@ static status_code_t aux_set_value (setting_id_t id, uint_fast16_t value) config.inverted = xbar->mode.inverted; config.debounce = xbar->mode.inverted; xbar->config(xbar, &config, false); - sync_input_settings(xbar->function, &config); } } port++; @@ -527,7 +537,7 @@ static status_code_t aux_set_value (setting_id_t id, uint_fast16_t value) do { if((changed.mask & 0x01) && (xbar = hal.port.get_pin_info(Port_Digital, Port_Output, ioports_map_reverse(digital.out.ports, port)))) { - if(xbar->config) { + if(xbar->config && !(xbar->mode.pwm || xbar->mode.servo_pwm)) { config.inverted = !!(change.mask & (1 << xbar->id)); config.open_drain = xbar->mode.open_drain; xbar->config(xbar, &config, false); @@ -553,7 +563,7 @@ static status_code_t aux_set_value (setting_id_t id, uint_fast16_t value) do { if((changed.mask & 0x01) && (xbar = hal.port.get_pin_info(Port_Digital, Port_Output, ioports_map_reverse(digital.out.ports, port)))) { - if(xbar->config) { + if(xbar->config && !(xbar->mode.pwm || xbar->mode.servo_pwm)) { config.inverted = xbar->mode.inverted; config.open_drain = !!(change.mask & (1 << xbar->id)); xbar->config(xbar, &config, false); @@ -636,7 +646,10 @@ static void ioport_settings_load (void) gpio_in_config_t in_config = {0}; gpio_out_config_t out_config = {0}; - // TODO: change labels for mapped settings? + settings.ioport.invert_in.mask &= digital.inx.mask; + settings.ioport.pullup_disable_in.mask &= digital.inx.mask; + settings.ioport.invert_out.mask &= digital.outx.mask; + settings.ioport.od_enable_out.mask &= digital.outx.mask; if(digital.in.ports && (port = digital.in.ports->n_ports)) do { if((xbar = hal.port.get_pin_info(Port_Digital, Port_Input, ioports_map_reverse(digital.in.ports, --port)))) { @@ -671,7 +684,7 @@ static void ioport_settings_load (void) if(digital.out.ports && (port = digital.out.ports->n_ports)) do { if((xbar = hal.port.get_pin_info(Port_Digital, Port_Output, ioports_map_reverse(digital.in.ports, --port)))) { - if(xbar->config) { + if(xbar->config && !(xbar->mode.pwm || xbar->mode.servo_pwm)) { out_config.inverted = !!(settings.ioport.invert_out.mask & (1 << xbar->id)); out_config.open_drain = !!(settings.ioport.od_enable_out.mask & (1 << xbar->id)); xbar->config(xbar, &out_config, false); diff --git a/ioports.h b/ioports.h index d4395d8..0e47dfe 100644 --- a/ioports.h +++ b/ioports.h @@ -128,7 +128,9 @@ bool ioport_claim (io_port_type_t type, io_port_direction_t dir, uint8_t *port, bool ioport_can_claim_explicit (void); bool ioports_enumerate (io_port_type_t type, io_port_direction_t dir, pin_cap_t filter, ioports_enumerate_callback_ptr callback, void *data); void ioport_assign_function (aux_ctrl_t *aux_ctrl, pin_function_t *function); +void ioport_assign_out_function (aux_ctrl_out_t *aux_ctrl, pin_function_t *function); bool ioport_analog_out_config (uint8_t port, pwm_config_t *config); +bool ioport_digital_pwm_config (uint8_t port, pwm_config_t *config); bool ioport_digital_in_config (uint8_t port, gpio_in_config_t *config); bool ioport_enable_irq (uint8_t port, pin_irq_mode_t irq_mode, ioport_interrupt_callback_ptr handler); bool ioport_digital_out_config (uint8_t port, gpio_out_config_t *config); diff --git a/planner.c b/planner.c index 65e2989..fce0b4a 100644 --- a/planner.c +++ b/planner.c @@ -8,18 +8,18 @@ Copyright (c) 2009-2011 Simen Svale Skogsrud Copyright (c) 2011 Jens Geisler - Grbl is free software: you can redistribute it and/or modify + grblHAL is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. - Grbl is distributed in the hope that it will be useful, + grblHAL is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License - along with Grbl. If not, see . + along with grblHAL. If not, see . */ #include @@ -310,14 +310,16 @@ bool plan_check_full_buffer (void) // NOTE: All system motion commands, such as homing/parking, are not subject to overrides. float plan_compute_profile_nominal_speed (plan_block_t *block) { - float nominal_speed = block->spindle.state.synchronized ? block->programmed_rate * block->spindle.hal->get_data(SpindleData_RPM)->rpm : block->programmed_rate; + float nominal_speed = block->condition.units_per_rev || block->spindle.state.synchronized + ? block->programmed_rate * block->spindle.hal->get_data(SpindleData_RPM)->rpm + : block->programmed_rate; - if (block->condition.rapid_motion) + if(block->condition.rapid_motion) nominal_speed *= (0.01f * (float)sys.override.rapid_rate); else { - if (!block->condition.no_feed_override) + if(!block->condition.no_feed_override) nominal_speed *= (0.01f * (float)sys.override.feed_rate); - if (nominal_speed > block->rapid_rate) + if(nominal_speed > block->rapid_rate) nominal_speed = block->rapid_rate; } diff --git a/planner.h b/planner.h index 1f44369..b4c776f 100644 --- a/planner.h +++ b/planner.h @@ -3,22 +3,22 @@ Part of grblHAL - Copyright (c) 2019-2023 Terje Io + Copyright (c) 2019-2024 Terje Io Copyright (c) 2011-2016 Sungeun K. Jeon for Gnea Research LLC Copyright (c) 2009-2011 Simen Svale Skogsrud - Grbl is free software: you can redistribute it and/or modify + grblHAL is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. - Grbl is distributed in the hope that it will be useful, + grblHAL is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License - along with Grbl. If not, see . + along with grblHAL. If not, see . */ #ifndef _PLANNER_H_ @@ -33,11 +33,12 @@ typedef union { backlash_motion :1, no_feed_override :1, inverse_time :1, + units_per_rev :1, is_rpm_rate_adjusted :1, is_laser_ppi_mode :1, target_valid :1, target_validated :1, - unassigned :6; + unassigned :5; coolant_state_t coolant; }; } planner_cond_t; diff --git a/report.c b/report.c index 1d653d6..5e02aaa 100644 --- a/report.c +++ b/report.c @@ -698,7 +698,7 @@ void report_gcode_modes (void) hal.stream.write(gc_state.modal.distance_incremental ? " G91" : " G90"); hal.stream.write(" G"); - hal.stream.write(uitoa((uint32_t)(94 - gc_state.modal.feed_mode))); + hal.stream.write(uitoa((uint32_t)(93 + (gc_state.modal.feed_mode == FeedMode_UnitsPerRev ? 2 : gc_state.modal.feed_mode ^ 1)))); if(settings.mode == Mode_Lathe && gc_spindle_get()->cap.variable) hal.stream.write(gc_state.modal.spindle.rpm_mode == SpindleSpeedMode_RPM ? " G97" : " G96"); @@ -2341,14 +2341,16 @@ status_code_t report_pin_states (sys_state_t state, char *args) for(idx = 0; idx < ports; idx++) { if((port = hal.port.get_pin_info(Port_Digital, Port_Output, idx))) { hal.stream.write("[PINSTATE:DOUT|"); - hal.stream.write(port->description); + hal.stream.write(port->description ? port->description : xbar_fn_to_pinname(port->function)); hal.stream.write("|"); hal.stream.write(uitoa(port->id)); hal.stream.write("|"); hal.stream.write(port->mode.inverted ? "I" : "N"); +// hal.stream.write(port->mode.pwm ? "P" : (port->mode.servo_pwm ? "S" : "N")); // hal.stream.write(port->mode.open_drain ? "O" : "-"); hal.stream.write("|"); hal.stream.write(port->cap.invert ? "I" : "-"); +// hal.stream.write(port->cap.pwm ? "P" : (port->cap.servo_pwm ? "S" : "N")); // hal.stream.write(port->cap.open_drain ? "O" : "-"); hal.stream.write("|"); hal.stream.write(port->get_value ? uitoa((uint32_t)port->get_value(port)) : "?"); diff --git a/settings.c b/settings.c index 9721692..a2bb2f1 100644 --- a/settings.c +++ b/settings.c @@ -2561,20 +2561,25 @@ static void setting_remove_element (setting_id_t id, uint_fast8_t pos) // Note: setting format string has to reside in RAM. void setting_remove_elements (setting_id_t id, uint32_t mask) { - char *format = (char *)setting_get_details(id, NULL)->format, *s; - uint_fast8_t idx, entries = strnumentries(format, ','); + const setting_detail_t *setting; - for(idx = 0; idx < entries; idx++ ) { - if(!(mask & 0x1)) - setting_remove_element(id, idx); - mask >>= 1; - } + if((setting = setting_get_details(id, NULL))) { - // Strip trailing N/A's - while((s = strrchr(format, ','))) { - if(strncmp(s, ",N/A", 4)) - break; - *s = '\0'; + char *format = (char *)setting->format, *s; + uint_fast8_t idx, entries = strnumentries(format, ','); + + for(idx = 0; idx < entries; idx++ ) { + if(!(mask & 0x1)) + setting_remove_element(id, idx); + mask >>= 1; + } + + // Strip trailing N/A's + while((s = strrchr(format, ','))) { + if(strncmp(s, ",N/A", 4)) + break; + *s = '\0'; + } } }