From c28d544e900d1c54d578511ae6cc64ca21b33854 Mon Sep 17 00:00:00 2001
From: Tobias Bodmer <Tobias.Bodmer@gmx.de>
Date: Sat, 18 Feb 2023 11:07:41 +0100
Subject: [PATCH] Way better parse error reporting

noticing broken items, even when they are not touched. Also coloring
their parents in an orange color, so that the error is noticable even
when not expanded.

For better generality (and was useful for error reporting): All widgets
introduced inherit from a common base class: CustomWidget. It holds a
signal definition that can be used to directly forward signals to its
QTreeWidgetItem underneath.
---
 .../SkillManagerPlugin/CMakeLists.txt         |   2 +
 .../SkillManagerPlugin/ColorPalettes.cpp      |  13 +-
 .../SkillManagerPlugin/ColorPalettes.h        |   2 +
 .../AronTreeWidgetController.cpp              |  34 ++---
 .../aronTreeWidget/AronTreeWidgetItem.cpp     |  78 +++++++++--
 .../aronTreeWidget/AronTreeWidgetItem.h       |  13 +-
 .../visitors/AronTreeWidgetConverter.cpp      | 123 +++++++++++-------
 .../visitors/AronTreeWidgetConverter.h        |  30 +++--
 .../visitors/AronTreeWidgetCreator.cpp        |  15 ++-
 .../visitors/AronTreeWidgetSetter.cpp         |   1 +
 .../aronTreeWidget/widgets/CustomWidget.cpp   |  42 ++++++
 .../aronTreeWidget/widgets/CustomWidget.h     |  26 ++++
 .../widgets/EditMatrixWidget.cpp              |  10 +-
 .../aronTreeWidget/widgets/EditMatrixWidget.h |  12 +-
 .../aronTreeWidget/widgets/IntEnumWidget.cpp  |   8 +-
 .../aronTreeWidget/widgets/IntEnumWidget.h    |   7 +-
 .../widgets/QuaternionWidget.cpp              |  20 +--
 .../aronTreeWidget/widgets/QuaternionWidget.h |   9 +-
 18 files changed, 322 insertions(+), 123 deletions(-)
 create mode 100644 source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/CustomWidget.cpp
 create mode 100644 source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/CustomWidget.h

diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/CMakeLists.txt b/source/RobotAPI/gui-plugins/SkillManagerPlugin/CMakeLists.txt
index 749f70029..d62a96edd 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/CMakeLists.txt
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/CMakeLists.txt
@@ -11,6 +11,7 @@ set(SOURCES
     aronTreeWidget/visitors/AronTreeWidgetModalCreator.cpp
     aronTreeWidget/visitors/AronTreeWidgetContextMenu.cpp
     aronTreeWidget/Data.cpp
+    aronTreeWidget/widgets/CustomWidget.cpp
     aronTreeWidget/widgets/EditMatrixWidget.cpp
     aronTreeWidget/widgets/IntEnumWidget.cpp
     aronTreeWidget/ListDictHelper.cpp
@@ -32,6 +33,7 @@ set(HEADERS
     aronTreeWidget/Data.h
     aronTreeWidget/widgets/NDArrayHelper.h
     aronTreeWidget/widgets/EditMatrixWidget.h
+    aronTreeWidget/widgets/CustomWidget.h
     aronTreeWidget/widgets/IntEnumWidget.h
     aronTreeWidget/ListDictHelper.h
     aronTreeWidget/widgets/QuaternionWidget.h
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/ColorPalettes.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/ColorPalettes.cpp
index d263456d7..05cd7a5cc 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/ColorPalettes.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/ColorPalettes.cpp
@@ -7,7 +7,6 @@ namespace armarx::gui_color_palette
     {
         QPalette errorPalette;
         errorPalette.setColor(QPalette::Base, Qt::red);
-        errorPalette.setColor(QPalette::Text, Qt::black);
         return errorPalette;
     }
     QPalette
@@ -17,7 +16,17 @@ namespace armarx::gui_color_palette
         QPalette normalPalette;
 
         normalPalette.setColor(QPalette::Base, Qt::white);
-        normalPalette.setColor(QPalette::Text, Qt::black);
         return normalPalette;
     }
+
+    QPalette
+    getIndirectErrorPalette()
+    {
+        QPalette indirectErr;
+        static QColor orange = QColor::fromRgb(255, 165, 0);
+        // orange color
+        indirectErr.setColor(QPalette::Base, orange);
+        return indirectErr;
+    }
+
 } // namespace armarx::gui_color_palette
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/ColorPalettes.h b/source/RobotAPI/gui-plugins/SkillManagerPlugin/ColorPalettes.h
index ade58f431..ea2c247a9 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/ColorPalettes.h
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/ColorPalettes.h
@@ -6,4 +6,6 @@ namespace armarx::gui_color_palette
     QPalette getNormalPalette();
 
     QPalette getErrorPalette();
+
+    QPalette getIndirectErrorPalette();
 } // namespace armarx::gui_color_palette
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetController.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetController.cpp
index a2493b0e1..e1bc78574 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetController.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetController.cpp
@@ -127,32 +127,24 @@ namespace armarx
     void
     AronTreeWidgetController::onTreeWidgetItemChanged(QTreeWidgetItem* item, int column)
     {
-        auto* aronElem = AronTreeWidgetItem::DynamicCast(item);
-        auto* parent = AronTreeWidgetItem::DynamicCast(item->parent());
+        tree->blockSignals(true);
 
-        if (!aronElem || !parent)
-        {
-            return;
-        }
-        if (column == 1)
+        auto* aronElem = AronTreeWidgetItem::DynamicCast(item);
+        if (aronElem)
         {
-            // Try to convert the type into aron to find errors
-            AronTreeWidgetConverterVisitor v(item->parent(), item->parent()->indexOfChild(item));
-            aron::type::visit(v, aronElem->aronType);
-            // set the state accordingly
-            aronElem->setValueErrorState(!v.isConversionSuccessful());
+            aronElem->onUserChange(column);
         }
-        else if (column == 0)
+        // start conversion for entire tree -- this also sets the highlighting
+        ARMARX_CHECK(parent->childCount() == 1);
+        auto* aronTreeRoot = AronTreeWidgetItem::DynamicCast(parent->child(0));
+        aronTreeRoot->resetError();
+        if (aronTreeRoot)
         {
-            // check if the element is child of a dict. If so, validate uniqueness of keys
-            if (parent->aronType->getDescriptor() == aron::type::Descriptor::DICT)
-            {
-                parent->checkKeyValidityOfChildren();
-            }
-            // maybe while editing keys, we try to edit a key that should not change by tabbing.
-            // Catch that here and undo the edit
-            aronElem->preventIllegalKeyChange();
+            AronTreeWidgetConverterVisitor v(parent, 0);
+            aron::type::visit(v, type);
+            aronTreeRoot->setValueErrorState(v.hasDirectError(), v.onlyChildFailedConversion());
         }
+        tree->blockSignals(false);
     }
 
 } // namespace armarx
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetItem.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetItem.cpp
index 7ab298945..29d895f7b 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetItem.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetItem.cpp
@@ -5,6 +5,10 @@
 
 #include <RobotAPI/libraries/aron/core/type/variant/All.h>
 
+#include "../ColorPalettes.h"
+#include "visitors/AronTreeWidgetConverter.h"
+#include "widgets/CustomWidget.h"
+
 namespace armarx
 {
     AronTreeWidgetItem*
@@ -32,31 +36,53 @@ namespace armarx
     }
 
     void
-    AronTreeWidgetItem::setValueErrorState(bool hasError)
+    AronTreeWidgetItem::setValueErrorState(bool isErrorSource, bool isTransitiveError)
     {
-        itemValueError = hasError;
-        if (hasError)
+        itemValueError |= isErrorSource;
+        transitiveValueError |= isTransitiveError;
+
+
+        if (CustomWidget::DynamicCast(treeWidget()->itemWidget(this, 1)))
         {
-            QTreeWidgetItem::setBackgroundColor(1, Qt::red);
+            // The widgets handle errors themselves
+            return;
         }
-        else
+        auto palette = gui_color_palette::getNormalPalette();
+        if (itemValueError)
         {
-            QTreeWidgetItem::setBackground(1, Qt::white);
+            palette = gui_color_palette::getErrorPalette();
         }
+        else if (transitiveValueError)
+        {
+            palette = gui_color_palette::getIndirectErrorPalette();
+        }
+
+        QTreeWidgetItem::setBackground(1, QBrush(palette.color(QPalette::Base)));
     }
 
     void
     AronTreeWidgetItem::setKeyErrorState(bool hasError)
     {
         ARMARX_CHECK(col0Editable); //only editable keys should call this function!
+        auto palette =
+            hasError ? gui_color_palette::getErrorPalette() : gui_color_palette::getNormalPalette();
+
         keyValueError = hasError;
-        if (hasError)
-        {
-            QTreeWidgetItem::setBackgroundColor(0, Qt::red);
-        }
-        else
+
+        QTreeWidgetItem::setBackground(1, QBrush(palette.color(QPalette::Base)));
+    }
+
+    void
+    AronTreeWidgetItem::resetError()
+    {
+        keyValueError = false;
+        itemValueError = false;
+        transitiveValueError = false;
+        // also reset children
+        for (int i = 0; i < childCount(); ++i)
         {
-            QTreeWidgetItem::setBackground(0, Qt::white);
+            auto* arChild = AronTreeWidgetItem::DynamicCastAndCheck(QTreeWidgetItem::child(i));
+            arChild->resetError();
         }
     }
 
