Enable parameter substitution in MSG comments

[skasti]

In many cases it is necessary to communicate something to an operator which contains a dynamic value, and being able to use the MSG comments for this seems natural.

The reason for not using DEBUG comments is that those are used for debugging, and create a lot of noise for the operator. So the operator should be able to disable DEBUG output and still get the messages they need

Example output with debug output enabled:

G65P200T2 
[MSG:Find pickup]
[MSG:Slot 0: 2 T:2]
[MSG:Returning 0]
[MSG:Tool T2 already stored in slot 1 from the back]

Example output with debug output disabled:

G65P200T2
[MSG:Put tool T2 in slot 1 from the back]

P200 is a macro that finds an available slot in my tool rack, tells the operator to put it there, and assigns the tool to the slot so that it knows where to find it.

[terjeio]

please move substitute_parameters() to the same #if #endif block where read_parameter () is located and mark it as static

[skasti]

please move substitute_parameters() to the same #if #endif block where read_parameter () is located and mark it as static

Hm, read_parameter seems to be in the #if #endif block for NGC_EXPRESSIONS_ENABLE. Does this mean I should put this feature behind that flag, and not NGC_PARAMETERS_ENABLE ?

I would have thought that NGC_PARAMETERS_ENABLE would be more appropriate for both of these functions, since they revolve around parameters and not expressions? Though I do see that a sub-section of read_parameters does call ngc_eval_expression 🤔

Will put my stuff in the NGC_EXPRESSIONS_ENABLE-block for now, since that is at least consistent

[terjeio]

FYI NGC_PARAMETERS_ENABLE was added to have basic support for G65 (with no parameter passing) for the 128K STM32F1xx MCUs, this since adding expressions would otherwise overflow flash. Perhaps a bad choice of name for the symbol...
Anyway if NGC_EXPRESSIONS_ENABLE is true NGC_PARAMETERS_ENABLE will always be as well.

[skasti]

FYI NGC_PARAMETERS_ENABLE was added to have basic support for G65 (with no parameter passing) for the 128K STM32F1xx MCUs, this since adding expressions would otherwise overflow flash. Perhaps a bad choice of name for the symbol... Anyway if NGC_EXPRESSIONS_ENABLE is true NGC_PARAMETERS_ENABLE will always be as well.

Aaah, that makes sense😅 I thought maybe parameters might have been a thing outside of using expressions/G65, so that you could have used them in "normal" g-code somehow.

[terjeio]

FYI you introduced a memory leak in MSG substitution...
I will fix this in the next commit and add also support for PRINT messages and formatting of values.

[skasti]

Oh no, I am so sorry! 😞
Please point out how, if possible, so I can hopefully learn 😅

[terjeio]

if(message && *message == NULL && !strncmp(comment, "(MSG,", 5) && (*message = malloc(len))) {

This line allocates memory for a copy of the MSG message, and your code in substitute_parameters() overwrites the *message pointer without freeing the already allocated memory. I changed the code to not allocate memory initally when expressions is enabled.

[skasti]

ah, I did not notice that this line did any memory allocation. I thought it only did the same thing as the debug-one 😅
I guess then the #else-block needs to include that malloc in order to work properly?