From c4210d4f0aeb157cbfcb221295a74054d000b461 Mon Sep 17 00:00:00 2001
From: Patrick Dormanns <patrick.dormanns@student.kit.edu>
Date: Wed, 25 Oct 2023 17:14:31 +0200
Subject: [PATCH] implemented feedback
---
.../RobotAPI/libraries/armem/CMakeLists.txt | 2 +
.../armem/client/util/MemoryListener.cpp | 76 +------------------
.../armem/client/util/MemoryListener.h | 47 ++----------
.../armem/client/util/SubscriptionHandle.cpp | 73 ++++++++++++++++++
.../armem/client/util/SubscriptionHandle.h | 60 +++++++++++++++
5 files changed, 143 insertions(+), 115 deletions(-)
create mode 100644 source/RobotAPI/libraries/armem/client/util/SubscriptionHandle.cpp
create mode 100644 source/RobotAPI/libraries/armem/client/util/SubscriptionHandle.h
diff --git a/source/RobotAPI/libraries/armem/CMakeLists.txt b/source/RobotAPI/libraries/armem/CMakeLists.txt
index 676c1fd07..336feb574 100644
--- a/source/RobotAPI/libraries/armem/CMakeLists.txt
+++ b/source/RobotAPI/libraries/armem/CMakeLists.txt
@@ -63,6 +63,7 @@ set(LIB_FILES
client/plugins/PluginUser.cpp
client/plugins/Plugin.cpp
+ client/util/SubscriptionHandle.cpp
client/util/MemoryListener.cpp
client/util/SimpleReaderBase.cpp
client/util/SimpleWriterBase.cpp
@@ -154,6 +155,7 @@ set(LIB_HEADERS
client/query/detail/NameSelectorOps.h
client/query/detail/SelectorOps.h
+ client/util/SubscriptionHandle.h
client/util/MemoryListener.h
client/util/SimpleReaderBase.h
client/util/SimpleWriterBase.h
diff --git a/source/RobotAPI/libraries/armem/client/util/MemoryListener.cpp b/source/RobotAPI/libraries/armem/client/util/MemoryListener.cpp
index c154298e0..9aa23153c 100644
--- a/source/RobotAPI/libraries/armem/client/util/MemoryListener.cpp
+++ b/source/RobotAPI/libraries/armem/client/util/MemoryListener.cpp
@@ -13,71 +13,6 @@
namespace armarx::armem::client::util
{
-
- MemoryListener::SubscriptionHandle::SubscriptionHandle(MemoryListener* memoryListener,
- const MemoryID& memoryID,
- long id) :
- valid{true}, memoryListener{memoryListener}, memoryID(memoryID), id{id}
- {
- }
-
- MemoryListener::SubscriptionHandle::SubscriptionHandle() : valid{false}
- {
- }
-
- MemoryListener::SubscriptionHandle::SubscriptionHandle(SubscriptionHandle&& other) :
- valid{other.valid},
- memoryListener{other.memoryListener},
- memoryID(std::move(other.memoryID)),
- id{other.id}
- {
- other.valid = false;
- }
-
- MemoryListener::SubscriptionHandle&
- MemoryListener::SubscriptionHandle::operator=(SubscriptionHandle other)
- {
- swap(*this, other);
- return *this;
- }
-
- void
- swap(MemoryListener::SubscriptionHandle& first, MemoryListener::SubscriptionHandle& second)
- {
- std::swap(first.valid, second.valid);
- std::swap(first.memoryListener, second.memoryListener);
- std::swap(first.memoryID, second.memoryID);
- std::swap(first.id, second.id);
- }
-
- void
- MemoryListener::SubscriptionHandle::release()
- {
- memoryListener->unsubscribe(*this);
- }
-
- MemoryListener::ScopedSubscriptionHandle::ScopedSubscriptionHandle()
- {
- }
-
- MemoryListener::ScopedSubscriptionHandle::ScopedSubscriptionHandle(
- SubscriptionHandle&& handle) :
- handle(std::move(handle))
- {
- }
-
- MemoryListener::ScopedSubscriptionHandle&
- MemoryListener::ScopedSubscriptionHandle::operator=(MemoryListener::SubscriptionHandle handle)
- {
- std::swap(this->handle, handle);
- return *this;
- }
-
- MemoryListener::ScopedSubscriptionHandle::~ScopedSubscriptionHandle()
- {
- handle.release();
- }
-
std::string
MemoryListener::MakeMemoryTopicName(const MemoryID& memoryID)
{
@@ -177,7 +112,7 @@ namespace armarx::armem::client::util
}
}
- MemoryListener::SubscriptionHandle
+ SubscriptionHandle
MemoryListener::subscribe(const MemoryID& memoryID, Callback callback)
{
ARMARX_CHECK_NOT_EMPTY(memoryID.memoryName)
@@ -188,7 +123,7 @@ namespace armarx::armem::client::util
component->usingTopic(MakeMemoryTopicName(memoryID));
}
- auto id = next_id++;
+ auto id = nextId++;
callbacks[memoryID].push_back({id, callback});
memoryRefCount[memoryID.memoryName]++;
@@ -196,7 +131,7 @@ namespace armarx::armem::client::util
return SubscriptionHandle(this, memoryID, id);
}
- MemoryListener::SubscriptionHandle
+ SubscriptionHandle
MemoryListener::subscribe(const MemoryID& subscriptionID, CallbackUpdatedOnly callback)
{
return subscribe(
@@ -219,11 +154,6 @@ namespace armarx::armem::client::util
callbacks[handle.memoryID].end(),
[&handle](ManagedCallback& mCb) { return mCb.id == handle.id; });
- if (it->id != handle.id)
- {
- return;
- }
-
std::iter_swap(it, callbacks[handle.memoryID].end() - 1);
callbacks[handle.memoryID].pop_back();
diff --git a/source/RobotAPI/libraries/armem/client/util/MemoryListener.h b/source/RobotAPI/libraries/armem/client/util/MemoryListener.h
index f4ac2530d..80d8eb331 100644
--- a/source/RobotAPI/libraries/armem/client/util/MemoryListener.h
+++ b/source/RobotAPI/libraries/armem/client/util/MemoryListener.h
@@ -9,6 +9,8 @@
#include <RobotAPI/interface/armem/client/MemoryListenerInterface.h>
#include <RobotAPI/libraries/armem/core/MemoryID.h>
+#include "SubscriptionHandle.h"
+
namespace armarx
{
class ManagedIceObject;
@@ -16,7 +18,6 @@ namespace armarx
namespace armarx::armem::client::util
{
-
/**
* @brief Handles update signals from the memory system and distributes it
* to its subsribers.
@@ -40,44 +41,6 @@ namespace armarx::armem::client::util
static std::string MakeMemoryTopicName(const MemoryID& memoryID);
- public:
- class SubscriptionHandle
- {
- friend class MemoryListener;
-
- public:
- SubscriptionHandle();
- SubscriptionHandle(SubscriptionHandle&& other);
- SubscriptionHandle& operator=(SubscriptionHandle other);
-
- friend void swap(SubscriptionHandle& first, SubscriptionHandle& second);
-
- void release();
-
- private:
- SubscriptionHandle(MemoryListener* memoryListener, const MemoryID& memoryID, long id);
-
- private:
- bool valid = false;
- MemoryListener* memoryListener = nullptr;
- MemoryID memoryID;
- long id = 0;
- };
-
- class ScopedSubscriptionHandle
- {
- public:
- ScopedSubscriptionHandle();
- ScopedSubscriptionHandle(SubscriptionHandle&& handle);
- ScopedSubscriptionHandle& operator=(SubscriptionHandle handle);
-
- ~ScopedSubscriptionHandle();
-
- private:
- SubscriptionHandle handle;
- };
-
-
public:
MemoryListener(ManagedIceObject* component = nullptr);
@@ -93,7 +56,7 @@ namespace armarx::armem::client::util
* @endcode
*/
template <class CalleeT>
- MemoryListener::SubscriptionHandle
+ SubscriptionHandle
subscribe(const MemoryID& subscriptionID, CalleeT* callee, MemberCallback<CalleeT> callback)
{
auto cb = [callee, callback](const MemoryID& subscriptionID,
@@ -103,7 +66,7 @@ namespace armarx::armem::client::util
}
template <class CalleeT>
- MemoryListener::SubscriptionHandle
+ SubscriptionHandle
subscribe(const MemoryID& subscriptionID,
CalleeT* callee,
MemberCallbackUpdatedOnly<CalleeT> callback)
@@ -127,7 +90,7 @@ namespace armarx::armem::client::util
protected:
- long next_id = 0;
+ long nextId = 0;
struct ManagedCallback
{
diff --git a/source/RobotAPI/libraries/armem/client/util/SubscriptionHandle.cpp b/source/RobotAPI/libraries/armem/client/util/SubscriptionHandle.cpp
new file mode 100644
index 000000000..114146c91
--- /dev/null
+++ b/source/RobotAPI/libraries/armem/client/util/SubscriptionHandle.cpp
@@ -0,0 +1,73 @@
+#include "SubscriptionHandle.h"
+
+#include "MemoryListener.h"
+
+namespace armarx::armem::client::util
+{
+ SubscriptionHandle::SubscriptionHandle(MemoryListener* memoryListener,
+ const MemoryID& memoryID,
+ long id) :
+ valid{true}, memoryListener{memoryListener}, memoryID(memoryID), id{id}
+ {
+ }
+
+ SubscriptionHandle::SubscriptionHandle() : valid{false}
+ {
+ }
+
+ SubscriptionHandle::SubscriptionHandle(SubscriptionHandle&& other) :
+ valid{other.valid},
+ memoryListener{other.memoryListener},
+ memoryID(std::move(other.memoryID)),
+ id{other.id}
+ {
+ other.valid = false;
+ }
+
+ SubscriptionHandle&
+ SubscriptionHandle::operator=(SubscriptionHandle other)
+ {
+ swap(*this, other);
+ return *this;
+ }
+
+ void
+ SubscriptionHandle::release()
+ {
+ memoryListener->unsubscribe(*this);
+ }
+
+ ScopedSubscriptionHandle::ScopedSubscriptionHandle()
+ {
+ }
+
+ ScopedSubscriptionHandle::ScopedSubscriptionHandle(SubscriptionHandle&& handle) :
+ handle(std::move(handle))
+ {
+ }
+
+ ScopedSubscriptionHandle&
+ ScopedSubscriptionHandle::operator=(SubscriptionHandle handle)
+ {
+ std::swap(this->handle, handle);
+ return *this;
+ }
+
+ ScopedSubscriptionHandle::~ScopedSubscriptionHandle()
+ {
+ handle.release();
+ }
+
+} // namespace armarx::armem::client::util
+
+namespace armarx::armem::client
+{
+ void
+ util::swap(util::SubscriptionHandle& first, util::SubscriptionHandle& second)
+ {
+ std::swap(first.valid, second.valid);
+ std::swap(first.memoryListener, second.memoryListener);
+ std::swap(first.memoryID, second.memoryID);
+ std::swap(first.id, second.id);
+ }
+} // namespace armarx::armem::client
diff --git a/source/RobotAPI/libraries/armem/client/util/SubscriptionHandle.h b/source/RobotAPI/libraries/armem/client/util/SubscriptionHandle.h
new file mode 100644
index 000000000..857f01644
--- /dev/null
+++ b/source/RobotAPI/libraries/armem/client/util/SubscriptionHandle.h
@@ -0,0 +1,60 @@
+#pragma once
+
+#include <RobotAPI/libraries/armem/core/MemoryID.h>
+
+namespace armarx::armem::client::util
+{
+
+ class MemoryListener;
+
+ class SubscriptionHandle
+ {
+ friend class MemoryListener;
+
+ public:
+ SubscriptionHandle();
+ SubscriptionHandle(SubscriptionHandle&& other);
+
+ /**
+ * @brief Assignment operator.
+ *
+ * @note Intentional call by value, since this leverages the move constructor. See
+ * https://stackoverflow.com/a/11540204 (section "Move assignment operators").
+ */
+ SubscriptionHandle& operator=(SubscriptionHandle other);
+
+ friend void swap(SubscriptionHandle& first, SubscriptionHandle& second);
+
+ void release();
+
+ private:
+ SubscriptionHandle(MemoryListener* memoryListener, const MemoryID& memoryID, long id);
+
+ private:
+ bool valid = false;
+ MemoryListener* memoryListener = nullptr;
+ MemoryID memoryID;
+ long id = 0;
+ };
+
+ class ScopedSubscriptionHandle
+ {
+ public:
+ ScopedSubscriptionHandle();
+ ScopedSubscriptionHandle(SubscriptionHandle&& handle);
+
+ /**
+ * @brief Assignment operator.
+ *
+ * @note Intentional call by value, since this leverages the move constructor. See
+ * https://stackoverflow.com/a/11540204 (section "Move assignment operators").
+ */
+ ScopedSubscriptionHandle& operator=(SubscriptionHandle handle);
+
+ ~ScopedSubscriptionHandle();
+
+ private:
+ SubscriptionHandle handle;
+ };
+
+} // namespace armarx::armem::client::util
--
GitLab