@@ -121,6 +147,34 @@ namespace armarx
         }
     }
 
+    void
+    AronTreeWidgetItem::onUserChange(int changedColumn)
+    {
+        QTreeWidgetItem* qParent = QTreeWidgetItem::parent();
+        ARMARX_CHECK(qParent);
+        AronTreeWidgetItem* aronParent = DynamicCast(qParent);
+        if (changedColumn == 0)
+        {
+            if (col0Editable)
+            {
+                // Topmost should always be an Object and col0 is not editable in the child then...
+                // -> aronParent should never be nullptr
+                ARMARX_CHECK(aronParent);
+                // check if the element is child of a dict. If so, validate uniqueness of keys
+                if (aronParent->aronType->getDescriptor() == aron::type::Descriptor::DICT)
+                {
+                    aronParent->checkKeyValidityOfChildren();
+                }
+            }
+            else
+            {
+                // maybe while editing keys, we try to edit a key that should not change by tabbing.
+                // Catch that here and undo the edit
+                preventIllegalKeyChange();
+            }
+        }
+    }
+
     void
     AronTreeWidgetItem::preventIllegalKeyChange()
     {
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetItem.h b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetItem.h
index 7da21fa54..ece37b918 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetItem.h
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/AronTreeWidgetItem.h
@@ -34,19 +34,28 @@ namespace armarx
         const bool col1Editable = false;
 
         bool isValueErrorneous();
-        void setValueErrorState(bool hasError);
+        // marks the gui in a specific color. Also stores if there is currently an error.
+        // the only way to reset the color back to normal is with resetError() (error is sticky)
+        void setValueErrorState(bool isErrorSource, bool isTransitiveError);
         void setKeyErrorState(bool hasError);
+        // reset error storage that influences coloring for this and all children
+        void resetError();
 
         // Checks if the children of a dict are unique
         // should not be called on other types! (does nothing then)
         void checkKeyValidityOfChildren();
 
+        // main logic on changes. Gets called from the onTreeWidgetItemChanged() slot in AronTreeWidgetController.
+        // (This class cannot directly consume this signal, at least I did not find a nice way...)
+        void onUserChange(int changedColumn);
+
+    private:
         // because the editable keyword counts for all columns, it is possible to TAB into uneditable keys.
         // We do not want those to change, so just change them back once they finished editing.
         void preventIllegalKeyChange();
 
-    private:
         bool itemValueError = false;
+        bool transitiveValueError = false;
         bool keyValueError = false;
         // hacky storage of previous key value for items that usually should not be editable.
         // Problem: Dict-keys are editable, one can tab from that to any other value field and edit it.
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetConverter.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetConverter.cpp
index d39095941..240d0e8c3 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetConverter.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetConverter.cpp
@@ -45,9 +45,44 @@ namespace armarx
     bool
     AronTreeWidgetConverterVisitor::isConversionSuccessful()
     {
-        return !errorOccured;
+        return !isDirectError && !hasTransitiveError;
+    }
+
+    bool
+    AronTreeWidgetConverterVisitor::onlyChildFailedConversion()
+    {
+        return hasTransitiveError;
+    }
+
+    bool
+    AronTreeWidgetConverterVisitor::hasDirectError() const
+    {
+        return isDirectError;
     }
 
+    void
+    AronTreeWidgetConverterVisitor::handleErrors(AronTreeWidgetConverterVisitor childV,
+                                                 bool ownFault)
+    {
+        // TODO debug this. Make target back to debugabble
+        isDirectError |= ownFault;
+        hasTransitiveError |= childV.isDirectError || childV.hasTransitiveError;
+
+        auto* aronItem = AronTreeWidgetItem::DynamicCast(parentItem->child(index));
+        ARMARX_CHECK(aronItem);
+        aronItem->setValueErrorState(isDirectError, hasTransitiveError);
+    }
+
+    void
+    AronTreeWidgetConverterVisitor::handleErrors(bool ownFault)
+    {
+        isDirectError = ownFault;
+        auto* aronItem = AronTreeWidgetItem::DynamicCast(parentItem->child(index));
+        ARMARX_CHECK(aronItem);
+        aronItem->setValueErrorState(isDirectError, false);
+    }
+
+
     void
     AronTreeWidgetConverterVisitor::visitAronVariant(const aron::type::ObjectPtr& i)
     {
@@ -61,11 +96,8 @@ namespace armarx
             AronTreeWidgetConverterVisitor v(el, x++);
             aron::type::visit(v, value);
 
-            if (v.errorOccured || !v.createdAron.get())
-            {
-                errorOccured = true;
-            }
-            else
+            handleErrors(v);
+            if (v.isConversionSuccessful())
             {
                 createdAronDict->addElement(key, v.createdAron);
             }
@@ -84,23 +116,12 @@ namespace armarx
             auto it = el->child(x);
             AronTreeWidgetConverterVisitor v(el, x);
             aron::type::visit(v, i->getAcceptedType());
-
-            if (v.createdAron && !v.errorOccured)
+            auto key = it->text(0).toStdString();
+            // TODO: handle key errors more elegantly / separately
+            handleErrors(v, createdAronDict->hasElement(key));
+            if (v.createdAron && v.isConversionSuccessful() && !createdAronDict->hasElement(key))
             {
-                // TODO find more elegant solution that does not crash aron code
-                auto key = it->text(0).toStdString();
-                if (createdAronDict->hasElement(key))
-                {
-                    errorOccured = true;
-                }
-                else
-                {
-                    createdAronDict->addElement(key, v.createdAron);
-                }
-            }
-            else
-            {
-                errorOccured = true;
+                createdAronDict->addElement(key, v.createdAron);
             }
         }
     }
@@ -117,14 +138,12 @@ namespace armarx
         {
             AronTreeWidgetConverterVisitor convVisitor(elem, j);
             aron::type::visit(convVisitor, childrenTypes[0]);
-            if (convVisitor.createdAron && !convVisitor.errorOccured)
+            handleErrors(convVisitor);
+
+            if (convVisitor.createdAron && convVisitor.isConversionSuccessful())
             {
                 createdAronList->addElement(convVisitor.createdAron);
             }
-            else
-            {
-                errorOccured = true;
-            }
         }
     }
 
@@ -139,14 +158,11 @@ namespace armarx
         for (int j = 0; j < 2; ++j)
         {
             AronTreeWidgetConverterVisitor convVisitor(elem, j);
-            if (convVisitor.createdAron && !convVisitor.errorOccured)
+            handleErrors(convVisitor);
+            if (convVisitor.createdAron && convVisitor.isConversionSuccessful())
             {
                 createdAronPair->addElement(convVisitor.createdAron);
             }
-            else
-            {
-                errorOccured = true;
-            }
         }
     }
 
@@ -162,15 +178,12 @@ namespace armarx
         {
             AronTreeWidgetConverterVisitor convVisitor(currElem, j);
             aron::type::visit(convVisitor, i);
+            handleErrors(convVisitor);
 
-            if (convVisitor.createdAron && !convVisitor.errorOccured)
+            if (convVisitor.createdAron && convVisitor.isConversionSuccessful())
             {
                 createdAronTuple->addElement(convVisitor.createdAron);
             }
-            else
-            {
-                errorOccured = true;
-            }
         }
     }
 
@@ -209,11 +222,13 @@ namespace armarx
         ARMARX_CHECK(rootWidget);
         auto* widget = rootWidget->itemWidget(currElem, 1);
         auto* matrixWidget = EditMatrixWidget::DynamicCastAndCheck(widget);
+
+        handleErrors(matrixWidget->hasParseErrors());
         if (matrixWidget->hasParseErrors())
         {
-            errorOccured = true;
             return;
         }
+        // write to aron data
         std::vector<unsigned char> elems;
         elems.reserve(totalByteSize);
         // CAUTION: Raw data has column based storage
@@ -241,11 +256,15 @@ namespace armarx
         auto* currTreeElem = parentItem->child(index);
         auto* itemWidget = currTreeElem->treeWidget()->itemWidget(currTreeElem, 1);
         auto* quatWidget = QuaternionWidget::DynamicCastAndCheck(itemWidget);
+
+        // error handling
+        handleErrors(quatWidget->hasParseErrors());
         if (quatWidget->hasParseErrors())
         {
-            errorOccured = true;
             return;
         }
+
+        // write to aron data
         auto serialized = quatWidget->parseAllToNDArray();
         if ((int)serialized.size() != dataSize * 4)
         {
@@ -280,7 +299,8 @@ namespace armarx
         }
         bool success;
         std::tie(success, createdAron) = intEnumWidget->parseToAron();
-        errorOccured &= success;
+
+        handleErrors(!success);
     }
 
     void
@@ -303,12 +323,12 @@ namespace armarx
         }
         catch (const simox::error::SimoxError& err)
         {
-
-
-            errorOccured = true;
+            handleErrors();
             ARMARX_VERBOSE << "Conversion from String to Int failed. Error:\"" << err.what()
                            << "\"";
+            return;
         }
+        handleErrors(false);
     }
 
     void
@@ -321,6 +341,7 @@ namespace armarx
         std::string str = el->text(1).toStdString();
         if (str.empty())
         {
+            //TODO: similar behaviour for rest?
             str = el->text(3).toStdString();
         }
         try
@@ -329,10 +350,12 @@ namespace armarx
         }
         catch (const simox::error::SimoxError& err)
         {
-            errorOccured = true;
+            handleErrors();
             ARMARX_VERBOSE << "Conversion from String to Long failed. Error:\"" << err.what()
                            << "\"";
+            return;
         }
+        handleErrors(false);
     }
 
     void
@@ -353,10 +376,12 @@ namespace armarx
         }
         catch (const simox::error::SimoxError& err)
         {
-            errorOccured = true;
+            handleErrors();
             ARMARX_VERBOSE << "Conversion from String to Float failed. Error:\"" << err.what()
                            << "\"";
+            return;
         }
+        handleErrors(false);
     }
 
     void
@@ -377,10 +402,12 @@ namespace armarx
         }
         catch (const simox::error::SimoxError& err)
         {
-            errorOccured = true;
+            handleErrors();
             ARMARX_VERBOSE << "Conversion from String to Double failed. Error:\"" << err.what()
                            << "\"";
+            return;
         }
+        handleErrors(false);
     }
 
     void
@@ -401,10 +428,12 @@ namespace armarx
         }
         catch (const simox::error::SimoxError& err)
         {
-            errorOccured = true;
+            handleErrors();
             ARMARX_VERBOSE << "Conversion from String to Bool failed. Error:\"" << err.what()
                            << "\"";
+            return;
         }
+        handleErrors(false);
     }
 
     void
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetConverter.h b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetConverter.h
index 8fb8b6fbe..c127ac5e4 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetConverter.h
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetConverter.h
@@ -21,8 +21,8 @@
  */
 #pragma once
 
-#include <RobotAPI/libraries/aron/core/type/variant/All.h>
 #include <RobotAPI/libraries/aron/core/data/variant/All.h>
+#include <RobotAPI/libraries/aron/core/type/variant/All.h>
 #include <RobotAPI/libraries/aron/core/type/visitor/variant/VariantVisitor.h>
 
 // forward declarations of qt
@@ -32,8 +32,7 @@ class QTreeWidgetItem;
 namespace armarx
 {
     // Conversion from TreeView to aron data
-    class AronTreeWidgetConverterVisitor :
-            public armarx::aron::type::ConstVariantVisitor
+    class AronTreeWidgetConverterVisitor : public armarx::aron::type::ConstVariantVisitor
     {
     public:
         QTreeWidgetItem* parentItem;
@@ -41,16 +40,27 @@ namespace armarx
         aron::data::VariantPtr createdAron = nullptr;
 
         AronTreeWidgetConverterVisitor() = delete;
-        AronTreeWidgetConverterVisitor(QTreeWidgetItem* i, int x) :
-            parentItem(i), index(x)
-        {}
+        AronTreeWidgetConverterVisitor(QTreeWidgetItem* i, int x) : parentItem(i), index(x)
+        {
+        }
+        // if the conversion was successful after calling visit()
         bool isConversionSuccessful();
+        // returns true if this type itself was sucessfully parsed, but some contained object failed.
+        // also false if there is no error
+        bool onlyChildFailedConversion();
+
+        bool hasDirectError() const;
 
     private:
-        bool errorOccured = false;
+        bool isDirectError = false;
+        bool hasTransitiveError = false;
+        // adds all errors from other visitor to our own error collection -> collecting errors
+        // with ownFault, we also add this node to the collection
+        void handleErrors(AronTreeWidgetConverterVisitor childV, bool ownFault = false);
+        // we are the cause...
+        void handleErrors(bool ownFault = true);
 
     public:
-
         void visitAronVariant(const aron::type::ObjectPtr&) final;
         void visitAronVariant(const aron::type::DictPtr&) final;
         void visitAronVariant(const aron::type::PairPtr&) final;
@@ -70,6 +80,4 @@ namespace armarx
         void visitAronVariant(const aron::type::StringPtr&) final;
         void visitUnknown(Input&) final;
     };
-}
-
-
+} // namespace armarx
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetCreator.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetCreator.cpp
index 02abb5c7a..ccc80db3c 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetCreator.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetCreator.cpp
@@ -202,24 +202,25 @@ namespace armarx
     void
     AronTreeWidgetCreatorVisitor::visitAronVariant(const aron::type::MatrixPtr& i)
     {
-        editableValue = true;
+        editableValue = false;
         insertNewTreeViewWidget(i, "");
         // TODO add size and type info to 2nd column manually here
 
         // Widget fiddling source: https://blog.manash.io/quick-qt-6-how-to-add-qpushbutton-or-widgets-to-a-qtreewidget-as-qtreewidgetitem-2ae9f54c0e5f
         // overlay custom widget in column 1
         auto* toplevelWidget = createdQWidgetItem->treeWidget();
-        EditMatrixWidget* matWidget =
-            new EditMatrixWidget(i->getRows(), i->getCols(), i->getElementType());
+        EditMatrixWidget* matWidget = new EditMatrixWidget(
+            i->getRows(), i->getCols(), i->getElementType(), createdQWidgetItem);
         toplevelWidget->setItemWidget(createdQWidgetItem, 1, matWidget);
     }
     void
     AronTreeWidgetCreatorVisitor::visitAronVariant(const aron::type::QuaternionPtr& i)
     {
-        editableValue = true;
+        editableValue = false;
         insertNewTreeViewWidget(i, "");
         auto* toplevelWidget = createdQWidgetItem->treeWidget();
-        QuaternionWidget* quatWidget = new QuaternionWidget(i->getElementType());
+        QuaternionWidget* quatWidget =
+            new QuaternionWidget(i->getElementType(), createdQWidgetItem);
         toplevelWidget->setItemWidget(createdQWidgetItem, 1, quatWidget);
     }
     void
