Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-introduce prime_tower_enable #18403

Merged
merged 11 commits into from
Feb 27, 2024
Merged

Re-introduce prime_tower_enable #18403

merged 11 commits into from
Feb 27, 2024

Conversation

casperlamboo
Copy link
Contributor

@casperlamboo casperlamboo commented Feb 22, 2024

Description

Previously we introduced new settings related to the prime tower. Specifically the setting prime_tower_enable was changed to prime_tower_mode where prime_tower_enable=False is reinterpreted to prime_tower_mode=None and prime_tower_enable=True is can be either prime_tower_mode=Normal or prime_tower_mode=Interleaved.

This reinterpretation poses some issues related to upgrade script, value resolving etc. Additionally it isn't consistent with how settings are defined elsewhere. For example, for the support setting we have two settings, namely: (bool)support_enabled and (enum)support_mode where support_mode can be either tree or normal. Following this same logic we should have introduced (bool)prime_tower_enable and (enum)prime_tower_mode.

also see enginge PR Ultimaker/CuraEngine#2033

CURA-11645

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Printer definition file(s)
  • Translations

How Has This Been Tested?

Yas

Test Configuration:

  • Operating System: macos13

Checklist:

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

@@ -55,6 +55,7 @@
"settable_per_extruder": false
},
"prime_blob_enable": { "default_value": false },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ diagnostic-definition-redundant-override ⚠️
Overriding prime_blob_enable with the same value (default_value: False) as defined in parent definition: fdmprinter

Suggested change
"prime_blob_enable": { "default_value": false },

@@ -44,8 +44,8 @@
"machine_width": { "default_value": 235 },
"meshfix_maximum_resolution": { "value": 0.25 },
"optimize_wall_printing_order": { "value": true },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ diagnostic-definition-redundant-override ⚠️
Overriding optimize_wall_printing_order with the same value (value: True) as defined in parent definition: fdmprinter

Suggested change
"optimize_wall_printing_order": { "value": true },

"prime_tower_min_volume": { "value": 30 },
"prime_tower_mode": { "value": "'normal'" },
"retract_at_layer_change": { "value": false },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ diagnostic-definition-redundant-override ⚠️
Overriding retract_at_layer_change with the same value (value: False) as defined in parent definition: fdmprinter

Suggested change
"retract_at_layer_change": { "value": false },

@casperlamboo casperlamboo marked this pull request as ready for review February 23, 2024 05:44
@casperlamboo casperlamboo changed the title Introduce prime_tower_enable Re-introduce prime_tower_enable Feb 23, 2024
Copy link
Contributor

@wawanbreton wawanbreton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor remark, I can make the change if you agree with it.
By the way, this change makes more and more sense to me. Some printers forcibly activate or deactivate the prime tower, so when forcing a mode instead, it wouldn't allow them to use the optimized "interleaved" mode. Now they can forcibly enable it, and the mode will depend on the actual materials, which is really expected.

@wawanbreton
Copy link
Contributor

wawanbreton commented Feb 26, 2024

I see a lot of settings in fdmprinter.def.json that still rely on prime_tower_mode to see if the prime tower is enabled. They should go back to using prime_tower_enable
It is also probably missing in SliceInfo.py and in settings visibility profiles.

@wawanbreton wawanbreton merged commit 284ff61 into main Feb 27, 2024
6 of 7 checks passed
@wawanbreton wawanbreton deleted the CURA-11645 branch February 27, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants