-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Cura-specific settings in xmlmaterials #2333
Cura-specific settings in xmlmaterials #2333
Conversation
Please make it so that it can be added under the machine specific details also, and not only as a general setting |
@maukcc, that should already work. Doesn't it? |
I do not know, I have not tested it. |
Out of curiosity, is CURA-4253 in this sprint or in a future sprint? In other words: bump |
Yeah it's in there, but it's a bit of a mess as to if it's something that we should add. The whole material profiles are a bit of a mess. |
And this could be a first step towards making it more consistent with the rest of the profiles system. Unless you want to drop the xml profiles all the way. |
+1 for dropping them. |
Well, we're not going to drop them all together as far as I know. I will try to get some concensus regarding this. The current implementation of Cura is bloated. A lot of the slowness in Cura is caused by the current material system. |
I don't see us dropping the XML format any time soon. There's lots of plans for the near future for the material files. It's not going to become simpler either... I see this sort of change as the only real solution that would make everybody happy, but some people see this as pollution of the files so apparently it doesn't. |
Does it make sense if I keep this PR updated? If it stands no chance of being merged anyway, I would rather just close the PR. |
There is still a discussion about whether this should be supported. XML spec clearly states that it's extendable thus this could be added, but not everyone agrees... |
Ok, I'll fix the conflict then... |
# Conflicts: # plugins/XmlMaterialProfile/XmlMaterialProfile.py
For reference: #1141 (comment) |
I just did a quick unit test and the firmware breaks when adding Cura specific settings to an XML file. This means that we can't add this functionality until the firmware supports it. |
So the firmware has an XML parser that does not understand namespaces? That's strange... |
Woah. That's weird... |
Surprising, the code was written to ignore anything unknown. What exception is it throwing? (Element-tree xml is a bit annoying to use with namespaces) |
# Conflicts: # plugins/XmlMaterialProfile/XmlMaterialProfile.py
Fixed conflicts with master |
# Conflicts: # plugins/XmlMaterialProfile/XmlMaterialProfile.py
Fixed conflicts with master |
To be clear, we need changes in the firmware for this PR and that is not likely to happen in the upcoming 4-6 months. |
@Appesteijn, understood. I've set a reminder for a birthday party on august 29. There will be cake. |
@Appesteijn Robin and I tested this a month ago in preparation for the model assistant issue. The firmware accepts this since version 4.0 apparently. So it's long been in there. |
Ok. I've been testing this. Number values seem to work like gravy but boolean ( Have you gotten this to work, @fieldOfView? |
I had not tested anything but numeric values, but adding a bool setting works for me. I added the following line to generic_pla, and it successfully turned off retraction for the materials.
|
Godmode plugin is a good way to inspect if the internal material profiles get created in the way you expect: |
<cura:setting key="retraction_enable">False</cura:setting> Not here unfortunately. Also, regardless, this should implement the same same boolean syntax as existing attributes which is |
I have fixed reading and writing yes/no as boolean values. True/False still work for reading, but they are always written as yes/no. If this still does not work, please check with GodMode plugin if the value gets overwritten by another profile higher up the stack. |
Yeah I was using GodMode. I've tested again it seems to work. I'm not sure why I was having troubles yesterday. |
cura/CuraApplication.py
Outdated
|
||
# Use the previously found center of the group bounding box as the new location of the group | ||
group_node.setPosition(group_node.getBoundingBox().center) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes don't seem to be related to material loading, but maybe I'm missing something. Can you explain why they're in the PR?
I think there's several files in this branch which need to have master pulled in. |
Something went wrong merging in master. Just a minute... |
Fixed the merge/master issue. |
Great! In this case, merging! |
@Appestein, scrap that cake in august. The birthdayparty is off... I’m having a beer instead now. |
Nooooo, @ianpaschal what did you do!!!! 😄 |
Loving this! Is this gonna make it into Cura 3.3 ?! |
It’s in. Note that there is no UI for it whatsoever, but you can edit values into the xml files now |
Nice! This is a major improvement that I've not even hoped for anymore! Now I can finally create functional material profiles (and not a strange conglomeration of materials and quality with hopes it falls in place.) PS: don't care about GUI crazy much even though it will be nice one of these days. More than happy to fire up the notepad. |
This PR adds functionality to include any Cura setting in an xml fdm material file. These Cura-specific settings are namespaced to not conflict with the fdm material xml spec.
A followup to this PR could enable the user to add settings to the material profile on the materials pane of the preferences. This UI is not part of this PR.
A material with a namespaced cura setting would look like this (note the namespace declaration at the top, and the
<cura.setting>
tag: