From 1dd0d30c985c48308b9d158f2ce0a0e0fcac78dd Mon Sep 17 00:00:00 2001
From: "fabian.peller-konrad@kit.edu" <fabian.peller-konrad@kit.edu>
Date: Thu, 20 May 2021 10:44:05 +0200
Subject: [PATCH] updates to review of fabian reister
---
.../armem/core/base/CoreSegmentBase.h | 6 +-
.../RobotAPI/libraries/aron/core/Randomizer.h | 2 +-
.../codeWriter/cpp/AronCppClass.h | 10 +-
.../codeWriter/cpp/serializer/Serializer.cpp | 159 +++++++++---------
.../typeReader/xml/ReaderFactory.cpp | 4 +-
5 files changed, 97 insertions(+), 84 deletions(-)
diff --git a/source/RobotAPI/libraries/armem/core/base/CoreSegmentBase.h b/source/RobotAPI/libraries/armem/core/base/CoreSegmentBase.h
index ead811e5a..3c3c0ef2c 100644
--- a/source/RobotAPI/libraries/armem/core/base/CoreSegmentBase.h
+++ b/source/RobotAPI/libraries/armem/core/base/CoreSegmentBase.h
@@ -146,10 +146,8 @@ namespace armarx::armem::base
ProviderSegmentT& provSeg = addProviderSegment(update.entityID.providerSegmentName);
return provSeg.update(update);
}
- else
- {
- throw armem::error::MissingEntry("provider segment", update.entityID.providerSegmentName, getLevelName(), this->getKeyString());
- }
+ throw armem::error::MissingEntry("provider segment", update.entityID.providerSegmentName, getLevelName(), this->getKeyString());
+
}
}
diff --git a/source/RobotAPI/libraries/aron/core/Randomizer.h b/source/RobotAPI/libraries/aron/core/Randomizer.h
index 71b174230..bdc272383 100644
--- a/source/RobotAPI/libraries/aron/core/Randomizer.h
+++ b/source/RobotAPI/libraries/aron/core/Randomizer.h
@@ -119,7 +119,7 @@ namespace armarx::aron
t->setSecondAcceptedType(b);
return t;
}
- case type::Descriptor::eEigenMatrix: // TODO
+ case type::Descriptor::eEigenMatrix: // TODO (fabian.peller)
case type::Descriptor::eEigenQuaternion:
case type::Descriptor::eIVTCByteImage:
case type::Descriptor::eOpenCVMat:
diff --git a/source/RobotAPI/libraries/aron/core/codegenerator/codeWriter/cpp/AronCppClass.h b/source/RobotAPI/libraries/aron/core/codegenerator/codeWriter/cpp/AronCppClass.h
index d50039b43..7249a7717 100644
--- a/source/RobotAPI/libraries/aron/core/codegenerator/codeWriter/cpp/AronCppClass.h
+++ b/source/RobotAPI/libraries/aron/core/codegenerator/codeWriter/cpp/AronCppClass.h
@@ -47,9 +47,17 @@ namespace armarx::aron::cppserializer
public:
AronCppClass() = default;
+
+ /// Reset all member values of this class to default (as stated in the XML). This may mean that maybe types are null or false and images may be created as headers_only
virtual void resetHard() = 0;
+
+ /// Reset all member values of this class softly, meaning if a maybe type has a value, we reset only the value (not the full maybe type) and if an image has data (width, height) we keep the data and width and height and only reset teh pixel values
virtual void resetSoft() = 0;
- virtual void read(armarx::aron::dataIO::ReaderInterface& r, bool skip_first_readStartDict = false) = 0;
+
+ /// Set members according to loaded data in ReaderInterface. If skip_first_readStartDict is true, we skip the first call of r.readStartDict() (It was already called outside. This is important for nested classes)
+ virtual void read(armarx::aron::dataIO::ReaderInterface& r, bool skip_first_readStartDict) = 0;
+
+ /// Set the data in WriterInterface to the values of this class. If a maybe type is null or false, this method calls w.writeNull().
virtual void write(armarx::aron::dataIO::WriterInterface& w) const = 0;
};
}
diff --git a/source/RobotAPI/libraries/aron/core/codegenerator/codeWriter/cpp/serializer/Serializer.cpp b/source/RobotAPI/libraries/aron/core/codegenerator/codeWriter/cpp/serializer/Serializer.cpp
index 757995c47..192204704 100644
--- a/source/RobotAPI/libraries/aron/core/codegenerator/codeWriter/cpp/serializer/Serializer.cpp
+++ b/source/RobotAPI/libraries/aron/core/codegenerator/codeWriter/cpp/serializer/Serializer.cpp
@@ -107,6 +107,7 @@ namespace armarx::aron::cppserializer
break;
}
case type::Maybe::eSharedPointer:
+ [[fallthrough]];
case type::Maybe::eUniquePointer:
{
b->addLine("if (" + accessor + " != nullptr)");
@@ -172,6 +173,7 @@ namespace armarx::aron::cppserializer
break;
}
case type::Maybe::eSharedPointer:
+ [[fallthrough]];
case type::Maybe::eUniquePointer:
{
auto s = FromAronTypeNaviagtorPtr(typenavigator);
@@ -203,43 +205,44 @@ namespace armarx::aron::cppserializer
b->addLine("// Comparing two objects of type " + ExtractCppTypename(typenavigator));
}
- if (typenavigator->getMaybe() != type::Maybe::eNone)
+ switch (typenavigator->getMaybe())
{
- switch (typenavigator->getMaybe())
+ case type::Maybe::eNone:
{
- case type::Maybe::eOptional:
- {
- b->addLine("if (" + accessor + ".has_value() and " + otherInstanceAccessor + ".has_value()) // both have a value set");
- b->addBlock(block_if_data);
- b->addLine("if ((" + accessor + ".has_value() and !" + otherInstanceAccessor + ".has_value()) or (!" + accessor + ".has_value() and " + otherInstanceAccessor + ".has_value())) // only one has a value set (XOR)");
- b->addLine("\t return false;");
- break;
- }
- case type::Maybe::eRawPointer:
- {
- b->addLine("if (" + accessor + " != NULL and " + otherInstanceAccessor + " != NULL) // both have a value set");
- b->addBlock(block_if_data);
- b->addLine("if ((" + accessor + " != NULL and " + otherInstanceAccessor + " == NULL) or (" + accessor + " == NULL and " + otherInstanceAccessor + " != NULL)) // only one has a value set (XOR)");
- b->addLine("\t return false;");
- break;
- }
- case type::Maybe::eSharedPointer:
- case type::Maybe::eUniquePointer:
- {
- b->addLine("if (" + accessor + " != nullptr and " + otherInstanceAccessor + " != nullptr) // both have a value set");
- b->addBlock(block_if_data);
- b->addLine("if ((" + accessor + " != nullptr and " + otherInstanceAccessor + " == nullptr) or (" + accessor + " == nullptr and " + otherInstanceAccessor + " != nullptr)) // only one has a value set (XOR)");
- b->addLine("\t return false;");
- break;
- }
- default:
- {
- throw error::MaybeNotValidException("Serializer", "ResolveMaybeEqualsBlock", "Unkown Maybe type (perhaps forgot to add in switch case?)", typenavigator->getMaybe(), typenavigator->getPath());
- }
+ b->appendBlock(block_if_data);
+ break;
+ }
+ case type::Maybe::eOptional:
+ {
+ b->addLine("if (" + accessor + ".has_value() and " + otherInstanceAccessor + ".has_value()) // both have a value set");
+ b->addBlock(block_if_data);
+ b->addLine("if ((" + accessor + ".has_value() and !" + otherInstanceAccessor + ".has_value()) or (!" + accessor + ".has_value() and " + otherInstanceAccessor + ".has_value())) // only one has a value set (XOR)");
+ b->addLine("\t return false;");
+ break;
+ }
+ case type::Maybe::eRawPointer:
+ {
+ b->addLine("if (" + accessor + " != NULL and " + otherInstanceAccessor + " != NULL) // both have a value set");
+ b->addBlock(block_if_data);
+ b->addLine("if ((" + accessor + " != NULL and " + otherInstanceAccessor + " == NULL) or (" + accessor + " == NULL and " + otherInstanceAccessor + " != NULL)) // only one has a value set (XOR)");
+ b->addLine("\t return false;");
+ break;
+ }
+ case type::Maybe::eSharedPointer:
+ [[fallthrough]];
+ case type::Maybe::eUniquePointer:
+ {
+ b->addLine("if (" + accessor + " != nullptr and " + otherInstanceAccessor + " != nullptr) // both have a value set");
+ b->addBlock(block_if_data);
+ b->addLine("if ((" + accessor + " != nullptr and " + otherInstanceAccessor + " == nullptr) or (" + accessor + " == nullptr and " + otherInstanceAccessor + " != nullptr)) // only one has a value set (XOR)");
+ b->addLine("\t return false;");
+ break;
+ }
+ default:
+ {
+ throw error::MaybeNotValidException("Serializer", "ResolveMaybeEqualsBlock", "Unkown Maybe type (perhaps forgot to add in switch case?)", typenavigator->getMaybe(), typenavigator->getPath());
}
- return b;
}
- b->appendBlock(block_if_data);
return b;
}
@@ -256,37 +259,38 @@ namespace armarx::aron::cppserializer
b->addLine("// Ressetting soft the type " + ExtractCppTypename(typenavigator));
}
- if (typenavigator->getMaybe() != type::Maybe::eNone)
+ switch (typenavigator->getMaybe())
{
- switch (typenavigator->getMaybe())
+ case type::Maybe::eNone:
{
- case type::Maybe::eOptional:
- {
- b->addLine("if (" + accessor + ".has_value())");
- b->addBlock(block_if_data);
- break;
- }
- case type::Maybe::eRawPointer:
- {
- b->addLine("if (" + accessor + " != NULL)");
- b->addBlock(block_if_data);
- break;
- }
- case type::Maybe::eSharedPointer:
- case type::Maybe::eUniquePointer:
- {
- b->addLine("if (" + accessor + " != nullptr)");
- b->addBlock(block_if_data);
- break;
- }
- default:
- {
- throw error::MaybeNotValidException("Serializer", "ResolveMaybeResetSoftBlock", "Unkown Maybe type (perhaps forgot to add in switch case?)", typenavigator->getMaybe(), typenavigator->getPath());
- }
+ b->appendBlock(block_if_data);
+ break;
+ }
+ case type::Maybe::eOptional:
+ {
+ b->addLine("if (" + accessor + ".has_value())");
+ b->addBlock(block_if_data);
+ break;
+ }
+ case type::Maybe::eRawPointer:
+ {
+ b->addLine("if (" + accessor + " != NULL)");
+ b->addBlock(block_if_data);
+ break;
+ }
+ case type::Maybe::eSharedPointer:
+ [[fallthrough]];
+ case type::Maybe::eUniquePointer:
+ {
+ b->addLine("if (" + accessor + " != nullptr)");
+ b->addBlock(block_if_data);
+ break;
+ }
+ default:
+ {
+ throw error::MaybeNotValidException("Serializer", "ResolveMaybeResetSoftBlock", "Unkown Maybe type (perhaps forgot to add in switch case?)", typenavigator->getMaybe(), typenavigator->getPath());
}
- return b;
}
- b->appendBlock(block_if_data);
return b;
}
@@ -302,26 +306,29 @@ namespace armarx::aron::cppserializer
b->addLine("// Ressetting hard the type " + ExtractCppTypename(typenavigator));
}
- if (typenavigator->getMaybe() != type::Maybe::eNone)
+ switch (typenavigator->getMaybe())
{
- switch (typenavigator->getMaybe())
+ case type::Maybe::eNone:
{
- case type::Maybe::eOptional:
- case type::Maybe::eRawPointer:
- case type::Maybe::eSharedPointer:
- case type::Maybe::eUniquePointer:
- {
- b->addLine(accessor + " = {};");
- break;
- }
- default:
- {
- throw error::MaybeNotValidException("Serializer", "ResolveMaybeResetSoftBlock", "Unkown Maybe type (perhaps forgot to add in switch case?)", typenavigator->getMaybe(), typenavigator->getPath());
- }
+ b->appendBlock(block_if_data);
+ break;
+ }
+ case type::Maybe::eOptional:
+ [[fallthrough]];
+ case type::Maybe::eRawPointer:
+ [[fallthrough]];
+ case type::Maybe::eSharedPointer:
+ [[fallthrough]];
+ case type::Maybe::eUniquePointer:
+ {
+ b->addLine(accessor + " = {};");
+ break;
+ }
+ default:
+ {
+ throw error::MaybeNotValidException("Serializer", "ResolveMaybeResetSoftBlock", "Unkown Maybe type (perhaps forgot to add in switch case?)", typenavigator->getMaybe(), typenavigator->getPath());
}
- return b;
}
- b->appendBlock(block_if_data);
return b;
}
diff --git a/source/RobotAPI/libraries/aron/core/codegenerator/typeReader/xml/ReaderFactory.cpp b/source/RobotAPI/libraries/aron/core/codegenerator/typeReader/xml/ReaderFactory.cpp
index 2a7310d54..6bc140e98 100644
--- a/source/RobotAPI/libraries/aron/core/codegenerator/typeReader/xml/ReaderFactory.cpp
+++ b/source/RobotAPI/libraries/aron/core/codegenerator/typeReader/xml/ReaderFactory.cpp
@@ -297,8 +297,8 @@ namespace armarx::aron::xmltypereader
Data::EnforceTagName(node, Data::GENERATE_EIGEN_MATRIX_MEMBER_TAG);
Data::EnforceAttribute(node, Data::TYPE_ATTRIBUTE_NAME);
- int rows = std::stoi(Data::GetAttributeWithDefault(node, Data::ROWS_ATTRIBUTE_NAME, "4"));
- int cols = std::stoi(Data::GetAttributeWithDefault(node, Data::COLS_ATTRIBUTE_NAME, "4"));
+ const int rows = std::stoi(Data::GetAttributeWithDefault(node, Data::ROWS_ATTRIBUTE_NAME, "4"));
+ const int cols = std::stoi(Data::GetAttributeWithDefault(node, Data::COLS_ATTRIBUTE_NAME, "4"));
std::string type = node.attribute_value(Data::TYPE_ATTRIBUTE_NAME);
typenavigator::EigenMatrixNavigatorPtr complex(new typenavigator::EigenMatrixNavigator(path));
--
GitLab