From 11298761662869197868a1ae0bfe3074de40df9d Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Wed, 11 Aug 2021 16:22:20 +0200
Subject: [PATCH] Fix max history size inheritance on adding children
---
.../armem/core/base/CoreSegmentBase.h | 21 ++++-----
.../libraries/armem/core/base/MemoryBase.h | 43 +++++++------------
.../armem/core/base/ProviderSegmentBase.h | 22 +++++++---
.../core/base/detail/MemoryContainerBase.h | 22 ++++++++++
.../armem/server/wm/detail/MaxHistorySize.h | 2 -
.../armem/server/wm/memory_definitions.h | 21 +++++++--
.../libraries/armem/test/ArMemMemoryTest.cpp | 5 ++-
7 files changed, 84 insertions(+), 52 deletions(-)
diff --git a/source/RobotAPI/libraries/armem/core/base/CoreSegmentBase.h b/source/RobotAPI/libraries/armem/core/base/CoreSegmentBase.h
index 1cc0a66fd..aa3a1c903 100644
--- a/source/RobotAPI/libraries/armem/core/base/CoreSegmentBase.h
+++ b/source/RobotAPI/libraries/armem/core/base/CoreSegmentBase.h
@@ -272,27 +272,28 @@ namespace armarx::armem::base
ProviderSegmentT& addProviderSegment(const std::string& name, aron::typenavigator::ObjectNavigatorPtr providerSegmentType = nullptr)
{
aron::typenavigator::ObjectNavigatorPtr type = providerSegmentType ? providerSegmentType : this->aronType();
- return addProviderSegment(ProviderSegmentT(name, this->id(), type));
+ return this->_derived().addProviderSegment(name, name, type);
}
/// Copy and insert a provider segment.
ProviderSegmentT& addProviderSegment(const ProviderSegmentT& providerSegment)
{
- return addProviderSegment(ProviderSegmentT(providerSegment));
+ return this->_derived().addProviderSegment(providerSegment.name(), ProviderSegmentT(providerSegment));
}
/// Move and insert a provider segment.
ProviderSegmentT& addProviderSegment(ProviderSegmentT&& providerSegment)
{
- if (hasProviderSegment(providerSegment.name()))
- {
- throw armem::error::ContainerEntryAlreadyExists(
- ProviderSegmentT::getLevelName(), providerSegment.name(), getLevelName(), this->getKeyString());
- }
+ return this->_derived().addProviderSegment(providerSegment.name(), std::move(providerSegment));
+ }
- auto it = this->_container.emplace(providerSegment.name(), std::move(providerSegment)).first;
- it->second.id().setCoreSegmentID(this->id());
- return it->second;
+ /// Insert a provider segment in-place.
+ template <class ...Args>
+ ProviderSegmentT& addProviderSegment(const std::string& name, Args... args)
+ {
+ ChildT& child = this->template _addChild<ChildT>(name, args...);
+ child.id() = this->id().withProviderSegmentName(name);
+ return child;
}
diff --git a/source/RobotAPI/libraries/armem/core/base/MemoryBase.h b/source/RobotAPI/libraries/armem/core/base/MemoryBase.h
index dc1ced897..e73db7d84 100644
--- a/source/RobotAPI/libraries/armem/core/base/MemoryBase.h
+++ b/source/RobotAPI/libraries/armem/core/base/MemoryBase.h
@@ -211,17 +211,28 @@ namespace armarx::armem::base
CoreSegmentT& addCoreSegment(
const std::string& name, aron::typenavigator::ObjectNavigatorPtr coreSegmentType = nullptr)
{
- return _addCoreSegment(name, name, this->id(), coreSegmentType);
+ return this->_derived().addCoreSegment(name, name, coreSegmentType);
}
+
/// Copy and insert a core segment.
CoreSegmentT& addCoreSegment(const CoreSegmentT& coreSegment)
{
- return _addCoreSegment(coreSegment.name(), CoreSegmentT(coreSegment));
+ return this->_derived().addCoreSegment(coreSegment.name(), CoreSegmentT(coreSegment));
}
+
/// Move and insert a core segment.
CoreSegmentT& addCoreSegment(CoreSegmentT&& coreSegment)
{
- return _addCoreSegment(coreSegment.name(), coreSegment);
+ return this->_derived().addCoreSegment(coreSegment.name(), std::move(coreSegment));
+ }
+
+ /// Move and insert a core segment.
+ template <class ...Args>
+ CoreSegmentT& addCoreSegment(const std::string& name, Args... args)
+ {
+ CoreSegmentT& child = this->template _addChild<ChildT>(name, args...);
+ child.id() = this->id().withCoreSegmentName(name);
+ return child;
}
@@ -317,31 +328,7 @@ namespace armarx::armem::base
}
- private:
-
- /**
- * This function allows to emplace a CoreSegment directly in the
- * container from its constructor arguments, instead of constructing
- * it outside and moving it.
- * This is necessary if CoreSegmentT is not movable.
- */
- template <class ...Args>
- CoreSegmentT& _addCoreSegment(const std::string& name, Args... args)
- {
- auto [it, inserted] = this->_container.try_emplace(name, args...);
- if (not inserted)
- {
- throw armem::error::ContainerEntryAlreadyExists(
- CoreSegmentT::getLevelName(), name, DerivedT::getLevelName(), this->name());
- }
- else
- {
- it->second.id().setMemoryID(this->id());
- it->second.id().coreSegmentName = name;
- return it->second;
- }
- }
-
+ protected:
std::pair<bool, CoreSegmentT*> _addCoreSegmentIfMissing(const std::string& coreSegmentName)
{
diff --git a/source/RobotAPI/libraries/armem/core/base/ProviderSegmentBase.h b/source/RobotAPI/libraries/armem/core/base/ProviderSegmentBase.h
index 27dae3122..ba9b091a3 100644
--- a/source/RobotAPI/libraries/armem/core/base/ProviderSegmentBase.h
+++ b/source/RobotAPI/libraries/armem/core/base/ProviderSegmentBase.h
@@ -229,7 +229,7 @@ namespace armarx::armem::base
return ret;
}
- void append(const _Derived& m)
+ void append(const DerivedT& m)
{
// ARMARX_INFO << "ProviderSegment: Merge name '" << m.name() << "' into '" << name() << "'";
@@ -245,22 +245,32 @@ namespace armarx::armem::base
});
}
+
/// Add an empty entity with the given name.
EntityT& addEntity(const std::string& name)
{
- return addEntity(EntityT(name, this->id()));
+ return this->_derived().addEntity(name, name);
}
+
/// Copy and insert an entity.
EntityT& addEntity(const EntityT& entity)
{
- return addEntity(EntityT(entity));
+ return this->_derived().addEntity(entity.name(), EntityT(entity));
}
+
/// Move and insert an entity.
EntityT& addEntity(EntityT&& entity)
{
- auto it = this->_container.emplace(entity.name(), std::move(entity)).first;
- it->second.id().setProviderSegmentID(this->id());
- return it->second;
+ return this->_derived().addEntity(entity.name(), std::move(entity));
+ }
+
+ /// Insert an entity in-place.
+ template <class ...Args>
+ EntityT& addEntity(const std::string& name, Args... args)
+ {
+ ChildT& child = this->template _addChild<ChildT>(name, args...);
+ child.id() = this->id().withEntityName(name);
+ return child;
}
diff --git a/source/RobotAPI/libraries/armem/core/base/detail/MemoryContainerBase.h b/source/RobotAPI/libraries/armem/core/base/detail/MemoryContainerBase.h
index 4d7cbec18..3e22caa31 100644
--- a/source/RobotAPI/libraries/armem/core/base/detail/MemoryContainerBase.h
+++ b/source/RobotAPI/libraries/armem/core/base/detail/MemoryContainerBase.h
@@ -108,6 +108,15 @@ namespace armarx::armem::base::detail
return _container;
}
+ DerivedT& _derived()
+ {
+ return static_cast<DerivedT&>(*this);
+ }
+ const DerivedT& _derived() const
+ {
+ return static_cast<DerivedT&>(*this);
+ }
+
/**
* @throw `armem::error::ContainerNameMismatch` if `gottenName` does not match `actualName`.
@@ -123,6 +132,19 @@ namespace armarx::armem::base::detail
}
+ template <class ChildT, class KeyT, class ...ChildArgs>
+ ChildT& _addChild(const KeyT& key, ChildArgs... childArgs)
+ {
+ auto [it, inserted] = this->_container.try_emplace(key, childArgs...);
+ if (not inserted)
+ {
+ throw armem::error::ContainerEntryAlreadyExists(
+ ChildT::getLevelName(), key, DerivedT::getLevelName(), this->_derived().name());
+ }
+ return it->second;
+ }
+
+
protected:
mutable ContainerT _container;
diff --git a/source/RobotAPI/libraries/armem/server/wm/detail/MaxHistorySize.h b/source/RobotAPI/libraries/armem/server/wm/detail/MaxHistorySize.h
index e2f2d498c..962179a68 100644
--- a/source/RobotAPI/libraries/armem/server/wm/detail/MaxHistorySize.h
+++ b/source/RobotAPI/libraries/armem/server/wm/detail/MaxHistorySize.h
@@ -1,6 +1,5 @@
#pragma once
-
namespace armarx::armem::server::detail
{
// TODO: Replace by ConstrainedHistorySize (not only max entries, e.g. delete oldest / delete least accessed / ...)
@@ -50,7 +49,6 @@ namespace armarx::armem::server::detail
static_cast<DerivedT&>(*this).forEachChild([maxSize](auto & child)
{
child.setMaxHistorySize(maxSize);
- return true;
});
}
diff --git a/source/RobotAPI/libraries/armem/server/wm/memory_definitions.h b/source/RobotAPI/libraries/armem/server/wm/memory_definitions.h
index 4eb6fdbb3..0849a9b58 100644
--- a/source/RobotAPI/libraries/armem/server/wm/memory_definitions.h
+++ b/source/RobotAPI/libraries/armem/server/wm/memory_definitions.h
@@ -26,8 +26,8 @@ namespace armarx::armem::server::wm
/// @see base::EntityBase
class Entity :
- public base::EntityBase<EntitySnapshot, Entity>,
- public detail::MaxHistorySize
+ public base::EntityBase<EntitySnapshot, Entity>
+ , public detail::MaxHistorySize
{
public:
@@ -64,7 +64,14 @@ namespace armarx::armem::server::wm
using ProviderSegmentBase::addEntity;
- EntityT& addEntity(EntityT&& entity);
+
+ template <class ...Args>
+ Entity& addEntity(const std::string& name, Args... args)
+ {
+ Entity& added = ProviderSegmentBase::addEntity(name, args...);
+ added.setMaxHistorySize(this->getMaxHistorySize());
+ return added;
+ }
};
@@ -115,7 +122,13 @@ namespace armarx::armem::server::wm
/// @see base::CoreSegmentBase::addProviderSegment()
using CoreSegmentBase::addProviderSegment;
- ProviderSegment& addProviderSegment(ProviderSegment&& providerSegment);
+ template <class ...Args>
+ ProviderSegment& addProviderSegment(const std::string& name, Args... args)
+ {
+ ProviderSegmentT& added = CoreSegmentBase::addProviderSegment(name, args...);
+ added.setMaxHistorySize(this->getMaxHistorySize());
+ return added;
+ }
// Locking interface
diff --git a/source/RobotAPI/libraries/armem/test/ArMemMemoryTest.cpp b/source/RobotAPI/libraries/armem/test/ArMemMemoryTest.cpp
index 376f2ec68..143577fc3 100644
--- a/source/RobotAPI/libraries/armem/test/ArMemMemoryTest.cpp
+++ b/source/RobotAPI/libraries/armem/test/ArMemMemoryTest.cpp
@@ -753,7 +753,7 @@ BOOST_AUTO_TEST_CASE(test_history_size_in_provider_segment)
std::vector<std::string> entityNames = { "A", "B" };
// Fill entities and histories with unlimited size.
- for (const auto& name : entityNames)
+ for (const std::string& name : entityNames)
{
update.entityID.entityName = name;
@@ -805,11 +805,12 @@ BOOST_AUTO_TEST_CASE(test_history_size_in_provider_segment)
providerSegment.setMaxHistorySize(-1);
entityNames.push_back("C");
- for (const auto& name : entityNames)
+ for (const std::string& name : entityNames)
{
update.entityID.entityName = name;
update.timeCreated = armem::Time::milliSeconds(5000);
providerSegment.update(update);
+ BOOST_CHECK_EQUAL(providerSegment.getEntity(name).getMaxHistorySize(), -1);
BOOST_CHECK_EQUAL(providerSegment.getEntity(name).size(), 3);
}
}
--
GitLab