From 11d5707141b394c2eb5b9ea021ac8331bd183bba Mon Sep 17 00:00:00 2001 From: Samuel Abels Date: Sun, 29 Jun 2025 01:30:06 +0200 Subject: [PATCH] CAM: Fix: custom shape attributes not showing in toolbit editor --- src/Mod/CAM/CAMTests/TestPathToolShapeDoc.py | 17 ++++++- src/Mod/CAM/Path/Tool/shape/doc.py | 21 +++++--- src/Mod/CAM/Path/Tool/shape/models/base.py | 53 ++++++++++++++++++-- src/Mod/CAM/Path/Tool/toolbit/models/base.py | 23 +++++---- src/Mod/CAM/Path/Tool/toolbit/ui/editor.py | 2 +- 5 files changed, 90 insertions(+), 26 deletions(-) diff --git a/src/Mod/CAM/CAMTests/TestPathToolShapeDoc.py b/src/Mod/CAM/CAMTests/TestPathToolShapeDoc.py index 26ac88230f..e73674b388 100644 --- a/src/Mod/CAM/CAMTests/TestPathToolShapeDoc.py +++ b/src/Mod/CAM/CAMTests/TestPathToolShapeDoc.py @@ -34,6 +34,7 @@ class TestPathToolShapeDoc(unittest.TestCase): mock_doc.Objects = [mock_obj] mock_obj.Label = "MockObjectLabel" mock_obj.Name = "MockObjectName" + mock_obj.getTypeIdOfProperty = MagicMock(return_value="App::PropertyString") # Ensure mock_doc also has a Name attribute used in tests/code mock_doc.Name = "Document_Mock" # Used in closeDocument calls @@ -91,7 +92,13 @@ class TestPathToolShapeDoc(unittest.TestCase): setattr(mock_obj, "Length", "50 mm") params = doc.get_object_properties(mock_obj, ["Diameter", "Length"]) # Expecting just the values, not tuples - self.assertEqual(params, {"Diameter": "10 mm", "Length": "50 mm"}) + self.assertEqual( + params, + { + "Diameter": ("10 mm", "App::PropertyString"), + "Length": ("50 mm", "App::PropertyString"), + }, + ) mock_freecad.Console.PrintWarning.assert_not_called() def test_doc_get_object_properties_missing(self): @@ -105,7 +112,13 @@ class TestPathToolShapeDoc(unittest.TestCase): delattr(mock_obj, "Height") params = doc_patched.get_object_properties(mock_obj, ["Diameter", "Height"]) # Expecting just the values, not tuples - self.assertEqual(params, {"Diameter": "10 mm", "Height": None}) # Height is missing + self.assertEqual( + params, + { + "Diameter": ("10 mm", "App::PropertyString"), + "Height": (None, "App::PropertyString"), + }, + ) # Height is missing @patch("FreeCAD.openDocument") @patch("FreeCAD.getDocument") diff --git a/src/Mod/CAM/Path/Tool/shape/doc.py b/src/Mod/CAM/Path/Tool/shape/doc.py index 28387dc3c2..a442a1a5d4 100644 --- a/src/Mod/CAM/Path/Tool/shape/doc.py +++ b/src/Mod/CAM/Path/Tool/shape/doc.py @@ -23,7 +23,7 @@ import FreeCAD import Path import Path.Base.Util as PathUtil -from typing import Dict, List, Any, Optional +from typing import Dict, List, Any, Optional, Tuple import tempfile import os @@ -69,33 +69,38 @@ def get_object_properties( obj: "FreeCAD.DocumentObject", props: List[str] | None = None, group: Optional[str] = None, -) -> Dict[str, Any]: +) -> Dict[str, Tuple[Any, str]]: """ - Extract properties matching expected_params from a FreeCAD PropertyBag. + Extract properties from a FreeCAD PropertyBag, including their types. Issues warnings for missing parameters but does not raise an error. Args: obj: The PropertyBag to extract properties from. - expected_params (List[str]): A list of property names to look for. + props (List[str], optional): A list of property names to look for. + If None, all properties in obj.PropertiesList are considered. + group (str, optional): If provided, only properties belonging to this group are extracted. Returns: - Dict[str, Any]: A dictionary mapping property names to their values. - Values are FreeCAD native types. + Dict[str, Tuple[Any, str]]: A dictionary mapping property names to a tuple + (value, type_id). Values are FreeCAD native types. + If a property is missing, its value will be None. """ properties = {} for name in props or obj.PropertiesList: if group and not obj.getGroupOfProperty(name) == group: continue if hasattr(obj, name): - properties[name] = getattr(obj, name) + value = getattr(obj, name) + type_id = obj.getTypeIdOfProperty(name) + properties[name] = value, type_id else: # Log a warning if a parameter expected by the shape class is missing Path.Log.debug( f"Parameter '{name}' not found on object '{obj.Label}' " f"({obj.Name}). Default value will be used by the shape class." ) - properties[name] = None # Indicate missing value + properties[name] = None, "App::PropertyString" return properties diff --git a/src/Mod/CAM/Path/Tool/shape/models/base.py b/src/Mod/CAM/Path/Tool/shape/models/base.py index de7c42c210..2e06f5887f 100644 --- a/src/Mod/CAM/Path/Tool/shape/models/base.py +++ b/src/Mod/CAM/Path/Tool/shape/models/base.py @@ -41,6 +41,13 @@ from ..doc import ( from .icon import ToolBitShapeIcon +if False: + Path.Log.setLevel(Path.Log.Level.DEBUG, Path.Log.thisModule()) + Path.Log.trackModule(Path.Log.thisModule()) +else: + Path.Log.setLevel(Path.Log.Level.INFO, Path.Log.thisModule()) + + class ToolBitShape(Asset): """Abstract base class for tool bit shapes.""" @@ -75,6 +82,9 @@ class ToolBitShape(Asset): # Stores default parameter values loaded from the FCStd file self._defaults: Dict[str, Any] = {} + # Stores the FreeCAD property types for each parameter + self._param_types: Dict[str, str] = {} + # Keeps the loaded FreeCAD document content for this instance self._data: Optional[bytes] = None @@ -321,6 +331,7 @@ class ToolBitShape(Asset): Exception: For other potential FreeCAD errors during loading. """ assert serializer == DummyAssetSerializer, "ToolBitShape supports only native import" + Path.Log.debug(f"{id}: ToolBitShape.from_bytes called with {len(data)} bytes") # Open the shape data temporarily to get the Body label and parameters with ShapeDocFromBytes(data) as temp_doc: @@ -335,12 +346,25 @@ class ToolBitShape(Asset): except Exception as e: Path.Log.debug(f"{id}: Failed to determine shape class from bytes: {e}") shape_class = ToolBitShape.get_shape_class_from_id("Custom") + if shape_class is None: + # This should ideally not happen due to get_shape_class_from_bytes fallback + # but added for linter satisfaction. + raise ValueError("Shape class could not be determined.") # Load properties from the temporary document props_obj = ToolBitShape._find_property_object(temp_doc) if not props_obj: raise ValueError("No 'Attributes' PropertyBag object found in document bytes") - loaded_params = get_object_properties(props_obj, group="Shape") + + # loaded_raw_params will now be Dict[str, Tuple[Any, str]] + loaded_raw_params = get_object_properties(props_obj, group="Shape") + + # Separate values and types, and populate _param_types + loaded_params = {} + loaded_param_types = {} + for name, (value, type_id) in loaded_raw_params.items(): + loaded_params[name] = value + loaded_param_types[name] = type_id # For now, we log missing parameters, but do not raise an error. # This allows for more flexible shape files that may not have all @@ -362,11 +386,14 @@ class ToolBitShape(Asset): for param in missing_params: param_type = shape_class.get_parameter_property_type(param) loaded_params[param] = get_unset_value_for(param_type) + loaded_param_types[param] = param_type # Store the type for missing params # Instantiate the specific subclass with the provided ID instance = shape_class(id=id) instance._data = data # Keep the byte content instance._defaults = loaded_params + instance._param_types = loaded_param_types + Path.Log.debug(f"Params: {instance._params} {instance._defaults}") instance._params = instance._defaults | instance._params if dependencies: # dependencies is None = shallow load @@ -442,6 +469,7 @@ class ToolBitShape(Asset): """ if not filepath.exists(): raise FileNotFoundError(f"Shape file not found: {filepath}") + Path.Log.debug(f"{id}: ToolBitShape.from_file called with {filepath}") try: data = filepath.read_bytes() @@ -664,6 +692,13 @@ class ToolBitShape(Asset): Retrieves the thumbnail data for the tool bit shape in PNG format. """ + def get_parameter_type(self, name: str) -> str: + """ + Get the FreeCAD property type string for a given parameter name, + as loaded from the FCStd file. + """ + return self._param_types.get(name, "App::PropertyString") + def get_icon(self) -> Optional[ToolBitShapeIcon]: """ Get the associated ToolBitShapeIcon instance. Tries to load one from @@ -676,18 +711,26 @@ class ToolBitShape(Asset): return self.icon # Try to get a matching SVG from the asset manager. - self.icon = cam_assets.get_or_none(f"toolbitshapesvg://{self.id}.svg") + self.icon = cast( + ToolBitShapeIcon, cam_assets.get_or_none(f"toolbitshapesvg://{self.id}.svg") + ) if self.icon: return self.icon - self.icon = cam_assets.get_or_none(f"toolbitshapesvg://{self.name.lower()}.svg") + self.icon = cast( + ToolBitShapeIcon, cam_assets.get_or_none(f"toolbitshapesvg://{self.name.lower()}.svg") + ) if self.icon: return self.icon # Try to get a matching PNG from the asset manager. - self.icon = cam_assets.get_or_none(f"toolbitshapepng://{self.id}.png") + self.icon = cast( + ToolBitShapeIcon, cam_assets.get_or_none(f"toolbitshapepng://{self.id}.png") + ) if self.icon: return self.icon - self.icon = cam_assets.get_or_none(f"toolbitshapepng://{self.name.lower()}.png") + self.icon = cast( + ToolBitShapeIcon, cam_assets.get_or_none(f"toolbitshapepng://{self.name.lower()}.png") + ) if self.icon: return self.icon return None diff --git a/src/Mod/CAM/Path/Tool/toolbit/models/base.py b/src/Mod/CAM/Path/Tool/toolbit/models/base.py index 6a8d85bd98..e6670537ba 100644 --- a/src/Mod/CAM/Path/Tool/toolbit/models/base.py +++ b/src/Mod/CAM/Path/Tool/toolbit/models/base.py @@ -674,16 +674,19 @@ class ToolBit(Asset, ABC): if value is not None and getattr(self.obj, name) != value: setattr(self.obj, name, value) - # 2. Remove obsolete shape properties - # These are properties currently listed AND in the Shape group, - # but not required by the new shape. - current_shape_prop_names = set(self._get_props("Shape")) - new_shape_param_names = self._tool_bit_shape.schema().keys() - obsolete = current_shape_prop_names - new_shape_param_names - Path.Log.debug(f"Removing obsolete shape properties: {obsolete} from {self.obj.Label}") - # Gracefully skipping the deletion for now; - # in future releases we may handle schema violations more strictly - # self._remove_properties("Shape", obsolete) + # 2. Add additional properties that are part of the shape, + # but not part of the schema. + schema_prop_names = set(self._tool_bit_shape.schema().keys()) + for name, value in self._tool_bit_shape.get_parameters().items(): + if name in schema_prop_names: + continue + prop_type = self._tool_bit_shape.get_parameter_type(name) + docstring = QT_TRANSLATE_NOOP("App::Property", f"Custom property from shape: {name}") + if not hasattr(self.obj, name): + self.obj.addProperty(prop_type, name, PropertyGroupShape, docstring) + Path.Log.debug(f"Added custom shape property: {name} ({prop_type})") + PathUtil.setProperty(self.obj, name, value) + self.obj.setEditorMode(name, 0) def _update_visual_representation(self): """ diff --git a/src/Mod/CAM/Path/Tool/toolbit/ui/editor.py b/src/Mod/CAM/Path/Tool/toolbit/ui/editor.py index 09c310e5c1..27bfd9440c 100644 --- a/src/Mod/CAM/Path/Tool/toolbit/ui/editor.py +++ b/src/Mod/CAM/Path/Tool/toolbit/ui/editor.py @@ -22,9 +22,9 @@ """Widget for editing a ToolBit object.""" +from PySide import QtGui, QtCore import FreeCAD import FreeCADGui -from PySide import QtGui, QtCore from ...shape.ui.shapewidget import ShapeWidget from ...docobject.ui import DocumentObjectEditorWidget from ..models.base import ToolBit