Aggregate Entity with Talker/Listener examples #74
Comments
|
Sure let's take Talker as an example. Keep in mind that the project is split in 2 libraries:
In order to add the talker functionnalities to the project, you'll have to:
Working on la_avdeccCreate Talker related classesCreate a new file
Create Talker Capabilities classCreate a new file
Extend AggregateEntity (can optionnaly be done later)Extend the AggregateEntity class to be a talker:
Working on new la_avdecc_talkerYou can start by using a TalkerEntity instead of an AggregateEntity if you chose to delay extending the latter. Hope it will help you to get started. |
|
@christophe-calmejane Thanks a lot for you explanation! Looks pretty straightforward. I'll dive into this during the weekend and get back to you with some results and questions. |
|
@christophe-calmejane I started looking into this and have first questions.
The original code looks like
class CapabilityDelegate
{
public:
using UniquePointer = std::unique_ptr<CapabilityDelegate>;
virtual ~CapabilityDelegate() = default;
/* **** Global notifications **** */
virtual void onControllerDelegateChanged(controller::Delegate* const delegate) noexcept = 0;
//virtual void onListenerDelegateChanged(listener::Delegate* const delegate) noexcept = 0;
//virtual void onTalkerDelegateChanged(talker::Delegate* const delegate) noexcept = 0;But
/** Entity Capability delegate interface (Controller, Listener, Talker) */
class CapabilityDelegate
{
public:
/* **** Global notifications **** */
virtual void onDelegateChanged(entity::CapabilityDelegate* const delegate) noexcept = 0;
};So, every specific delegate don't have to implement unrelated features and bring unrelated dependencies. What do you think about it? |
|
oops, looks like I've messed up with |
|
@christophe-calmejane would you be so kind to tell more about the role of different delegates in this library? I'm familiar with the delegate pattern itself, just want to get some explanation from library's perspective. What should they do to bring me closer to talker implementation. |
|
As for now, I'm kinda copy-pasted all |
|
It's something I will have to look more deeply. The library started as a pure Controller implementation, and slowly I started to refactor a bit so adding Talker/Listener implementation would be easy. As I never got past pure theory, maybe there is room for improvement, and only starting to implement stuff can tell. Just keep in mind that changes to the existing base code is not advised if unecessary as the library is used by many projects and breaking it would have huge impact. Maybe I should try to implement just a single Talker or Listener method as a PoC so that other contributors know what to do more easily. That way I would have more control over what base code has to be changed (if any). |
Indeed you messed up the different Delegates, but you are right about one thing, there is no need for the base class to know about the inherited ones. |
|
@christophe-calmejane you're welcome! It is always interesting to dive into some complex c++ stuff and try to improve one. Regarding your proposal with talker or listener implementation: bringing one makes sense, especially in the lack of human-generated architectural documentation to the library itself (Or am I missing one?). I will also continue implementation by myself, but it can take much more time to come up with a clean solution than building one on top of provided example. |
|
I've a lot of UML on paper, but never had the time to use a proper tool and commit it. I hope I'll be able to complete this task in the near future. I'm often using the Doxygen generated UML when I need to check if something changed :) |
|
Trying to make a small implementation, I noticed it would be better (for the low level library at least) to handle Talker and Listeners the same way. I quickly made a PoC for the low level lib in order to handle such endpoints, as well as a new sample showing what the upper level library should do (I only implemented a few methods to showcase the concept).
If the PoC seems usable for you, I'll quickly complete the missing methods in the Delegate (won't take long as it's just a matter of search and replace) |
|
I just noticed something is not building on macOS (I mainly develop on windows). I'll fix the branch tomorrow. |
|
@christophe-calmejane Thanks for the effort! I'll check out this implementation in a couple of days |
|
@christophe-calmejane As for building error: diff --git a/examples/src/CMakeLists.txt b/examples/src/CMakeLists.txt
index 19966ab..fd1736f 100644
--- a/examples/src/CMakeLists.txt
+++ b/examples/src/CMakeLists.txt
@@ -39,7 +39,7 @@ set_target_properties(SimpleEndpoint PROPERTIES FOLDER "Examples")
target_link_libraries(SimpleEndpoint PRIVATE la_avdecc_cxx)
copy_runtime(SimpleEndpoint la_avdecc_cxx)
if(NOT WIN32)
- target_link_libraries(SimpleEndpoint PRIVATE ncurses)
+ target_link_libraries(SimpleEndpoint PRIVATE ncurses pthread)
endif()
# Setup common options
setup_executable_options(SimpleEndpoint)
diff --git a/include/la/avdecc/internals/protocolAemAecpdu.hpp b/include/la/avdecc/internals/protocolAemAecpdu.hpp
index bf54691..62caf81 100644
--- a/include/la/avdecc/internals/protocolAemAecpdu.hpp
+++ b/include/la/avdecc/internals/protocolAemAecpdu.hpp
@@ -69,7 +69,7 @@ public:
LA_AVDECC_API AemAecpdu(bool const isResponse) noexcept;
/** Destructor (for some reason we have to define it in the cpp file or clang complains about missing vtable, using = default or inline not working) */
- virtual LA_AVDECC_API ~AemAecpdu() noexcept override;
+ virtual LA_AVDECC_API ~AemAecpdu() noexcept override {};
// Setters
LA_AVDECC_API void LA_AVDECC_CALL_CONVENTION setUnsolicited(bool const unsolicited) noexcept;
diff --git a/src/endStationImpl.cpp b/src/endStationImpl.cpp
index 7d84b7d..a454c58 100644
--- a/src/endStationImpl.cpp
+++ b/src/endStationImpl.cpp
@@ -29,6 +29,7 @@
// Entities
#include "entity/controllerEntityImpl.hpp"
#include "entity/aggregateEntityImpl.hpp"
+#include "entity/endpointEntityImpl.hpp"
namespace la
{
diff --git a/src/protocol/protocolAemAecpdu.cpp b/src/protocol/protocolAemAecpdu.cpp
index 5585692..82ed6b1 100644
--- a/src/protocol/protocolAemAecpdu.cpp
+++ b/src/protocol/protocolAemAecpdu.cpp
@@ -48,7 +48,7 @@ AemAecpdu::AemAecpdu(bool const isResponse) noexcept
Aecpdu::setAecpSpecificDataLength(HeaderLength);
}
-AemAecpdu::~AemAecpdu() noexcept {}
+//AemAecpdu::~AemAecpdu() noexcept {}
void LA_AVDECC_CALL_CONVENTION AemAecpdu::setUnsolicited(bool const unsolicited) noexcept
{
@@ -154,7 +154,6 @@ void LA_AVDECC_CALL_CONVENTION AemAecpdu::deserialize(DeserializationBuffer& buf
{
_commandSpecificDataLength = _controlDataLength - minCDL;
}
-
// Check if there is more advertised data than actual bytes in the buffer (not checking earlier since we want to get as much information as possible from the packet to display a proper log message)
auto const remainingBytes = buffer.remaining();
if (_commandSpecificDataLength > remainingBytes)but still failing at linking step with Have tried both virtual destructor in .hpp and .cpp - not working yet. Not sure what is missing, will take a look at it a bit later |
|
I fixed the compilation issues, I'll integrate my fix in dev as it's something general I should have fixed a long time ago. Don't rely too much on the branch |
|
@christophe-calmejane yep, successfully built and ran |
|
@christophe-calmejane I'll also put micro utility patches here, in order not to introduce enormous amount of branches and PRs between our repos From 6a7f079a52e866522ce91c3ce7a2553441380b7c Mon Sep 17 00:00:00 2001
From: Roman Savrulin <[email protected]>
Date: Fri, 28 Feb 2020 15:42:45 +0300
Subject: [PATCH] auto select interface if only one is available
---
examples/src/utils.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/examples/src/utils.cpp b/examples/src/utils.cpp
index ba40ddf..dcb9408 100644
--- a/examples/src/utils.cpp
+++ b/examples/src/utils.cpp
@@ -158,6 +158,11 @@ la::avdecc::networkInterface::Interface chooseNetworkInterface()
return {};
}
+ if (interfaces.size() == 1)
+ {
+ return interfaces.at(0);
+ }
+
// Let the user choose an interface
outputText("Choose an interface:\n");
unsigned int intNum = 1;
--
2.25.0
second one From 97ef2ef4a7bb5c924c2c73a1f4543f18d7979eac Mon Sep 17 00:00:00 2001
From: Roman Savrulin <[email protected]>
Date: Fri, 28 Feb 2020 16:41:15 +0300
Subject: [PATCH] remove Clion-generated directories
---
.gitignore | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/.gitignore b/.gitignore
index 2602bbd..6a7b756 100644
--- a/.gitignore
+++ b/.gitignore
@@ -11,4 +11,5 @@
/externals/3rdparty/winpcap/Include
/externals/3rdparty/winpcap/Lib
-.idea
\ No newline at end of file
+.idea/
+cmake-build-debug/
\ No newline at end of file
--
2.25.0
|
|
I've seen those commits in a previous PR, and might pull one or two, although it might change a bit how I proceed with my tests. Like automatically selecting the only interface, as I usually use the fact that I only have one available to detect I have an issue with my thunderbolt adapter :) As for the second one, I don't like having every possible build folder in the ignore list. I've already added a generic |
|
@christophe-calmejane JetBrains offers to set |
|
hi, curious is anyone has completed support for configuring talker/listeners? |
|
Not to my knowledge, but it would be a nice addition. Especially now that the library has more internal features to better support entity models. |
I'm looking into writing an Entity implementation for Presonus CS18AI unit I'm working with. This is my open source initiative to do so.
I've successfully ran default library examples on the unit and ensured that the unit is enumerated in MACs AVB controller with all the zeroes when the example is running.
When I dig a little deeper into AEM entity implementation, I've discovered that Talker/Listener capability Delegates are disabled and there's no public API to set those capabilities on LocalEntity.
Can you give any info about what's missing in the library and how to make local entity
The text was updated successfully, but these errors were encountered: