Update start_print.cfg with FORCE_MESH by 0rinsb3lt · Pull Request #407 · Frix-x/klippain

0rinsb3lt · 2023-12-19T01:14:02Z

Added FORCE_MESH as a parameter to ADAPTIVE_BED_MESH

Surion79 · 2023-12-19T06:20:17Z

Can you elaborate the use case? Actually it is missing something. 1. I don't see the variable in variables.cfg.
2. is it an option for start_print as parameter within the slicer? That would be missing too

Benoitone · 2023-12-19T06:43:16Z

Can you elaborate the use case? Actually it is missing something. 1. I don't see the variable in variables.cfg. 2. is it an option for start_print as parameter within the slicer? That would be missing too

I think he must add {% set force_mesh = params.FORCE_MESH|default(False) %} in the param from the slicer?

Surion79 · 2023-12-19T06:47:23Z

He remarks an unknown user variable. So not sure of the integration. It would mean defaulting to the variable and not to a hard coded default.

Benoitone · 2023-12-19T07:41:12Z

?

{% set force_mesh = params.FORCE_MESH|default(False) %} in the param from the slicer?
Is just set to a default variable?

Surion79 · 2023-12-19T08:40:26Z

?

{% set force_mesh = params.FORCE_MESH|default(False) %} in the param from the slicer? Is just set to a default variable?

no, that is setting to a default (hard coded) VALUE. not a default variable. Default variable is declared in his PR, but I was either blind or the variable is not available in variables.cfg

0rinsb3lt · 2023-12-19T14:45:33Z

You are correct I didn't put it into variables.cfg. This PR is to give the user the option to force a bed mesh.
Most of the code was already there it just defaulted to False and never actually read from the variables.cfg file (User variables) or gave the end user a way to enable it.

I will update with a default variables_force_mesh: False in variables.cfg

Added variable_force_mesh with default False value Gives the end user an option to dis/enable.

Surion79 · 2023-12-19T15:03:30Z

i would also like to have it as parameter in start_print available, if we are already integrating it

0rinsb3lt · 2023-12-19T15:04:52Z

i would also like to have it as parameter in start_print available, if we are already integrating it

So that you could enable from the slicer?

Surion79 · 2023-12-19T15:05:58Z

yes. you can look the start_print macro with PARAMS. and adapt it accordingly for bed mesh. I expect some integration since the bed mesh module which is a starting module was modified.

Benoitone · 2023-12-19T17:36:43Z

I don't really see the real point of adding another user variable with all the problems that can cause (when upgrading Klippain) for a function that has limited interest. Why make a bed mesh when the printed part is very small?
So in my opinion it could remain something marginal that the user would be able to pass into their slicer start g_code and which would be used here .

And in this case I don't see how replacing {% set force_mesh = printer["gcode_macro _USER_VARIABLES"].force_mesh %} by {% set force_mesh = params.FORCE_MESH|default(False) %} could cause a problem.

Finally, in the event that you absolutely want to add the new user variable entry in variables.cfg the description can easily lead to confusion: the way it is worded can make you think that there will be no bed mesh if you leave it on False...

Surion79 · 2023-12-19T20:14:26Z

the whole thing makes no sense since the addition is in the bed_mesh_module within the start print sequence. you either need to pass it through start_print params or through variables. if neither is done, the addition is plain never in use. Please look at the change. it is NOT done in the START_PRINT macro.

Surion79 · 2023-12-19T20:15:17Z

and i agree, the var wording creates more issues than it solves

Frix-x

Yes I think this doesn't need a variable but rather a START_PRINT param as it is likely to be used based on the parts to be printed directly from the slicer profile and setting a variable for that is not a good usecase.

Also please note that since this parameter is originally designed to be used from the slicer (even if in Klippain it was a little bit forgotten), so it's not using a Jinja True/False value but a 0/1 instead.

I added some suggestions to make it ready for a merge :)