@@ -235,13 +236,13 @@ namespace armarx
     void
     AronTreeWidgetCreatorVisitor::visitAronVariant(const aron::type::IntEnumPtr& i)
     {
-        editableValue = true;
+        editableValue = false;
         ARMARX_CHECK_NOT_NULL(i);
         ARMARX_CHECK_GREATER(i->getAcceptedValueNames().size(), 0);
 
         insertNewTreeViewWidget(i, "");
         // TODO set combobox widget with correct content
-        IntEnumWidget* widget = new IntEnumWidget(i);
+        IntEnumWidget* widget = new IntEnumWidget(i, createdQWidgetItem);
         //widget->postCtorCall();
         createdQWidgetItem->treeWidget()->setItemWidget(createdQWidgetItem, 1, widget);
     }
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetSetter.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetSetter.cpp
index 279e8bf08..cf719f8a2 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetSetter.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/visitors/AronTreeWidgetSetter.cpp
@@ -119,6 +119,7 @@ namespace armarx
             for (const auto& [key, value] : i->getElements())
             {
                 el->child(x)->setText(0, {key.c_str()});
+
                 AronTreeWidgetSetterVisitor v(el, x++);
                 aron::data::visit(v, value);
             }
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/CustomWidget.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/CustomWidget.cpp
new file mode 100644
index 000000000..51836fde9
--- /dev/null
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/CustomWidget.cpp
@@ -0,0 +1,42 @@
+#include "CustomWidget.h"
+
+#include "ArmarXCore/core/exceptions/local/ExpressionException.h"
+
+namespace armarx
+{
+    CustomWidget::CustomWidget(QTreeWidgetItem* overlayingItem) : overlayingItem(overlayingItem)
+    {
+        // connect to general QTreeWidgetItem callback.
+        // This also lets the conversion start for other objects. (Not just this widget)
+        ARMARX_CHECK(connect(this,
+                             SIGNAL(elemChanged(QTreeWidgetItem*, int)),
+                             overlayingItem->treeWidget(),
+                             SIGNAL(itemChanged(QTreeWidgetItem*, int))));
+    }
+
+    CustomWidget*
+    CustomWidget::DynamicCast(QWidget* elem)
+    {
+        return dynamic_cast<CustomWidget*>(elem);
+    }
+
+    CustomWidget*
+    CustomWidget::DynamicCastAndCheck(QWidget* elem)
+    {
+        if (!elem)
+        {
+            return nullptr;
+        }
+        auto* casted = DynamicCast(elem);
+        ARMARX_CHECK_NOT_NULL(casted);
+        return casted;
+    }
+
+    void
+    CustomWidget::setSupressSignals(bool doSupress)
+    {
+        QObject::blockSignals(doSupress);
+    }
+
+
+} // namespace armarx
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/CustomWidget.h b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/CustomWidget.h
new file mode 100644
index 000000000..49477ffa1
--- /dev/null
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/CustomWidget.h
@@ -0,0 +1,26 @@
+#pragma once
+
+#include <QTreeWidgetItem>
+#include <QWidget>
+
+namespace armarx
+{
+
+    class CustomWidget : public QWidget
+    {
+        Q_OBJECT
+
+    public:
+        CustomWidget(QTreeWidgetItem* overlayingItem);
+        static CustomWidget* DynamicCast(QWidget*);
+        static CustomWidget* DynamicCastAndCheck(QWidget*);
+
+        virtual void setSupressSignals(bool doSupress);
+
+    protected:
+        QTreeWidgetItem* overlayingItem;
+
+    signals:
+        void elemChanged(QTreeWidgetItem* elem, int col);
+    };
+} // namespace armarx
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/EditMatrixWidget.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/EditMatrixWidget.cpp
index de5f5c331..a3d6f0ffd 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/EditMatrixWidget.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/EditMatrixWidget.cpp
@@ -14,7 +14,9 @@ namespace armarx
 {
     EditMatrixWidget::EditMatrixWidget(size_t numRows,
                                        size_t numCols,
-                                       aron::type::matrix::ElementType elemType)
+                                       aron::type::matrix::ElementType elemType,
+                                       QTreeWidgetItem* currentItem) :
+        CustomWidget(currentItem)
     {
         realRows = numRows;
         realCols = numCols;
@@ -75,7 +77,8 @@ namespace armarx
 
         for (size_t i = 0; i < dispElements.size(); ++i)
         {
-            connect(dispElements[i], SIGNAL(editingFinished()), this, SLOT(matrixElementChanged()));
+            ARMARX_CHECK(connect(
+                dispElements[i], SIGNAL(editingFinished()), this, SLOT(matrixElementChanged())));
         }
     }
 
