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

[CURA-9224] Approver settings json #13209

Merged
merged 11 commits into from
Sep 13, 2022
Merged

Conversation

Joeydelarago
Copy link
Contributor

Add settings to ufp files so that changes in settings can be reviewed on Digital Factory.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2022

Unit Test Results

15 340 tests  +15 340   15 338 ✔️ +15 338   41s ⏱️ +41s
         1 suites +         1            2 💤 +         2 
         1 files   +         1            0 ±         0 

Results for commit 4e785c3. ± Comparison against base commit 4b2d03f.

♻️ This comment has been updated with latest results.

cura/Settings/CuraStackBuilder.py Outdated Show resolved Hide resolved
plugins/UFPWriter/UFPWriter.py Outdated Show resolved Hide resolved
plugins/UFPWriter/UFPWriter.py Outdated Show resolved Hide resolved
plugins/UFPWriter/UFPWriter.py Outdated Show resolved Hide resolved
plugins/UFPWriter/UFPWriter.py Outdated Show resolved Hide resolved
plugins/UFPWriter/UFPWriter.py Outdated Show resolved Hide resolved
@nallath nallath merged commit ce71cbb into main Sep 13, 2022
@nallath nallath deleted the CURA-9224_approver_settings_json branch September 13, 2022 12:07
@Piezoid
Copy link
Contributor

Piezoid commented Sep 19, 2022

This breaks profiles having SettingFunction, which you get by simply clicking the fx button.

Traceback (most recent call last):
  File "Cura/cura/CuraApplication.py", line 1127, in event
    return super().event(event)
  File "Uranium/UM/Qt/QtApplication.py", line 502, in event
    event._function_event.call()
  File "Uranium/UM/Event.py", line 218, in call
    self._function(*self._args, **self._kwargs)
  File "Cura/cura/Utils/Threading.py", line 34, in _handle_call
    ico.result = func(*args, **kwargs)
  File "Cura/plugins/UFPWriter/UFPWriter.py", line 87, in write
    json.dump(self._getSliceMetadata(), setting_textio, separators=(", ", ": "), indent=4)
  File "$HOME/.conan/data/cpython/3.10.4/_/_/package/9054a82b60899c3ed90865d5b990fd99ef2dc167/lib/python3.10/json/__init__.py", line 179, in dump
    for chunk in iterable:
  File "$HOME/.conan/data/cpython/3.10.4/_/_/package/9054a82b60899c3ed90865d5b990fd99ef2dc167/lib/python3.10/json/encoder.py", line 431, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "$HOME/.conan/data/cpython/3.10.4/_/_/package/9054a82b60899c3ed90865d5b990fd99ef2dc167/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "$HOME/.conan/data/cpython/3.10.4/_/_/package/9054a82b60899c3ed90865d5b990fd99ef2dc167/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "$HOME/.conan/data/cpython/3.10.4/_/_/package/9054a82b60899c3ed90865d5b990fd99ef2dc167/lib/python3.10/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "$HOME/.conan/data/cpython/3.10.4/_/_/package/9054a82b60899c3ed90865d5b990fd99ef2dc167/lib/python3.10/json/encoder.py", line 438, in _iterencode
    o = _default(o)
  File "$HOME/.conan/data/cpython/3.10.4/_/_/package/9054a82b60899c3ed90865d5b990fd99ef2dc167/lib/python3.10/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type SettingFunction is not JSON serializable

@Piezoid
Copy link
Contributor

Piezoid commented Sep 19, 2022

@Joeydelarago GcodeWriter use InstanceContainer.serialize which calls str() on the settings value.
To do the same in UFPWriter:

diff --git a/plugins/UFPWriter/UFPWriter.py b/plugins/UFPWriter/UFPWriter.py
index 49751e1a2a..aa63eb6d2e 100644
--- a/plugins/UFPWriter/UFPWriter.py
+++ b/plugins/UFPWriter/UFPWriter.py
@@ -230,11 +230,11 @@ class UFPWriter(MeshWriter):
         # Add global user or quality changes
         global_flattened_changes = InstanceContainer.createMergedInstanceContainer(global_stack.userChanges, global_stack.qualityChanges)
         for setting in global_flattened_changes.getAllKeys():
-            settings["global"]["changes"][setting] = global_flattened_changes.getProperty(setting, "value")
+            settings["global"]["changes"][setting] = str(global_flattened_changes.getProperty(setting, "value"))
 
         # Get global all settings values without user or quality changes
         for setting in global_stack.getAllKeys():
-            settings["global"]["all_settings"][setting] = global_stack.getProperty(setting, "value")
+            settings["global"]["all_settings"][setting] = str(global_stack.getProperty(setting, "value"))
 
         for i, extruder in enumerate(global_stack.extruderList):
             # Add extruder fields to settings dictionary
@@ -246,10 +246,10 @@ class UFPWriter(MeshWriter):
             # Add extruder user or quality changes
             extruder_flattened_changes = InstanceContainer.createMergedInstanceContainer(extruder.userChanges, extruder.qualityChanges)
             for setting in extruder_flattened_changes.getAllKeys():
-                settings[f"extruder_{i}"]["changes"][setting] = extruder_flattened_changes.getProperty(setting, "value")
+                settings[f"extruder_{i}"]["changes"][setting] = str(extruder_flattened_changes.getProperty(setting, "value"))
 
             # Get extruder all settings values without user or quality changes
             for setting in extruder.getAllKeys():
-                settings[f"extruder_{i}"]["all_settings"][setting] = extruder.getProperty(setting, "value")
+                settings[f"extruder_{i}"]["all_settings"][setting] = str(extruder.getProperty(setting, "value"))
 
         return settings

@Piezoid
Copy link
Contributor

Piezoid commented Sep 27, 2022

@nallath I see that you're about to ship 5.2. The issue with this PR is still present on the pre-release branch. To trigger it, reset any setting to its computed value, then save as UFP.

I could make a PR, but the fix above is probably not what you want, it outputs the formulas rather than the computed values.

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.

4 participants