Frix-x · 2023-12-19T21:45:32Z

    {% set FL_SIZE = printer["gcode_macro START_PRINT"].fl_size %}
    {% set BED_MESH_PROFILE = printer["gcode_macro START_PRINT"].bed_mesh_profile %}

+    {% set force_mesh = printer["gcode_macro _USER_VARIABLES"].force_mesh %}


Can you remove the variable and use this line instead?

Suggested change

{% set force_mesh = printer["gcode_macro _USER_VARIABLES"].force_mesh %}

{% set force_mesh = params.FORCE_MESH|default(0) %}

Can you remove the variable and use this line instead?

Suggested change

{% set force_mesh = printer["gcode_macro _USER_VARIABLES"].force_mesh %}

{% set force_mesh = params.FORCE_MESH|default(0) %}

I think this params: {% set force_mesh = params.FORCE_MESH|default(0) %} must be parse in this section of print_start:
# Get all the parameters passed from the slicer

and call after in bed mesh module?
{% set FORCE_MESH = printer["gcode_macro START_PRINT"].force_mesh %}

why not using the implementation style in start print like it is done previously all the time? the bed mesh module is never called directly outside start print

Frix-x · 2023-12-19T21:46:33Z

+## Always force an adaptive bed mesh during the START_PRINT macro
+variable_force_mesh: False
+


Remove this as it's not needed for this and would add more confusion since it would need a restart of Klipper each time you want to change it and it's not really flexible to configure it per print

Force Mesh in Start_Print with slicer params in Module capsulation

removed variable

Surion79 · 2023-12-20T05:59:08Z

i took the liberty to correct it directly. Should be fine now, since it is 100% matching Module coding

Benoitone · 2023-12-20T08:56:07Z

i took the liberty to correct it directly. Should be fine now, since it is 100% matching Module coding

It seems perfect for me...
@Surion79 @Frix-x : Just for my knowledge... is there a difference if we use False instead of 0 as the default value in this case?

Surion79 · 2023-12-20T11:56:06Z

i took the liberty to correct it directly. Should be fine now, since it is 100% matching Module coding

It seems perfect for me... @Surion79 @Frix-x : Just for my knowledge... is there a difference if we use False instead of 0 as the default value in this case?

i made it int, 0/1 as frix stated it in an above comment

Benoitone · 2023-12-20T13:16:57Z

i took the liberty to correct it directly. Should be fine now, since it is 100% matching Module coding

It seems perfect for me... @Surion79 @Frix-x : Just for my knowledge... is there a difference if we use False instead of 0 as the default value in this case?

i made it int, 0/1 as frix stated it in an above comment

Because in

klippain/macros/calibration/adaptive_bed_mesh.cfg

Line 67 in a207639

    
           {% set force_mesh = params.FORCE_MESH|default(False) %} # force the mesh even if it's a small part (ie. computed less than 3x3)

it seems to be a boolean...

Benoitone · 2023-12-20T13:27:02Z

And maybe take the opportunity to include this function in configuration.md... https://github.com/Frix-x/klippain/blob/e1a6cba3f009e8248b473754599ea1ad2f7bf8c1/docs/configuration.md

Surion79 · 2023-12-20T15:00:26Z

i took the liberty to correct it directly. Should be fine now, since it is 100% matching Module coding

It seems perfect for me... @Surion79 @Frix-x : Just for my knowledge... is there a difference if we use False instead of 0 as the default value in this case?

i made it int, 0/1 as frix stated it in an above comment

Because in

klippain/macros/calibration/adaptive_bed_mesh.cfg

Line 67 in a207639

{% set force_mesh = params.FORCE_MESH|default(False) %} # force the mesh even if it's a small part (ie. computed less than 3x3)

it seems to be a boolean...

could you please comment the line directly in the PR? I don't see it in the current version of the PR

Benoitone · 2023-12-20T15:14:52Z

i took the liberty to correct it directly. Should be fine now, since it is 100% matching Module coding

It seems perfect for me... @Surion79 @Frix-x : Just for my knowledge... is there a difference if we use False instead of 0 as the default value in this case?

i made it int, 0/1 as frix stated it in an above comment

Because in

klippain/macros/calibration/adaptive_bed_mesh.cfg

Line 67 in a207639

{% set force_mesh = params.FORCE_MESH|default(False) %} # force the mesh even if it's a small part (ie. computed less than 3x3)

it seems to be a boolean...

could you please comment the line directly in the PR? I don't see it in the current version of the PR

I don't see how to do this on mobile app...
it's in macros/calibration/adaptive_bed_mesh.cfg -> Ligne67

Surion79 · 2023-12-20T15:49:10Z

oooh, you mean outside this PR. Silly me :D

@Frix-x regarding your statement of force_mesh being 0/1 and in bed mesh it was programmed true/false. Which way to go? Stay int, go boolean or switch from int to bool?

Frix-x

i took the liberty to correct it directly. Should be fine now, since it is 100% matching Module coding

Be careful by doing this, I usually avoid it because it will commit and push directly on the user repo without its consentement and that could lead to troubles especially if it's pushing directly on his main branch like for this PR (ie. worst case is directly breaking his machine by pushing some bad code). Also I like to keep the commits authered by each users as possible as it's good to keep the authorship/copyright for them but it's also easier to find out who have made what on Klippain for us.

@Frix-x regarding your statement of force_mesh being 0/1 and in bed mesh it was programmed true/false. Which way to go? Stay int, go boolean or switch from int to bool?

Yes, I will need to test it. In Jinja, when a string like "False" is used, it can be very confusing because, as a string, it is non-null and thus evaluates as True in a boolean context. So I know that I got a lot of trouble with this in the past and remember that to make it work, I had to use FORCE_MESH=1 on my Prusa. But this need to be tested to be sure.

Finally I added two small change request:

First one is a small error on the same subject that will make it not work so it needs to be changed if it's really an integer that needs to be used
Second one is just to keep it well formated and coding style consistent (uppercase wanted)

Co-authored-by: Félix Boisselier <[email protected]>

Surion79 · 2023-12-20T16:52:28Z

understood and done. usually I don't do that :)

Benoitone · 2023-12-20T19:41:29Z

    {% set MATERIAL = params.MATERIAL|default(printer["gcode_macro _USER_VARIABLES"].print_default_material)|string %} # Material type set in the slicer
    {% set FL_SIZE = params.SIZE|default("0_0_0_0")|string %} # Get bounding box of the first layer for the adaptive bed mesh
    {% set BED_MESH_PROFILE = params.MESH|default("")|string %} # Bed mesh profile to load
+    {% set FORCE_MESH = params.FORCE_MESH|default(0)|int %}


I don't understant why int is better than boolean in this case?
when user pass the start_print parameter: FORCE_MESH=1 is interpreted as True, and with FORCE_MESH=0 is False??

Suggested change

{% set FORCE_MESH = params.FORCE_MESH|default(0)|int %}

{% set force_mesh = params.FORCE_MESH|default(False) %}

Yes this should be the case, but as I said, I need to test it properly since Jinja and boolean is pretty finicky.

Here is why I prefer 0/1 from the slicer:

If you put FORCE_MESH=False -> this should result in False so it's ok

If you put FORCE_MESH="False" -> this will result is True since "False" is a non-null string so it will be very confusing and I'm not sure how the slicer will create the Gcode.

But again I know that it's finicky and that I already battled with this in the past, just not sure if it's exactly this case or if it's still the case.

Frix-x · 2023-12-21T09:57:55Z

Ok, so now the PR is ready to be merged. I just want to test it thoroughly to make sure it's working before I merge it. So please give me some time to do that (currently batch printing Christmas decorations haha)

Update start_print.cfg with FORCE_MESH

9c228d3

Added FORCE_MESH as a parameter to ADAPTIVE_BED_MESH

Update variables.cfg

3504cc8

Added variable_force_mesh with default False value Gives the end user an option to dis/enable.

Frix-x requested changes Dec 19, 2023

View reviewed changes

Surion79 added 2 commits December 20, 2023 06:56

Update start_print.cfg

a674835

Force Mesh in Start_Print with slicer params in Module capsulation

Update variables.cfg

8c17680

removed variable

Frix-x requested changes Dec 20, 2023

View reviewed changes

Comment thread macros/base/start_print.cfg Outdated

Comment thread macros/base/start_print.cfg Outdated

Comment thread macros/base/start_print.cfg Outdated

Surion79 and others added 3 commits December 20, 2023 17:51

Update macros/base/start_print.cfg

060b511

Co-authored-by: Félix Boisselier <[email protected]>

Update macros/base/start_print.cfg

1e23b2f

Co-authored-by: Félix Boisselier <[email protected]>

Update macros/base/start_print.cfg

73953b5

Co-authored-by: Félix Boisselier <[email protected]>

Surion79 requested a review from Frix-x December 20, 2023 16:51

Benoitone reviewed Dec 20, 2023

View reviewed changes

Frix-x self-assigned this Dec 21, 2023

Frix-x added next version This thing is already done or is planned in the next version in preparation tracking This issue is tracked and work will be done labels Dec 21, 2023

	{% set force_mesh = printer["gcode_macro _USER_VARIABLES"].force_mesh %}
	{% set force_mesh = params.FORCE_MESH\|default(0) %}

		## Always force an adaptive bed mesh during the START_PRINT macro
		variable_force_mesh: False

Uh oh!

Conversation

0rinsb3lt commented Dec 19, 2023

Uh oh!

Surion79 commented Dec 19, 2023

Uh oh!

Benoitone commented Dec 19, 2023

Uh oh!

Surion79 commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Benoitone commented Dec 19, 2023

Uh oh!

Surion79 commented Dec 19, 2023

Uh oh!

0rinsb3lt commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Surion79 commented Dec 19, 2023

Uh oh!

0rinsb3lt commented Dec 19, 2023

Uh oh!

Surion79 commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Benoitone commented Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Surion79 commented Dec 19, 2023

Uh oh!

Surion79 commented Dec 19, 2023

Uh oh!

Frix-x left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frix-x Dec 19, 2023

Choose a reason for hiding this comment

Uh oh!

Benoitone Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Surion79 Dec 20, 2023

Choose a reason for hiding this comment

Uh oh!

Frix-x Dec 19, 2023

Choose a reason for hiding this comment

Uh oh!

Surion79 commented Dec 20, 2023

Uh oh!

Benoitone commented Dec 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Surion79 commented Dec 20, 2023

Uh oh!

Benoitone commented Dec 20, 2023

Uh oh!

Benoitone commented Dec 20, 2023

Uh oh!

Surion79 commented Dec 20, 2023

Uh oh!

Benoitone commented Dec 20, 2023

Uh oh!

Surion79 commented Dec 20, 2023

Uh oh!

Frix-x left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Surion79 commented Dec 20, 2023

Uh oh!

Benoitone Dec 20, 2023

Choose a reason for hiding this comment

Uh oh!

Frix-x Dec 21, 2023

Choose a reason for hiding this comment

Uh oh!

Surion79 commented Dec 19, 2023 •

edited

Loading

0rinsb3lt commented Dec 19, 2023 •

edited

Loading

Surion79 commented Dec 19, 2023 •

edited

Loading

Benoitone commented Dec 19, 2023 •

edited

Loading

Frix-x left a comment •

edited

Loading

Benoitone Dec 19, 2023 •

edited

Loading

Benoitone commented Dec 20, 2023 •

edited

Loading