@@ -239,6 +242,9 @@ namespace armarx
     void
     EditMatrixWidget::matrixElementChanged()
     {
+        blockSignals(true);
         highlightUnparsableEntries();
+        blockSignals(false);
+        emit elemChanged(overlayingItem, 1);
     }
 } // namespace armarx
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/EditMatrixWidget.h b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/EditMatrixWidget.h
index 5948d0c27..b086fb2ca 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/EditMatrixWidget.h
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/EditMatrixWidget.h
@@ -4,21 +4,26 @@
 #include <QHBoxLayout>
 #include <QLineEdit>
 #include <QObject>
+#include <QTreeWidgetItem>
 #include <QVBoxLayout>
-#include <QWidget>
 
 #include "RobotAPI/libraries/aron/core/type/variant/ndarray/Matrix.h"
 
+#include "CustomWidget.h"
+
 namespace armarx
 {
     // Custom Widget used to display Matrices
     // MAX_ROWS_DISPLAY and MAX_COLS_DISPLAY are the maximal number of elements displayable
     // everything else will not be displayed. However, there can still be made changes to this data with setText()
-    class EditMatrixWidget : public QWidget
+    class EditMatrixWidget : public CustomWidget
     {
         Q_OBJECT
     public:
-        EditMatrixWidget(size_t numRows, size_t numCols, aron::type::matrix::ElementType elemType);
+        EditMatrixWidget(size_t numRows,
+                         size_t numCols,
+                         aron::type::matrix::ElementType elemType,
+                         QTreeWidgetItem* currentWidget);
 
         static EditMatrixWidget* DynamicCast(QWidget*);
         static EditMatrixWidget* DynamicCastAndCheck(QWidget*);
@@ -31,6 +36,7 @@ namespace armarx
         // returns an empty vector if parsing failed. Else, contains the individual bytes.
         std::vector<unsigned char> parseElement(size_t row, size_t col);
 
+
     private:
         // Dimensions of the underlying Type to represent
         size_t realRows;
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/IntEnumWidget.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/IntEnumWidget.cpp
index 578a65acb..bfe1be2bf 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/IntEnumWidget.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/IntEnumWidget.cpp
@@ -9,7 +9,9 @@
 
 namespace armarx
 {
-    IntEnumWidget::IntEnumWidget(const aron::type::IntEnumPtr& enumPtr) : element_type(enumPtr)
+    IntEnumWidget::IntEnumWidget(const aron::type::IntEnumPtr& enumPtr,
+                                 QTreeWidgetItem* currentItem) :
+        CustomWidget(currentItem), element_type(enumPtr)
     {
         auto names = enumPtr->getAcceptedValueNames();
         auto outerLayout = new QHBoxLayout();
@@ -128,6 +130,10 @@ namespace armarx
     void
     IntEnumWidget::textChanged(const QString&)
     {
+        setSupressSignals(true);
         highlightUnparsableEntries();
+        setSupressSignals(false);
+        // fire change signal
+        emit CustomWidget::elemChanged(overlayingItem, 1);
     }
 } // namespace armarx
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/IntEnumWidget.h b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/IntEnumWidget.h
index 72e568f2a..095ea480f 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/IntEnumWidget.h
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/IntEnumWidget.h
@@ -1,23 +1,23 @@
 #pragma once
 
 #include <QComboBox>
-#include <QWidget>
 
 #include "RobotAPI/libraries/aron/core/data/variant/primitive/Int.h"
 #include "RobotAPI/libraries/aron/core/type/variant/forward_declarations.h"
 
 #include "../AronTreeWidgetItem.h"
+#include "CustomWidget.h"
 
 namespace armarx
 {
     // Custom wrapper around the QComboBox widget to represent IntEnums
     // This class handles parsing once the user edited the field and is also responsible
     // to parse the according aron data from text
-    class IntEnumWidget : public QWidget
+    class IntEnumWidget : public CustomWidget
     {
         Q_OBJECT
     public:
-        IntEnumWidget(const aron::type::IntEnumPtr&);
+        IntEnumWidget(const aron::type::IntEnumPtr&, QTreeWidgetItem* currentItem);
         // needs to be called after the constructor, because qt needs the meta object to be fully constructed at this point
         void postCtorCall();
 
@@ -32,6 +32,7 @@ namespace armarx
     private:
         const aron::type::IntEnumPtr element_type;
         QComboBox* innerWidget;
+
         bool noParseErrors = true;
 
         void highlightUnparsableEntries();
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/QuaternionWidget.cpp b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/QuaternionWidget.cpp
index 39f80e503..b1d6fa3b9 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/QuaternionWidget.cpp
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/QuaternionWidget.cpp
@@ -38,11 +38,12 @@ namespace armarx
     bool
     QuaternionWidget::hasParseErrors()
     {
-        return !noParseErrors;
+        return parseErrors;
     }
 
-    armarx::QuaternionWidget::QuaternionWidget(aron::type::quaternion::ElementType elType) :
-        element_type(elType)
+    armarx::QuaternionWidget::QuaternionWidget(aron::type::quaternion::ElementType elType,
+                                               QTreeWidgetItem* currentItem) :
+        CustomWidget(currentItem), element_type(elType)
     {
         auto outerLayout = new QHBoxLayout();
         auto labelCol1 = new QVBoxLayout();
@@ -76,7 +77,8 @@ namespace armarx
 
         for (const auto& el : xyzw_components)
         {
-            connect(el, SIGNAL(editingFinished()), this, SLOT(quaternionElementChanged()));
+            ARMARX_CHECK(
+                connect(el, SIGNAL(editingFinished()), this, SLOT(quaternionElementChanged())));
         }
 
         setLayout(outerLayout);
@@ -86,7 +88,7 @@ namespace armarx
     void
     QuaternionWidget::highlightUnparsableEntries()
     {
-        noParseErrors = true;
+        parseErrors = false;
         for (size_t i = 0; i < 4; ++i)
         {
             bool ok = false;
@@ -98,7 +100,6 @@ namespace armarx
                     std::tie(ok, flt) = parseQuatVals<float>((QuaternionComponents)i);
                     break;
                 }
-
                 case armarx::aron::type::quaternion::FLOAT64:
                 {
                     double dbl;
@@ -113,7 +114,7 @@ namespace armarx
             else
             {
                 xyzw_components.at(i)->setPalette(gui_color_palette::getErrorPalette());
-                noParseErrors = false;
+                parseErrors = true;
             }
         }
     }
@@ -130,7 +131,7 @@ namespace armarx
             {
                 auto bytevec =
                     NDArrayHelper::toByteVector<float>(xyzw_components.at(i)->text().toStdString());
-                success &= bytevec.size();
+                success &= !bytevec.empty();
                 res.insert(res.end(), bytevec.begin(), bytevec.end());
             }
         }
@@ -151,6 +152,9 @@ namespace armarx
     void
     QuaternionWidget::quaternionElementChanged()
     {
+        CustomWidget::setSupressSignals(true);
         highlightUnparsableEntries();
+        CustomWidget::setSupressSignals(false);
+        emit CustomWidget::elemChanged(overlayingItem, 1);
     }
 } // namespace armarx
diff --git a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/QuaternionWidget.h b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/QuaternionWidget.h
index 869ff7ce7..d55d2bba3 100644
--- a/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/QuaternionWidget.h
+++ b/source/RobotAPI/gui-plugins/SkillManagerPlugin/aronTreeWidget/widgets/QuaternionWidget.h
@@ -4,20 +4,21 @@
 #include <QLineEdit>
 #include <QObject>
 #include <QVBoxLayout>
-#include <QWidget>
 
 #include "RobotAPI/libraries/aron/core/type/variant/ndarray/Matrix.h"
 
+#include "CustomWidget.h"
+
 
 namespace armarx
 {
     // Custom Widget which represents a Quaternion - This class does not normalize the inputs
     // It can parse its data to NDArray and also handles user edit signals itself. (And also the highlighting if errors occur)
-    class QuaternionWidget : public QWidget
+    class QuaternionWidget : public CustomWidget
     {
         Q_OBJECT
     public:
-        QuaternionWidget(aron::type::quaternion::ElementType);
+        QuaternionWidget(aron::type::quaternion::ElementType type, QTreeWidgetItem* currentItem);
 
         static QuaternionWidget* DynamicCast(QWidget*);
         static QuaternionWidget* DynamicCastAndCheck(QWidget*);
@@ -37,7 +38,7 @@ namespace armarx
     private:
         std::vector<QLineEdit*> xyzw_components;
         aron::type::quaternion::ElementType element_type;
-        bool noParseErrors = true;
+        bool parseErrors = false;
 
         void highlightUnparsableEntries();
         template <typename T>
-- 
GitLab