Improved expression and parameter handling, simplified gcode parser related to this. Fixed typo in OR statement decode.

Added clear of return value on CALL statement and added optional return value expression support to ENDSUB.
Improved handling of multiple simultaneous spindles. Still work in progress.
For developers: moved user M-code entry point hooks from the HAL structure to the core handlers and changed signature of the check() function to better support valueless words (letters) when parameter support is enabled. Removed deprecated parameter from the validate() call signature.

Author: terjeio

README.md

@@ -13,7 +13,7 @@ It has been written to complement grblHAL and has features such as proper keyboa
 
 ---
 
-Latest build date is 20241006, see the [changelog](changelog.md) for details.  
+Latest build date is 20241014, 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.

changelog.md

@@ -1,6 +1,31 @@
 ## grblHAL changelog
 
-<a name="2024106">Build 2024106
+<a name="20241014">Build 20241014
+
+Core:
+
+* Improved expression and parameter handling, simplified gcode parser related to this. Fixed typo in `OR` statement decode. 
+
+* Added clear of return value on `CALL` statement and added optional return value expression support to `ENDSUB`.
+
+* For developers: moved user M-code entry point hooks from the HAL structure to the core handlers and
+changed signature of the _check()_ function to better support valueless words \(letters\) when parameter support is enabled.
+Removed deprecated parameter from the _validate()_ call signature.
+
+* Improved handling of multiple simultaneous spindles. Still work in progress.
+
+Drivers: 
+
+* STM32F4xx and STM32F7xx: fixed some typos in new timer API. Ref. [issue #192](https://github.com/grblHAL/STM32F4xx/issues/192)
+and [issue #18](https://github.com/grblHAL/STM32F7xx/issues/18).
+
+Plugins and templates:
+
+* Some updated for move of core M-code entry points.
+
+---
+
+<a name="20241006">Build 20241006
 
 Core:
 

core_handlers.h

@@ -140,6 +140,69 @@ typedef struct {
     clear_tool_data_ptr clear;
 } tool_table_t;
 
+/*****************
+ *  User M-code  *
+ *****************/
+
+typedef enum {
+    UserMCode_Unsupported = 0,  //!< 0 - M-code is not supported.
+    UserMCode_Normal,           //!< 1 - M-code is supported.
+    UserMCode_NoValueWords      //!< 2 - M-code supports valueless parameter words.
+} user_mcode_type_t;
+
+/*! \brief Pointer to function for checking if user defined M-code is supported.
+\param mcode as a #user_mcode_t enum.
+\returns #UserMCode_Normal or #UserMCode_NoValueWords if M-code is handled, #UserMCode_Unsupported if not.
+*/
+typedef user_mcode_type_t (*user_mcode_check_ptr)(user_mcode_t mcode);
+
+/*! \brief Pointer to function for validating parameters for a user defined M-code.
+
+The M-code to validate is found in \a gc_block->user_mcode, parameter values in \a gc_block->values
+ and corresponding parameter letters in the \a gc_block->words bitfield union.
+
+Parameter values claimed by the M-code must be flagged in the \a gc_block->words bitfield union by setting the
+ respective parameter letters to \a false or the parser will raise the #Status_GcodeUnusedWords error.
+
+The validation function may set \a gc_block->user_mcode_sync to \a true if it is to be executed
+after all buffered motions are completed, otherwise it will be executed immediately.
+
+__NOTE:__ Valueless parameter letters are allowed for floats if the check function returns
+#UserMCode_NoValueWords for the M-code. The corresponding values are set to NAN (not a number)
+if no value is given.
+The validation function should always test all parameter values by using the isnan() function in addition
+to any range checks when the check function returns #UserMCode_NoValueWords for the M-code.
+
+\param gc_block pointer to a parser_block_t structure.
+\returns #Status_OK enum value if validated ok, appropriate \ref status_code_t enum value if not or #Status_Unhandled if the M-code is not recognized.
+*/
+typedef status_code_t (*user_mcode_validate_ptr)(parser_block_t *gc_block);
+
+/*! \brief Pointer to function for executing a user defined M-code.
+
+The M-code to execute is found in \a gc_block->user_mcode, parameter values in \a gc_block->values
+ and claimed/validated parameter letters in the \a gc_block->words bitfield union.
+
+\param state as a #sys_state_t variable.
+\param gc_block pointer to a parser_block_t structure.
+\returns #Status_OK enum value if validated ok, appropriate \ref status_code_t enum value if not or #Status_Unhandled if M-code is not recognized.
+*/
+typedef void (*user_mcode_execute_ptr)(sys_state_t state, parser_block_t *gc_block);
+
+/*! \brief Optional handlers for user defined M-codes.
+
+Handlers may be chained so that several plugins can add M-codes.
+Chaining is achieved by saving a copy of the current user_mcode_ptrs_t struct
+ when the plugin is initialized and calling the same handler via the copy when a
+ M-code is not recognized.
+ */
+typedef struct {
+    user_mcode_check_ptr check;         //!< Handler for checking if a user defined M-code is supported.
+    user_mcode_validate_ptr validate;   //!< Handler for validating parameters for a user defined M-code.
+    user_mcode_execute_ptr execute;     //!< Handler for executing a user defined M-code.
+} user_mcode_ptrs_t;
+
+
 typedef struct {
     // report entry points set by core at reset.
     report_t report;
@@ -189,6 +252,7 @@ typedef struct {
     on_spindle_selected_ptr on_spindle_selected;        //!< Called when spindle is selected, do not change HAL pointers here!
     on_reset_ptr on_reset;                              //!< Called from interrupt context.
     on_file_open_ptr on_file_open;                      //!< Called when a file is opened for streaming.
+    user_mcode_ptrs_t user_mcode;                       //!< Optional handlers for user defined M-codes.
     // core entry points - set up by core before driver_init() is called.
     home_machine_ptr home_machine;
     travel_limits_ptr check_travel_limits;

gcode.c

@@ -144,6 +144,11 @@ inline static bool motion_is_lasercut (motion_mode_t motion)
     return motion == MotionMode_Linear || motion == MotionMode_CwArc || motion == MotionMode_CcwArc || motion == MotionMode_CubicSpline || motion == MotionMode_QuadraticSpline;
 }
 
+inline static bool no_word_value (char letter)
+{
+    return letter == '\0' || (letter >= 'A' && letter <= 'Z') || letter == '$';
+}
+
 parser_state_t *gc_get_state (void)
 {
     return &gc_state;
@@ -348,6 +353,21 @@ void gc_init (void)
         grbl.on_parser_init(&gc_state);
 }
 
+#if N_SYS_SPINDLE > 1
+
+inline static bool is_single_spindle_block (parser_block_t *gc_block, modal_groups_t command_words)
+{
+    return gc_block->words.s ||
+            (command_words.G1 && (gc_block->modal.motion == MotionMode_SpindleSynchronized ||
+                                   gc_block->modal.motion == MotionMode_RigidTapping ||
+                                    gc_block->modal.motion == MotionMode_Threading)) ||
+             (command_words.G5 && gc_block->modal.feed_mode == FeedMode_UnitsPerRev) ||
+               command_words.G14 ||
+               (command_words.M9 && gc_block->override_command == Override_SpindleSpeed);
+}
+
+#endif
+
 // Set dynamic laser power mode to PPI (Pulses Per Inch)
 // Returns true if driver uses hardware implementation.
 // Driver support for pulsing the laser on signal is required for this to work.
@@ -487,54 +507,6 @@ void gc_output_message (char *message)
 
 static ngc_param_t ngc_params[NGC_N_ASSIGN_PARAMETERS_PER_BLOCK];
 
-static status_code_t read_parameter (char *line, uint_fast8_t *char_counter, float *value)
-{
-    char c = *(line + *char_counter);
-    status_code_t status = Status_OK;
-
-    if(c == '#') {
-
-        (*char_counter)++;
-
-        if(*(line + *char_counter) == '<') {
-
-            (*char_counter)++;
-            char *pos = line = line + *char_counter;
-
-            while(*line && *line != '>') {
-                if(*line == ' ') {
-                    char *s1 = line, *s2 = line + 1;
-                    while(*s2)
-                        *s1++ = *s2++;
-                    *(--s2) = '\0';
-                } else
-                    line++;
-            }
-
-            *char_counter += line - pos + 1;
-
-            if(*line == '>') {
-                *line = '\0';
-                if(!ngc_named_param_get(pos, value))
-                    status = Status_BadNumberFormat;
-                *line = '>';
-            } else
-                status = Status_BadNumberFormat;
-
-        } else if(read_float(line, char_counter, value)) {
-            if(!ngc_param_get((ngc_param_id_t)*value, value))
-                status = Status_BadNumberFormat;
-        } else
-            status = Status_BadNumberFormat;
-
-    } else if(c == '[')
-        status = ngc_eval_expression(line, char_counter, value);
-    else if(!read_float(line, char_counter, value))
-        *value = NAN;
-
-    return status;
-}
-
 static int8_t get_format (char c, int8_t pos, uint8_t *decimals)
 {
     static uint8_t d;
@@ -611,7 +583,7 @@ static void substitute_parameters (char *comment, char **message)
             parse_format = 1;
         else if(c == '#') {
             char_counter--;
-            if(read_parameter(comment, &char_counter, &value) == Status_OK)
+            if(ngc_read_parameter(comment, &char_counter, &value, true) == Status_OK)
                 len += strlen(decimals ? ftoa(value, decimals) : trim_float(ftoa(value, decimals)));
             else
                 len += 3; // "N/A"
@@ -642,7 +614,7 @@ static void substitute_parameters (char *comment, char **message)
                 fmt[0] = c;
             } else if(c == '#') {
                 char_counter--;
-                if(read_parameter(comment, &char_counter, &value) == Status_OK)
+                if(ngc_read_parameter(comment, &char_counter, &value, true) == Status_OK)
                     strcat(s, decimals ? ftoa(value, decimals) : trim_float(ftoa(value, decimals)));
                 else
                     strcat(s, "N/A");
@@ -1006,7 +978,7 @@ status_code_t gc_execute_block (char *block)
     float value;
     uint32_t int_value = 0;
     uint_fast16_t mantissa = 0;
-    bool is_user_mcode = false;
+    user_mcode_type_t user_mcode = UserMCode_Unsupported;
     word_bit_t word_bit = { .parameter = {0}, .modal_group = {0} }; // Bit-value for assigning tracking variables
 
     while ((letter = block[char_counter++]) != '\0') { // Loop until no more g-code words in block.
@@ -1034,7 +1006,7 @@ status_code_t gc_execute_block (char *block)
                     *s++ = '\0';
                     s++;
                     char_counter += s - name;
-                    if((status = read_parameter(block, &char_counter, &value)) != Status_OK)
+                    if((status = ngc_read_real_value(block, &char_counter, &value)) != Status_OK)
                         FAIL(status);   // [Expected parameter value]
                     if(!ngc_named_param_set(name, value))
                         FAIL(Status_BadNumberFormat);   // [Expected equal sign]
@@ -1043,13 +1015,16 @@ status_code_t gc_execute_block (char *block)
             } else {
 
                 float param;
-                if (!read_float(block, &char_counter, &param))
-                    FAIL(Status_BadNumberFormat);   // [Expected parameter number]
 
-                if (block[char_counter++] != '=')
+                if((status = ngc_read_real_value(block, &char_counter, &param)) != Status_OK) {
+                    FAIL(status);   // [Expected parameter number]
+                } else if(!ngc_param_is_rw((ngc_param_id_t)param))
+                    FAIL(Status_GcodeValueOutOfRange);   // [Parameter does not exist or is read only]
+
+                if(block[char_counter++] != '=')
                     FAIL(Status_BadNumberFormat);   // [Expected equal sign]
 
-                if((status = read_parameter(block, &char_counter, &value)) != Status_OK)
+                if((status = ngc_read_real_value(block, &char_counter, &value)) != Status_OK)
                     FAIL(status);   // [Expected parameter value]
 
                 if(ngc_param_count < NGC_N_ASSIGN_PARAMETERS_PER_BLOCK && ngc_param_is_rw((ngc_param_id_t)param)) {
@@ -1070,13 +1045,15 @@ status_code_t gc_execute_block (char *block)
         if((letter < 'A' && letter != '$') || letter > 'Z')
             FAIL(Status_ExpectedCommandLetter); // [Expected word letter]
 
-        if((status = read_parameter(block, &char_counter, &value)) != Status_OK)
+        if(user_mcode == UserMCode_NoValueWords && no_word_value(block[char_counter]))
+            value = NAN;
+        else if((status = ngc_read_real_value(block, &char_counter, &value)) != Status_OK)
             return status;
 
         if(gc_state.skip_blocks && letter != 'O')
             return Status_OK;
 
-        if(!is_user_mcode && isnanf(value))
+        if(user_mcode != UserMCode_NoValueWords && isnanf(value))
             FAIL(Status_BadNumberFormat);   // [Expected word value]
 
         g65_words.value = 0;
@@ -1085,11 +1062,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(is_user_mcode)                   // Valueless parameters allowed for user defined M-codes.
-                value = NAN;                    // Parameter validation deferred to implementation.
+        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
-                FAIL(Status_BadNumberFormat);   // [Expected word value]
+                FAIL(Status_BadNumberFormat);           // [Expected word value]
         }
 
 #endif
@@ -1118,7 +1095,7 @@ status_code_t gc_execute_block (char *block)
 
             case 'G': // Determine 'G' command and its modal group
 
-                is_user_mcode = false;
+                user_mcode = UserMCode_Unsupported;
                 word_bit.modal_group.mask = 0;
 
                 switch(int_value) {
@@ -1358,7 +1335,7 @@ status_code_t gc_execute_block (char *block)
                 if(mantissa > 0)
                     FAIL(Status_GcodeCommandValueNotInteger); // [No Mxx.x commands]
 
-                is_user_mcode = false;
+                user_mcode = UserMCode_Unsupported;
                 word_bit.modal_group.mask = 0;
 
                 switch(int_value) {
@@ -1474,8 +1451,8 @@ status_code_t gc_execute_block (char *block)
                         break;
 
                     default:
-                        if(hal.user_mcode.check && (gc_block.user_mcode = hal.user_mcode.check((user_mcode_t)int_value))) {
-                            is_user_mcode = true;
+                        if(grbl.user_mcode.check && (user_mcode = grbl.user_mcode.check((user_mcode_t)int_value))) {
+                            gc_block.user_mcode = (user_mcode_t)int_value;
                             word_bit.modal_group.M10 = On;
                         } else
                             FAIL(Status_GcodeUnsupportedCommand); // [Unsupported M command]
@@ -1824,7 +1801,7 @@ status_code_t gc_execute_block (char *block)
     if(command_words.M10 && gc_block.user_mcode) {
 
         user_words.mask = gc_block.words.mask;
-        if((int_value = (uint_fast16_t)hal.user_mcode.validate(&gc_block, &gc_block.words)))
+        if((int_value = (uint_fast16_t)grbl.user_mcode.validate(&gc_block)))
             FAIL((status_code_t)int_value);
         user_words.mask ^= gc_block.words.mask; // Flag "taken" words for execution
 
@@ -1909,13 +1886,7 @@ status_code_t gc_execute_block (char *block)
 
     // [4. Set spindle speed and address spindle ]: S or D is negative (done.)
     if(gc_block.words.$) {
-        bool single_spindle_only = gc_block.words.s ||
-                                    (command_words.G0 && (gc_block.modal.motion == MotionMode_SpindleSynchronized ||
-                                                           gc_block.modal.motion == MotionMode_RigidTapping ||
-                                                            gc_block.modal.motion == MotionMode_Threading)) ||
-                                     (command_words.G5 && gc_block.modal.feed_mode == FeedMode_UnitsPerRev) ||
-                                       command_words.G14 ||
-                                        (command_words.M9 && gc_block.override_command == Override_SpindleSpeed);
+        bool single_spindle_only = is_single_spindle_block(&gc_block, command_words);
         if(command_words.M7 || single_spindle_only) {
             if(gc_block.values.$ < (single_spindle_only ? 0 : -1))
                 FAIL(single_spindle_only ? Status_NegativeValue : Status_GcodeValueOutOfRange);
@@ -1936,6 +1907,11 @@ status_code_t gc_execute_block (char *block)
             gc_block.words.$ = Off;
         }
     }
+#if N_SYS_SPINDLE > 1
+    // For now, remove when downstream code can handle multiple spindles?
+    else if(command_words.M7 || is_single_spindle_block(&gc_block, command_words))
+        sspindle = &gc_state.modal.spindle[0];
+#endif
 
     if(gc_block.modal.feed_mode == FeedMode_UnitsPerRev && (sspindle == NULL || !sspindle->hal->get_data))
         FAIL(Status_GcodeUnsupportedCommand); // [G95 not supported]
@@ -3538,7 +3514,7 @@ status_code_t gc_execute_block (char *block)
         gc_block.values.o = single_meaning_value.o;
         gc_block.values.s = single_meaning_value.s;
         gc_block.values.t = single_meaning_value.t;
-        hal.user_mcode.execute(state_get(), &gc_block);
+        grbl.user_mcode.execute(state_get(), &gc_block);
         gc_block.words.mask = 0;
     }
 
@@ -3960,7 +3936,7 @@ status_code_t gc_execute_block (char *block)
             idx = N_SYS_SPINDLE;
             spindle_t *spindle;
             do {
-                if((spindle = &gc_state.modal.spindle[--idx])) {
+                if((spindle = &gc_state.modal.spindle[--idx])->hal) {
                     spindle->css = NULL;
                     spindle->state = (spindle_state_t){0};
                     spindle->rpm_mode = SpindleSpeedMode_RPM; // NOTE: not compliant with linuxcnc (?);

gcode.h

@@ -231,7 +231,6 @@ typedef enum {
 __NOTE:__ Not used by the core, may be used by private user code, drivers or plugins.
 */
 typedef enum {
-    UserMCode_Ignore = 0,               //!< 0  - Default, must be zero
     OpenPNP_SetPinState = 42,           //!< 42 - M42
     UserMCode_Generic1 = 101,           //!< 101 - For private use only
     UserMCode_Generic2 = 102,           //!< 102 - For private use only
@@ -637,7 +636,7 @@ It will also be passed to mc_jog_execute() and any user M-code validation and ex
 typedef struct {
     non_modal_t non_modal_command;      //!< Non modal command
     override_mode_t override_command;   //!< Override command TODO: add to non_modal above?
-    user_mcode_t user_mcode;            //!< Set > #UserMCode_Ignore if a user M-code is found.
+    user_mcode_t user_mcode;            //!< Set > 0 if a user M-code is found.
     bool user_mcode_sync;               //!< Set to \a true by M-code validation handler if M-code is to be executed after synchronization.
     gc_modal_t modal;                   //!< The current modal state is copied here before parsing starts.
     spindle_modal_t spindle_modal;