From 738107f3fb1e6e7b1394e287fa92c354da1bfe03 Mon Sep 17 00:00:00 2001 From: Billy Huddleston Date: Fri, 3 Oct 2025 23:20:01 -0400 Subject: [PATCH 1/2] CAM: Enhance Path.Command annotations with variant type, type-safe API, and robust persistence - Refactored `Annotations` member to use `std::variant` for type-safe storage of both string and numeric values. - Implemented C++ methods: - `setAnnotation(key, value)`: overloaded for string and double types. - `getAnnotation(key)`: returns annotation value as string. - `getAnnotationString(key)`: returns string annotation. - `getAnnotationDouble(key, fallback)`: returns numeric annotation. - `getAnnotationValue(key)`: returns raw variant value. - `hasAnnotation(key)`: checks for annotation existence. - `setAnnotations(annotationString)`: parses and stores values as double if possible, otherwise as string. - Improved XML serialization (`Save`) and deserialization (`Restore`) to persist annotation types and values, including annotation count for robust restoration. - Updated Python bindings: - `Annotations` property now supports mixed-type values (str/float). - Values are returned as native Python types. - Type errors are raised for invalid assignments. - Expanded tests in `TestPathCommandAnnotations.py`: - Added cases for mixed-type annotations, edge cases, and in-memory persistence using `dumpContent`/`restoreContent`. - Verified type preservation and correct restoration. - Ensured backward compatibility for string-only annotations and improved error handling. **How to use annotations in Python:** ```import Path c = Path.Command('G1', {'X': 10.0, 'Y': 20.0, 'F': 1000.0}) c.Annotations = { 'tool_name': '6mm_endmill', # string 'spindle_speed': 12000.0, # float 'feed_rate': 1500, # int (stored as float) 'operation': 'pocket', # string 'depth_of_cut': -2.5, # negative float } print(c.Annotations) # {'tool_name': '6mm_endmill', 'spindle_speed': 12000.0, ...} print(type(c.Annotations['spindle_speed'])) # print(type(c.Annotations['tool_name'])) # xml = c.dumpContent() c2 = Path.Command() c2.restoreContent(xml) print(c2.Annotations) # Restored with correct types c.addAnnotations('speed:1000 operation:drill') print(c.Annotations['speed']) # 1000.0 (float) print(c.Annotations['operation']) # 'drill' (str) ``` --- src/Mod/CAM/App/Command.cpp | 159 ++++++++- src/Mod/CAM/App/Command.h | 18 ++ src/Mod/CAM/App/Command.pyi | 8 +- src/Mod/CAM/App/CommandPyImp.cpp | 92 ++++++ .../CAMTests/TestPathCommandAnnotations.py | 306 ++++++++++++++++++ src/Mod/CAM/CMakeLists.txt | 1 + src/Mod/CAM/TestCAMApp.py | 1 + 7 files changed, 580 insertions(+), 5 deletions(-) create mode 100644 src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py diff --git a/src/Mod/CAM/App/Command.cpp b/src/Mod/CAM/App/Command.cpp index a713bb7689..f7496834e6 100644 --- a/src/Mod/CAM/App/Command.cpp +++ b/src/Mod/CAM/App/Command.cpp @@ -45,9 +45,11 @@ TYPESYSTEM_SOURCE(Path::Command, Base::Persistence) Command::Command(const char* name, const std::map& parameters) : Name(name) , Parameters(parameters) + , Annotations() {} Command::Command() + : Annotations() {} Command::~Command() @@ -332,6 +334,95 @@ void Command::scaleBy(double factor) } } +void Command::setAnnotation(const std::string& key, const std::string& value) +{ + Annotations[key] = value; +} + +void Command::setAnnotation(const std::string& key, double value) +{ + Annotations[key] = value; +} + +std::string Command::getAnnotation(const std::string& key) const +{ + auto it = Annotations.find(key); + if (it != Annotations.end()) { + if (std::holds_alternative(it->second)) { + return std::get(it->second); + } + else if (std::holds_alternative(it->second)) { + return std::to_string(std::get(it->second)); + } + } + return ""; +} + +std::string Command::getAnnotationString(const std::string& key) const +{ + auto it = Annotations.find(key); + if (it != Annotations.end() && std::holds_alternative(it->second)) { + return std::get(it->second); + } + return ""; +} + +double Command::getAnnotationDouble(const std::string& key, double fallback) const +{ + auto it = Annotations.find(key); + if (it != Annotations.end() && std::holds_alternative(it->second)) { + return std::get(it->second); + } + return fallback; +} + +std::variant Command::getAnnotationValue(const std::string& key) const +{ + auto it = Annotations.find(key); + if (it != Annotations.end()) { + return it->second; + } + return std::string(""); +} + +bool Command::hasAnnotation(const std::string& key) const +{ + return Annotations.find(key) != Annotations.end(); +} + +Command& Command::setAnnotations(const std::string& annotationString) +{ + // Simple parser: split by space, then by colon + std::stringstream ss(annotationString); + std::string token; + while (ss >> token) { + auto pos = token.find(':'); + if (pos != std::string::npos) { + std::string key = token.substr(0, pos); + std::string value = token.substr(pos + 1); + + // Try to parse as double, if successful store as double, otherwise as string + try { + size_t processed = 0; + double numValue = std::stod(value, &processed); + if (processed == value.length()) { + // Entire string was successfully parsed as a number + Annotations[key] = numValue; + } + else { + // Partial parse, treat as string + Annotations[key] = value; + } + } + catch (const std::exception&) { + // Not a valid number, store as string + Annotations[key] = value; + } + } + } + return *this; +} + // Reimplemented from base class unsigned int Command::getMemSize() const @@ -341,10 +432,39 @@ unsigned int Command::getMemSize() const void Command::Save(Writer& writer) const { - // this will only get used if saved as XML (probably never) + // Save command with GCode and annotations writer.Stream() << writer.ind() << ""; - writer.Stream() << std::endl; + << "gcode=\"" << toGCode() << "\""; + + // Add annotation count for faster restoration + if (!Annotations.empty()) { + writer.Stream() << " annotationCount=\"" << Annotations.size() << "\""; + } + + if (Annotations.empty()) { + writer.Stream() << " />" << std::endl; + } + else { + writer.Stream() << ">" << std::endl; + writer.incInd(); + + // Save each annotation with type information + for (const auto& annotation : Annotations) { + writer.Stream() << writer.ind() << "(annotation.second)) { + writer.Stream() << " type=\"string\" value=\"" + << std::get(annotation.second) << "\" />" << std::endl; + } + else if (std::holds_alternative(annotation.second)) { + writer.Stream() << " type=\"double\" value=\"" + << std::get(annotation.second) << "\" />" << std::endl; + } + } + + writer.decInd(); + writer.Stream() << writer.ind() << "" << std::endl; + } } void Command::Restore(XMLReader& reader) @@ -352,4 +472,37 @@ void Command::Restore(XMLReader& reader) reader.readElement("Command"); std::string gcode = reader.getAttribute("gcode"); setFromGCode(gcode); + + // Clear any existing annotations + Annotations.clear(); + + // Check if there are annotations to restore + int annotationCount = reader.getAttribute("annotationCount", 0); + + if (annotationCount > 0) { + // Read annotation elements + for (int i = 0; i < annotationCount; i++) { + reader.readElement("Annotation"); + std::string key = reader.getAttribute("key"); + std::string type = reader.getAttribute("type"); + std::string value = reader.getAttribute("value"); + + if (type == "string") { + Annotations[key] = value; + } + else if (type == "double") { + try { + double dvalue = std::stod(value); + Annotations[key] = dvalue; + } + catch (const std::exception&) { + // If conversion fails, store as string + Annotations[key] = value; + } + } + } + + // Read closing tag + reader.readEndElement("Command"); + } } diff --git a/src/Mod/CAM/App/Command.h b/src/Mod/CAM/App/Command.h index 0fc02615d9..48ca010057 100644 --- a/src/Mod/CAM/App/Command.h +++ b/src/Mod/CAM/App/Command.h @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -68,6 +69,22 @@ public: double getValue(const std::string& name) const; // returns the value of a given parameter void scaleBy(double factor); // scales the receiver - use for imperial/metric conversions + // annotation methods + void setAnnotation(const std::string& key, + const std::string& value); // sets a string annotation + void setAnnotation(const std::string& key, + double value); // sets a numeric annotation + std::string getAnnotation(const std::string& key) const; // gets an annotation value as string + std::string getAnnotationString(const std::string& key) const; // gets string annotation + double getAnnotationDouble(const std::string& key, + double fallback = 0.0) const; // gets numeric annotation + std::variant + getAnnotationValue(const std::string& key) const; // gets raw annotation value + bool hasAnnotation(const std::string& key) const; // checks if annotation exists + Command& + setAnnotations(const std::string& annotationString); // sets annotations from string and + // returns reference for chaining + // this assumes the name is upper case inline double getParam(const std::string& name, double fallback = 0.0) const { @@ -78,6 +95,7 @@ public: // attributes std::string Name; std::map Parameters; + std::map> Annotations; }; } // namespace Path diff --git a/src/Mod/CAM/App/Command.pyi b/src/Mod/CAM/App/Command.pyi index 6667069d8a..16f2d3e791 100644 --- a/src/Mod/CAM/App/Command.pyi +++ b/src/Mod/CAM/App/Command.pyi @@ -23,19 +23,23 @@ class Command(Persistence): def toGCode(self) -> str: """toGCode(): returns a GCode representation of the command""" ... - def setFromGCode(self, gcode: str) -> None: """setFromGCode(): sets the path from the contents of the given GCode string""" ... - def transform(self, placement: Placement) -> "CommandPy": """transform(Placement): returns a copy of this command transformed by the given placement""" ... + def addAnnotations(self, annotations) -> "Command": + """addAnnotations(annotations): adds annotations from dictionary or string and returns self for chaining""" + ... Name: str """The name of the command""" Parameters: dict[str, float] """The parameters of the command""" + Annotations: dict[str, str] + """The annotations of the command""" + Placement: Placement """The coordinates of the endpoint of the command""" diff --git a/src/Mod/CAM/App/CommandPyImp.cpp b/src/Mod/CAM/App/CommandPyImp.cpp index d9598ed88a..9b3d48b20f 100644 --- a/src/Mod/CAM/App/CommandPyImp.cpp +++ b/src/Mod/CAM/App/CommandPyImp.cpp @@ -207,6 +207,57 @@ void CommandPy::setParameters(Py::Dict arg) } } +// Annotations attribute get/set + +Py::Dict CommandPy::getAnnotations() const +{ + Py::Dict annotationsDict; + for (const auto& pair : getCommandPtr()->Annotations) { + if (std::holds_alternative(pair.second)) { + annotationsDict.setItem(pair.first, Py::String(std::get(pair.second))); + } + else if (std::holds_alternative(pair.second)) { + annotationsDict.setItem(pair.first, Py::Float(std::get(pair.second))); + } + } + return annotationsDict; +} + +void CommandPy::setAnnotations(Py::Dict arg) +{ + getCommandPtr()->Annotations.clear(); + PyObject *key, *value; + Py_ssize_t pos = 0; + while (PyDict_Next(arg.ptr(), &pos, &key, &value)) { + std::string ckey; + if (PyUnicode_Check(key)) { + ckey = PyUnicode_AsUTF8(key); + + if (PyUnicode_Check(value)) { + // String value + std::string cvalue = PyUnicode_AsUTF8(value); + getCommandPtr()->Annotations[ckey] = cvalue; + } + else if (PyFloat_Check(value)) { + // Float value + double dvalue = PyFloat_AsDouble(value); + getCommandPtr()->Annotations[ckey] = dvalue; + } + else if (PyLong_Check(value)) { + // Integer value (convert to double) + double dvalue = static_cast(PyLong_AsLong(value)); + getCommandPtr()->Annotations[ckey] = dvalue; + } + else { + throw Py::TypeError("Annotation values must be strings or numbers"); + } + } + else { + throw Py::TypeError("Annotation keys must be strings"); + } + } +} + // GCode methods PyObject* CommandPy::toGCode(PyObject* args) const @@ -272,6 +323,47 @@ PyObject* CommandPy::transform(PyObject* args) } } +PyObject* CommandPy::addAnnotations(PyObject* args) +{ + PyObject* annotationsObj; + if (PyArg_ParseTuple(args, "O", &annotationsObj)) { + if (PyDict_Check(annotationsObj)) { + // Handle dictionary input + PyObject *key, *value; + Py_ssize_t pos = 0; + while (PyDict_Next(annotationsObj, &pos, &key, &value)) { + std::string ckey, cvalue; + if (PyUnicode_Check(key) && PyUnicode_Check(value)) { + ckey = PyUnicode_AsUTF8(key); + cvalue = PyUnicode_AsUTF8(value); + getCommandPtr()->setAnnotation(ckey, cvalue); + } + else { + PyErr_SetString(PyExc_TypeError, "Dictionary keys and values must be strings"); + return nullptr; + } + } + } + else if (PyUnicode_Check(annotationsObj)) { + // Handle string input like "xyz:abc test:1234" + std::string annotationString = PyUnicode_AsUTF8(annotationsObj); + getCommandPtr()->setAnnotations(annotationString); + } + else { + PyErr_SetString(PyExc_TypeError, "Argument must be a dictionary or string"); + return nullptr; + } + + // Return self for chaining + Py_INCREF(this); + return static_cast(this); + } + else { + PyErr_SetString(PyExc_TypeError, "Invalid arguments"); + return nullptr; + } +} + // custom attributes get/set PyObject* CommandPy::getCustomAttributes(const char* attr) const diff --git a/src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py b/src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py new file mode 100644 index 0000000000..e834de9b21 --- /dev/null +++ b/src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py @@ -0,0 +1,306 @@ +# -*- coding: utf-8 -*- +# *************************************************************************** +# * Copyright (c) 2025 FreeCAD Contributors * +# * * +# * This program is free software; you can redistribute it and/or modify * +# * it under the terms of the GNU Lesser General Public License (LGPL) * +# * as published by the Free Software Foundation; either version 2 of * +# * the License, or (at your option) any later version. * +# * for detail see the LICENCE text file. * +# * * +# * This program is distributed in the hope that it will be useful, * +# * but WITHOUT ANY WARRANTY; without even the implied warranty of * +# * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * +# * GNU Library General Public License for more details. * +# * * +# * You should have received a copy of the GNU Library General Public * +# * License along with this program; if not, write to the Free Software * +# * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 * +# * USA * +# * * +# *************************************************************************** + +import FreeCAD +import Path +from CAMTests.PathTestUtils import PathTestBase + + +class TestPathCommandAnnotations(PathTestBase): + """Test Path.Command annotations functionality.""" + + def test00(self): + """Test basic annotations property access.""" + # Create empty command + c = Path.Command() + self.assertIsInstance(c, Path.Command) + + # Test empty annotations + self.assertEqual(c.Annotations, {}) + + # Set annotations via property + c.Annotations = {"tool": "tap", "material": "steel"} + self.assertEqual(c.Annotations, {"tool": "tap", "material": "steel"}) + + # Test individual annotation access + self.assertEqual(c.Annotations.get("tool"), "tap") + self.assertEqual(c.Annotations.get("material"), "steel") + self.assertIsNone(c.Annotations.get("nonexistent")) + + def test01(self): + """Test annotations with command creation.""" + # Create command with parameters + c = Path.Command("G84", {"X": 10, "Y": 20, "Z": -5}) + + # Set annotations + c.Annotations = {"operation": "tapping", "thread": "M6"} + + # Verify command still works normally + self.assertEqual(c.Name, "G84") + self.assertEqual(c.Parameters["X"], 10.0) + self.assertEqual(c.Parameters["Y"], 20.0) + self.assertEqual(c.Parameters["Z"], -5.0) + + # Verify annotations are preserved + self.assertEqual(c.Annotations["operation"], "tapping") + self.assertEqual(c.Annotations["thread"], "M6") + + def test02(self): + """Test addAnnotations method with dictionary input.""" + c = Path.Command("G1", {"X": 5, "Y": 5}) + + # Test method chaining with dictionary + result = c.addAnnotations({"note": "test note", "tool": "end mill"}) + + # Verify method returns the command object for chaining + self.assertIs(result, c) + + # Verify annotations were set + self.assertEqual(c.Annotations["note"], "test note") + self.assertEqual(c.Annotations["tool"], "end mill") + + def test03(self): + """Test addAnnotations method with string input.""" + c = Path.Command("G2", {"X": 15, "Y": 15}) + + # Test method chaining with string + result = c.addAnnotations("xyz:abc test:1234 operation:milling") + + # Verify method returns the command object for chaining + self.assertIs(result, c) + + # Verify annotations were parsed and set correctly + self.assertEqual(c.Annotations["xyz"], "abc") + self.assertEqual(c.Annotations["test"], 1234) + self.assertEqual(c.Annotations["operation"], "milling") + + def test04(self): + """Test annotations update behavior.""" + c = Path.Command("G0", {"Z": 20}) + + # Set initial annotations + c.Annotations = {"initial": "value"} + self.assertEqual(c.Annotations, {"initial": "value"}) + + # Add more annotations - should merge/update + c.addAnnotations({"additional": "value2", "initial": "updated"}) + + expected = {"initial": "updated", "additional": "value2"} + self.assertEqual(c.Annotations, expected) + + def test05(self): + """Test method chaining in fluent interface.""" + # Test the fluent interface - create command and set annotations in one line + c = Path.Command("G84", {"X": 10, "Y": 10, "Z": 0.0}).addAnnotations("thread:M8 depth:15mm") + + # Verify command parameters + self.assertEqual(c.Name, "G84") + self.assertEqual(c.Parameters["X"], 10.0) + self.assertEqual(c.Parameters["Y"], 10.0) + self.assertEqual(c.Parameters["Z"], 0.0) + + # Verify annotations + self.assertEqual(c.Annotations["thread"], "M8") + self.assertEqual(c.Annotations["depth"], "15mm") + + def test06(self): + """Test annotations with special characters and edge cases.""" + c = Path.Command("G1") + + # Test annotations with special characters + c.Annotations = { + "unicode": "café", + "numbers": "123.45", + "empty": "", + "spaces": "value with spaces", + } + + self.assertEqual(c.Annotations["unicode"], "café") + self.assertEqual(c.Annotations["numbers"], "123.45") + self.assertEqual(c.Annotations["empty"], "") + self.assertEqual(c.Annotations["spaces"], "value with spaces") + + def test07(self): + """Test annotations persistence through operations.""" + c = Path.Command("G1", {"X": 10, "Y": 20}) + c.Annotations = {"persistent": "value"} + + # Test that annotations survive parameter changes + c.Parameters = {"X": 30, "Y": 40} + self.assertEqual(c.Annotations["persistent"], "value") + + # Test that annotations survive name changes + c.Name = "G2" + self.assertEqual(c.Annotations["persistent"], "value") + + def test08(self): + """Test multiple annotation update methods.""" + c = Path.Command() + + # Method 1: Property assignment + c.Annotations = {"method1": "property"} + + # Method 2: addAnnotations with dict + c.addAnnotations({"method2": "dict"}) + + # Method 3: addAnnotations with string + c.addAnnotations("method3:string") + + # Verify all methods worked and annotations are merged + expected = {"method1": "property", "method2": "dict", "method3": "string"} + self.assertEqual(c.Annotations, expected) + + def test09(self): + """Test string parsing edge cases.""" + c = Path.Command() + + # Test various string formats + c.addAnnotations("simple:value") + self.assertEqual(c.Annotations["simple"], "value") + + # Test multiple key:value pairs + c.Annotations = {} # Clear first + c.addAnnotations("key1:val1 key2:val2 key3:val3") + expected = {"key1": "val1", "key2": "val2", "key3": "val3"} + self.assertEqual(c.Annotations, expected) + + # Test that malformed strings are ignored + c.Annotations = {} # Clear first + c.addAnnotations("valid:value invalid_no_colon") + self.assertEqual(c.Annotations, {"valid": "value"}) + + def test10(self): + """Test annotations in gcode context.""" + # Create a tapping command with annotations + c = Path.Command( + "G84", {"X": 25.0, "Y": 30.0, "Z": -10.0, "R": 2.0, "P": 0.5, "F": 100.0} + ).addAnnotations("operation:tapping thread:M6x1.0 depth:10mm") + + # Verify gcode output is unaffected by annotations + gcode = c.toGCode() + self.assertIn("G84", gcode) + self.assertIn("X25", gcode) + self.assertIn("Y30", gcode) + self.assertIn("Z-10", gcode) + + # Verify annotations are preserved + self.assertEqual(c.Annotations["operation"], "tapping") + self.assertEqual(c.Annotations["thread"], "M6x1.0") + self.assertEqual(c.Annotations["depth"], "10mm") + + # Annotations should not appear in gcode output + self.assertNotIn("operation", gcode) + self.assertNotIn("tapping", gcode) + self.assertNotIn("thread", gcode) + + def test11(self): + """Test save/restore with mixed string and numeric annotations (in-memory).""" + # Create command with mixed annotations + original = Path.Command("G1", {"X": 10.0, "Y": 20.0, "F": 1000.0}) + original.Annotations = { + "tool_name": "6mm_endmill", # string + "spindle_speed": 12000.0, # float + "feed_rate": 1500, # int -> float + "operation": "pocket", # string + "depth_of_cut": -2.5, # negative float + } + + # Use FreeCAD's in-memory serialization + content = original.dumpContent() + + # Create new command and restore from memory + restored = Path.Command() + restored.restoreContent(content) + + # Verify all annotations were restored with correct types + self.assertEqual(restored.Annotations["tool_name"], "6mm_endmill") + self.assertEqual(restored.Annotations["spindle_speed"], 12000.0) + self.assertEqual(restored.Annotations["feed_rate"], 1500.0) + self.assertEqual(restored.Annotations["operation"], "pocket") + self.assertEqual(restored.Annotations["depth_of_cut"], -2.5) + + # Verify types are preserved + self.assertIsInstance(restored.Annotations["tool_name"], str) + self.assertIsInstance(restored.Annotations["spindle_speed"], float) + self.assertIsInstance(restored.Annotations["feed_rate"], float) + self.assertIsInstance(restored.Annotations["operation"], str) + self.assertIsInstance(restored.Annotations["depth_of_cut"], float) + + # Verify GCode parameters were also restored correctly + self.assertEqual(restored.Name, "G1") + # Note: Parameters are restored via GCode parsing + + def test12(self): + """Test save/restore with empty and complex annotations (in-memory).""" + # Test 1: Empty annotations (should work and use compact format) + simple = Path.Command("G0", {"Z": 5.0}) + self.assertEqual(simple.Annotations, {}) + + simple_content = simple.dumpContent() + simple_restored = Path.Command() + simple_restored.restoreContent(simple_content) + + self.assertEqual(simple_restored.Annotations, {}) + self.assertEqual(simple_restored.Name, "G0") + + # Test 2: Complex CAM annotations with edge cases + complex_cmd = Path.Command("G84", {"X": 25.4, "Y": 12.7, "Z": -8.0}) + complex_cmd.Annotations = { + # Mixed types with edge cases + "tool_type": "tap", # string + "spindle_speed": 500.0, # float + "zero_value": 0.0, # zero + "negative": -123.456, # negative + "large_number": 999999.999, # large number + "operation_id": "OP_030", # alphanumeric string + "thread_spec": "M4x0.7", # string with numbers + "scientific": 1.23e-6, # scientific notation + } + + # Serialize and restore + complex_content = complex_cmd.dumpContent() + complex_restored = Path.Command() + complex_restored.restoreContent(complex_content) + + # Verify all complex data restored correctly + self.assertEqual(len(complex_restored.Annotations), 8) + + # Check specific values and types + self.assertEqual(complex_restored.Annotations["tool_type"], "tap") + self.assertIsInstance(complex_restored.Annotations["tool_type"], str) + + self.assertEqual(complex_restored.Annotations["spindle_speed"], 500.0) + self.assertIsInstance(complex_restored.Annotations["spindle_speed"], float) + + self.assertEqual(complex_restored.Annotations["zero_value"], 0.0) + self.assertEqual(complex_restored.Annotations["negative"], -123.456) + self.assertEqual(complex_restored.Annotations["large_number"], 999999.999) + + # Verify strings with numbers stay as strings + self.assertEqual(complex_restored.Annotations["operation_id"], "OP_030") + self.assertEqual(complex_restored.Annotations["thread_spec"], "M4x0.7") + self.assertIsInstance(complex_restored.Annotations["operation_id"], str) + self.assertIsInstance(complex_restored.Annotations["thread_spec"], str) + + # Check scientific notation + self.assertAlmostEqual(complex_restored.Annotations["scientific"], 1.23e-6, places=8) + self.assertIsInstance(complex_restored.Annotations["scientific"], float) diff --git a/src/Mod/CAM/CMakeLists.txt b/src/Mod/CAM/CMakeLists.txt index 6e965b361a..dfb5b50d03 100644 --- a/src/Mod/CAM/CMakeLists.txt +++ b/src/Mod/CAM/CMakeLists.txt @@ -497,6 +497,7 @@ SET(Tests_SRCS CAMTests/TestLinuxCNCPost.py CAMTests/TestMach3Mach4Post.py CAMTests/TestPathAdaptive.py + CAMTests/TestPathCommandAnnotations.py CAMTests/TestPathCore.py CAMTests/TestPathDepthParams.py CAMTests/TestPathDressupArray.py diff --git a/src/Mod/CAM/TestCAMApp.py b/src/Mod/CAM/TestCAMApp.py index 26e734c759..131150c87c 100644 --- a/src/Mod/CAM/TestCAMApp.py +++ b/src/Mod/CAM/TestCAMApp.py @@ -26,6 +26,7 @@ from CAMTests.TestCAMSanity import TestCAMSanity from CAMTests.TestPathProfile import TestPathProfile from CAMTests.TestPathAdaptive import TestPathAdaptive +from CAMTests.TestPathCommandAnnotations import TestPathCommandAnnotations from CAMTests.TestPathCore import TestPathCore from CAMTests.TestPathDepthParams import depthTestCases from CAMTests.TestPathDressupDogbone import TestDressupDogbone From 5bc8fe2f740215c2a8ec99a86df7015a87903204 Mon Sep 17 00:00:00 2001 From: Billy Huddleston Date: Sun, 5 Oct 2025 12:13:30 -0400 Subject: [PATCH 2/2] CAM: Simplify annotation handling in GCode and improve annotation parsing src/Mod/CAM/App/Command.cpp: - Removed requirement for annotations= prefix; now all text after ; is treated as annotation data. - Updated Command::toGCode to output annotations as key-value pairs in comments. - Improved setFromGCode to extract annotations from any comment after ;. - Enhanced annotation parsing to handle quoted strings and floating-point numbers. - Simplified XML serialization and restoration logic for annotations. src/Mod/CAM/App/Path.cpp: - Added addCommandNoRecalc, allowing bulk loading of commands without repeated recalculation. - Refactored RestoreDocFile to read GCode files line-by-line, parse each command, and call recalculate() only once after all commands are loaded. - Added explanatory comment above the old implementation. src/Mod/CAM/App/Path.h: - Declared addCommandNoRecalc in the Toolpath class. src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py: - Adjusted unit test for scientific notation annotation to check only 6 decimal places. - Adjusted unit test 10 to properly handle assertions with the toGCode method src/Mod/CAM/PathSimulator/AppGL/GCodeParser.cpp: - GCodeParser::ParseLine: Truncate at first semicolon (annotations / comment) --- src/Mod/CAM/App/Command.cpp | 185 ++++++++++-------- src/Mod/CAM/App/Command.pyi | 3 + src/Mod/CAM/App/Path.cpp | 33 +++- src/Mod/CAM/App/Path.h | 1 + .../CAMTests/TestPathCommandAnnotations.py | 17 +- .../CAM/PathSimulator/AppGL/GCodeParser.cpp | 8 + 6 files changed, 160 insertions(+), 87 deletions(-) diff --git a/src/Mod/CAM/App/Command.cpp b/src/Mod/CAM/App/Command.cpp index f7496834e6..59e9afa31d 100644 --- a/src/Mod/CAM/App/Command.cpp +++ b/src/Mod/CAM/App/Command.cpp @@ -25,7 +25,6 @@ #include #include - #include #include #include @@ -139,20 +138,54 @@ std::string Command::toGCode(int precision, bool padzero) const } str << '.' << std::setw(width) << std::right << digits; } + + // Add annotations as a comment if they exist + if (!Annotations.empty()) { + str << " ; "; + bool first = true; + for (const auto& pair : Annotations) { + if (!first) { + str << " "; + } + first = false; + str << pair.first << ":"; + if (std::holds_alternative(pair.second)) { + str << "'" << std::get(pair.second) << "'"; + } + else if (std::holds_alternative(pair.second)) { + std::ostringstream oss; + oss << std::fixed << std::setprecision(6) << std::get(pair.second); + str << oss.str(); + } + } + } + return str.str(); } void Command::setFromGCode(const std::string& str) { Parameters.clear(); + Annotations.clear(); + + // Check for annotation comment and split the string + std::string gcode_part = str; + std::string annotation_part; + + auto comment_pos = str.find("; "); + if (comment_pos != std::string::npos) { + gcode_part = str.substr(0, comment_pos); + annotation_part = str.substr(comment_pos + 1); // length of "; " + } + std::string mode = "none"; std::string key; std::string value; - for (unsigned int i = 0; i < str.size(); i++) { - if ((isdigit(str[i])) || (str[i] == '-') || (str[i] == '.')) { - value += str[i]; + for (unsigned int i = 0; i < gcode_part.size(); i++) { + if ((isdigit(gcode_part[i])) || (gcode_part[i] == '-') || (gcode_part[i] == '.')) { + value += gcode_part[i]; } - else if (isalpha(str[i])) { + else if (isalpha(gcode_part[i])) { if (mode == "command") { if (!key.empty() && !value.empty()) { std::string cmd = key + value; @@ -183,24 +216,30 @@ void Command::setFromGCode(const std::string& str) } } else if (mode == "comment") { - value += str[i]; + value += gcode_part[i]; } - key = str[i]; + key = gcode_part[i]; } - else if (str[i] == '(') { + else if (gcode_part[i] == '(') { mode = "comment"; } - else if (str[i] == ')') { + else if (gcode_part[i] == ')') { key = "("; value += ")"; } else { // add non-ascii characters only if this is a comment if (mode == "comment") { - value += str[i]; + value += gcode_part[i]; } } } + + // Parse annotations if found + if (!annotation_part.empty()) { + setAnnotations(annotation_part); + } + if (!key.empty() && !value.empty()) { if ((mode == "command") || (mode == "comment")) { std::string cmd = key + value; @@ -392,117 +431,109 @@ bool Command::hasAnnotation(const std::string& key) const Command& Command::setAnnotations(const std::string& annotationString) { - // Simple parser: split by space, then by colon - std::stringstream ss(annotationString); + std::istringstream iss(annotationString); std::string token; - while (ss >> token) { + while (iss >> token) { auto pos = token.find(':'); if (pos != std::string::npos) { std::string key = token.substr(0, pos); std::string value = token.substr(pos + 1); - // Try to parse as double, if successful store as double, otherwise as string - try { - size_t processed = 0; - double numValue = std::stod(value, &processed); - if (processed == value.length()) { - // Entire string was successfully parsed as a number - Annotations[key] = numValue; + // If value starts and ends with single quote, treat as string + if (value.size() >= 2 && value.front() == '\'' && value.back() == '\'') { + Annotations[key] = value.substr(1, value.size() - 2); + } + else { + // Try to parse as double, if successful store as double, otherwise as string + try { + size_t processed = 0; + double numValue = std::stod(value, &processed); + if (processed == value.length()) { + Annotations[key] = numValue; + } + else { + Annotations[key] = value; + } } - else { - // Partial parse, treat as string + catch (const std::exception&) { Annotations[key] = value; } } - catch (const std::exception&) { - // Not a valid number, store as string - Annotations[key] = value; - } } } return *this; } -// Reimplemented from base class - unsigned int Command::getMemSize() const { return toGCode().size(); } -void Command::Save(Writer& writer) const +void Command::Save(Base::Writer& writer) const { - // Save command with GCode and annotations - writer.Stream() << writer.ind() << "" << std::endl; - } - else { - writer.Stream() << ">" << std::endl; - writer.incInd(); - - // Save each annotation with type information - for (const auto& annotation : Annotations) { - writer.Stream() << writer.ind() << "(annotation.second)) { - writer.Stream() << " type=\"string\" value=\"" - << std::get(annotation.second) << "\" />" << std::endl; + writer.Stream() << " annotations=\""; + bool first = true; + for (const auto& pair : Annotations) { + if (!first) { + writer.Stream() << " "; } - else if (std::holds_alternative(annotation.second)) { - writer.Stream() << " type=\"double\" value=\"" - << std::get(annotation.second) << "\" />" << std::endl; + first = false; + writer.Stream() << pair.first << ":"; + if (std::holds_alternative(pair.second)) { + writer.Stream() << "'" << std::get(pair.second) << "'"; + } + else if (std::holds_alternative(pair.second)) { + writer.Stream() << std::fixed << std::setprecision(6) + << std::get(pair.second); } } - - writer.decInd(); - writer.Stream() << writer.ind() << "" << std::endl; + writer.Stream() << "\""; } + + writer.Stream() << " />" << std::endl; } -void Command::Restore(XMLReader& reader) +void Command::Restore(Base::XMLReader& reader) { reader.readElement("Command"); std::string gcode = reader.getAttribute("gcode"); setFromGCode(gcode); - // Clear any existing annotations Annotations.clear(); - // Check if there are annotations to restore - int annotationCount = reader.getAttribute("annotationCount", 0); + std::string attr; + try { + attr = reader.getAttribute("annotations"); + } + catch (...) { + return; // No annotations + } - if (annotationCount > 0) { - // Read annotation elements - for (int i = 0; i < annotationCount; i++) { - reader.readElement("Annotation"); - std::string key = reader.getAttribute("key"); - std::string type = reader.getAttribute("type"); - std::string value = reader.getAttribute("value"); + std::istringstream iss(attr); + std::string token; + while (iss >> token) { + auto pos = token.find(':'); + if (pos != std::string::npos) { + std::string key = token.substr(0, pos); + std::string value = token.substr(pos + 1); - if (type == "string") { - Annotations[key] = value; + // If value starts and ends with single quote, treat as string + if (value.size() >= 2 && value.front() == '\'' && value.back() == '\'') { + Annotations[key] = value.substr(1, value.size() - 2); } - else if (type == "double") { + else { try { - double dvalue = std::stod(value); - Annotations[key] = dvalue; + double d = std::stod(value); + Annotations[key] = d; } - catch (const std::exception&) { - // If conversion fails, store as string + catch (...) { Annotations[key] = value; } } } - - // Read closing tag - reader.readEndElement("Command"); } } diff --git a/src/Mod/CAM/App/Command.pyi b/src/Mod/CAM/App/Command.pyi index 16f2d3e791..a90117313c 100644 --- a/src/Mod/CAM/App/Command.pyi +++ b/src/Mod/CAM/App/Command.pyi @@ -23,12 +23,15 @@ class Command(Persistence): def toGCode(self) -> str: """toGCode(): returns a GCode representation of the command""" ... + def setFromGCode(self, gcode: str) -> None: """setFromGCode(): sets the path from the contents of the given GCode string""" ... + def transform(self, placement: Placement) -> "CommandPy": """transform(Placement): returns a copy of this command transformed by the given placement""" ... + def addAnnotations(self, annotations) -> "Command": """addAnnotations(annotations): adds annotations from dictionary or string and returns self for chaining""" ... diff --git a/src/Mod/CAM/App/Path.cpp b/src/Mod/CAM/App/Path.cpp index be3c9cf1fb..bbac240cdb 100644 --- a/src/Mod/CAM/App/Path.cpp +++ b/src/Mod/CAM/App/Path.cpp @@ -515,13 +515,36 @@ void Toolpath::Restore(XMLReader& reader) } } +// The previous implementation read the file word-by-word, merging all content into a single string. +// This caused GCode commands and annotations to be split and parsed incorrectly. +// The new implementation reads the file line-by-line, ensuring each command and its annotations are +// handled correctly. void Toolpath::RestoreDocFile(Base::Reader& reader) +// { +// std::string gcode; +// std::string line; +// while (reader >> line) { +// gcode += line; +// gcode += " "; +// } +// setFromGCode(gcode); +// } + +void Toolpath::addCommandNoRecalc(const Command& Cmd) +{ + Command* tmp = new Command(Cmd); + vpcCommands.push_back(tmp); + // No recalculate here +} + void Toolpath::RestoreDocFile(Base::Reader& reader) { - std::string gcode; std::string line; - while (reader >> line) { - gcode += line; - gcode += " "; + while (std::getline(reader.getStream(), line)) { + if (!line.empty()) { + Command cmd; + cmd.setFromGCode(line); + addCommandNoRecalc(cmd); + } } - setFromGCode(gcode); + recalculate(); // Only once, after all commands are loaded } diff --git a/src/Mod/CAM/App/Path.h b/src/Mod/CAM/App/Path.h index 7647630669..1912e0f6b6 100644 --- a/src/Mod/CAM/App/Path.h +++ b/src/Mod/CAM/App/Path.h @@ -57,6 +57,7 @@ public: // interface void clear(); // clears the internal data void addCommand(const Command& Cmd); // adds a command at the end + void addCommandNoRecalc(const Command& Cmd); // adds a command without recalculation void insertCommand(const Command& Cmd, int); // inserts a command void deleteCommand(int); // deletes a command double getLength(); // return the Length (mm) of the Path diff --git a/src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py b/src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py index e834de9b21..8416535e66 100644 --- a/src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py +++ b/src/Mod/CAM/CAMTests/TestPathCommandAnnotations.py @@ -208,9 +208,16 @@ class TestPathCommandAnnotations(PathTestBase): self.assertEqual(c.Annotations["depth"], "10mm") # Annotations should not appear in gcode output - self.assertNotIn("operation", gcode) - self.assertNotIn("tapping", gcode) - self.assertNotIn("thread", gcode) + gcode_parts = gcode.split(";", 1) + main_gcode = gcode_parts[0] + comment = gcode_parts[1] if len(gcode_parts) > 1 else "" + + self.assertIn("operation:'tapping'", comment) + self.assertIn("thread:'M6x1.0'", comment) + self.assertIn("depth:'10mm'", comment) + self.assertNotIn("operation", main_gcode) + self.assertNotIn("thread", main_gcode) + self.assertNotIn("depth", main_gcode) def test11(self): """Test save/restore with mixed string and numeric annotations (in-memory).""" @@ -301,6 +308,6 @@ class TestPathCommandAnnotations(PathTestBase): self.assertIsInstance(complex_restored.Annotations["operation_id"], str) self.assertIsInstance(complex_restored.Annotations["thread_spec"], str) - # Check scientific notation - self.assertAlmostEqual(complex_restored.Annotations["scientific"], 1.23e-6, places=8) + # Check scientific notation (now only 6 decimal places) + self.assertAlmostEqual(complex_restored.Annotations["scientific"], 1.23e-6, places=6) self.assertIsInstance(complex_restored.Annotations["scientific"], float) diff --git a/src/Mod/CAM/PathSimulator/AppGL/GCodeParser.cpp b/src/Mod/CAM/PathSimulator/AppGL/GCodeParser.cpp index b4458efe73..bd7fb03def 100644 --- a/src/Mod/CAM/PathSimulator/AppGL/GCodeParser.cpp +++ b/src/Mod/CAM/PathSimulator/AppGL/GCodeParser.cpp @@ -138,6 +138,14 @@ const char* GCodeParser::ParseFloat(const char* ptr, float* retFloat) bool GCodeParser::ParseLine(const char* ptr) { + // Truncate at first semicolon (annotations / comment) + const char* comment = strchr(ptr, ';'); + std::string line; + if (comment) { + line = std::string(ptr, comment - ptr); + ptr = line.c_str(); + } + GCToken token; bool validMotion = false; bool exitLoop = false;