-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add meta proto service for service discovery #543
Draft
oguzcanoguz
wants to merge
40
commits into
main
Choose a base branch
from
feature/proto_service_discovery
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
4c3eb9f
Add meta proto service for service discovery
oguzcanoguz cdc8fb3
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz adb0092
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz f15b249
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz d5d9dc2
Update protobuf/meta_services/test/TestServiceDiscoveryEcho.cpp
oguzcanoguz cf77db0
Update protobuf/meta_services/test/TestServiceDiscoveryEcho.cpp
oguzcanoguz 9622fe2
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz b6cc192
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz bbc7075
Update protobuf/meta_services/test/TestServiceDiscoveryEcho.cpp
oguzcanoguz b9239e9
Apply suggestions from code review
oguzcanoguz 2850b82
Add service changed notification to ServviceDiscovery
oguzcanoguz 151e119
Merge remote-tracking branch 'origin/main' into feature/proto_service…
oguzcanoguz e8ca6d6
ServiceDiscoveryEcho refactor header
oguzcanoguz 2dadfb7
Implement StartMethod and NotifyServiceChanges in ServiceDiscoveryEcho
oguzcanoguz 2e575b3
Refactor ServiceDiscoveryTest
oguzcanoguz 0db7aa5
Fix function static storage in ServiceDiscoveryEcho
oguzcanoguz ca8bdaa
Solve functio storage problem in ServiceDiscoveryTest
oguzcanoguz 914754e
Make EchoConsole accept recursive directories for protobuf files
oguzcanoguz a2dd642
Start with PeerServiceDiscoverer
oguzcanoguz f0fc274
PeerServiceDiscovererTest: Add iterative service discovery
oguzcanoguz a8a24b8
PeerServiceDiscovererTest contd
oguzcanoguz df27790
Merge commit 'e9afc63890f512f66b28fe2b409de5b9fe471934'
oguzcanoguz 0068cc6
Start with providing id range in NotifyServicesChanged
oguzcanoguz 3685020
Add notifying service changes with a range
oguzcanoguz 5db6745
Make service changes amnesiac
oguzcanoguz d5519bd
remove ServiceDiscoveryStarted from PeerServiceDiscovery
oguzcanoguz 067ec70
Refactor PeerServiceDiscovererObserver
oguzcanoguz 9118a61
PeerServiceDiscoverer: Start service discovery on construction
oguzcanoguz abfa695
PeerServiceDiscoverer: Handle service updated notification
oguzcanoguz e2c95a7
PeerServiceDiscovererTest: WIP Streamlining services changed tests
oguzcanoguz 5a0a2be
Refactor PeerServiceDiscovery tests
oguzcanoguz a89a492
PeerServiceDiscovery: Discovery can end with a service or no service
oguzcanoguz 6543e14
Start with a test echo server that can be run in devcontainer
oguzcanoguz a30b880
fix cmakelists
oguzcanoguz a471b1d
Try to instantiate PeerServiceDiscovery in echo console
oguzcanoguz 62a871f
Fix: Use echo only when the connection observer is attached to a conn…
oguzcanoguz 50d9170
ServiceDiscoveryEcho: Optimize FirstServiceSupported
oguzcanoguz 138fc45
Merge branch 'temp' into feature/proto_service_discovery
oguzcanoguz 161fcb6
trace discovered services
oguzcanoguz f65605a
Start working on attaching EchoConsole to Echo as a Service
oguzcanoguz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
add_library(protobuf.meta_services ${EMIL_EXCLUDE_FROM_ALL} STATIC) | ||
|
||
target_include_directories(protobuf.meta_services PUBLIC | ||
"$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/../..>" | ||
"$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>" | ||
) | ||
|
||
protocol_buffer_csharp(protobuf.meta_services ServiceDiscovery.proto) | ||
protocol_buffer_java(protobuf.meta_services ServiceDiscovery.proto) | ||
protocol_buffer_echo_all(protobuf.meta_services ServiceDiscovery.proto) | ||
|
||
target_sources(protobuf.meta_services PRIVATE | ||
PeerServiceDiscoverer.cpp | ||
PeerServiceDiscoverer.hpp | ||
ServiceDiscoveryEcho.cpp | ||
ServiceDiscoveryEcho.hpp | ||
) | ||
|
||
add_subdirectory(test) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
#include "protobuf/meta_services/PeerServiceDiscoverer.hpp" | ||
#include "infra/event/EventDispatcher.hpp" | ||
#include "infra/util/Function.hpp" | ||
#include <cstdint> | ||
#include <limits> | ||
|
||
namespace application | ||
{ | ||
PeerServiceDiscovererEcho::PeerServiceDiscovererEcho(services::Echo& echo) | ||
: service_discovery::ServiceDiscoveryProxy(echo) | ||
, service_discovery::ServiceDiscoveryResponse(echo) | ||
{ | ||
DiscoverPeerServices(); | ||
} | ||
|
||
void PeerServiceDiscovererEcho::NoServiceSupported() | ||
{ | ||
NotifyObservers([this](auto& observer) | ||
{ | ||
observer.ServicesDiscovered(services.range()); | ||
}); | ||
|
||
MethodDone(); | ||
} | ||
|
||
void PeerServiceDiscovererEcho::FirstServiceSupported(uint32_t id) | ||
{ | ||
services.push_back(id); | ||
|
||
if (id == SearchRangeEnd()) | ||
NotifyObservers([this](auto& observer) | ||
{ | ||
observer.ServicesDiscovered(services.range()); | ||
}); | ||
else | ||
RequestSend([this] | ||
{ | ||
FindFirstServiceInRange(services.back() + 1, SearchRangeEnd()); | ||
}); | ||
|
||
MethodDone(); | ||
} | ||
|
||
void PeerServiceDiscovererEcho::ServicesChanged(uint32_t startServiceId, uint32_t endServiceId) | ||
{ | ||
searchRange = ServiceRange{ startServiceId, endServiceId }; | ||
|
||
StartDiscovery(); | ||
|
||
MethodDone(); | ||
} | ||
|
||
void PeerServiceDiscovererEcho::DiscoverPeerServices() | ||
{ | ||
searchRange = serviceRangeMax; | ||
|
||
RequestSend([this] | ||
{ | ||
NotifyServiceChanges(true); | ||
StartDiscovery(); | ||
}); | ||
} | ||
|
||
void PeerServiceDiscovererEcho::ClearUpdatedServices() | ||
{ | ||
services.erase(std::remove_if(services.begin(), services.end(), [this](uint32_t serviceId) | ||
{ | ||
return serviceId >= SearchRangeBegin() && serviceId <= SearchRangeEnd(); | ||
}), | ||
services.end()); | ||
} | ||
|
||
void PeerServiceDiscovererEcho::StartDiscovery() | ||
{ | ||
ClearUpdatedServices(); | ||
|
||
RequestSend([this] | ||
{ | ||
FindFirstServiceInRange(SearchRangeBegin(), SearchRangeEnd()); | ||
}); | ||
} | ||
|
||
uint32_t PeerServiceDiscovererEcho::SearchRangeBegin() const | ||
{ | ||
return std::get<0>(searchRange); | ||
} | ||
|
||
uint32_t PeerServiceDiscovererEcho::SearchRangeEnd() const | ||
{ | ||
return std::get<1>(searchRange); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,55 @@ | ||||||||
#ifndef PROTOBUF_ECHO_PEER_SERVICE_DISCOVERER_HPP | ||||||||
#define PROTOBUF_ECHO_PEER_SERVICE_DISCOVERER_HPP | ||||||||
|
||||||||
#include "generated/echo/ServiceDiscovery.pb.hpp" | ||||||||
#include "infra/syntax/CppFormatter.hpp" | ||||||||
#include "infra/util/BoundedVector.hpp" | ||||||||
#include "infra/util/MemoryRange.hpp" | ||||||||
#include "infra/util/Observer.hpp" | ||||||||
#include "protobuf/echo/Echo.hpp" | ||||||||
#include <cstdint> | ||||||||
#include <tuple> | ||||||||
|
||||||||
namespace application | ||||||||
{ | ||||||||
class PeerServiceDiscovererEcho; | ||||||||
|
||||||||
class PeerServiceDiscoveryObserver | ||||||||
: public infra::SingleObserver<PeerServiceDiscoveryObserver, PeerServiceDiscovererEcho> | ||||||||
{ | ||||||||
public: | ||||||||
using infra::SingleObserver<PeerServiceDiscoveryObserver, PeerServiceDiscovererEcho>::SingleObserver; | ||||||||
|
||||||||
virtual void ServicesDiscovered(infra::MemoryRange<uint32_t> services) = 0; | ||||||||
}; | ||||||||
|
||||||||
class PeerServiceDiscovererEcho | ||||||||
: public service_discovery::ServiceDiscoveryProxy | ||||||||
, public service_discovery::ServiceDiscoveryResponse | ||||||||
, public infra::Subject<PeerServiceDiscoveryObserver> | ||||||||
{ | ||||||||
public: | ||||||||
explicit PeerServiceDiscovererEcho(services::Echo& echo); | ||||||||
|
||||||||
void NoServiceSupported() override; | ||||||||
void FirstServiceSupported(uint32_t id) override; | ||||||||
void ServicesChanged(uint32_t startServiceId, uint32_t endServiceId) override; | ||||||||
|
||||||||
private: | ||||||||
using ServiceRange = std::tuple<uint32_t, uint32_t>; | ||||||||
static constexpr ServiceRange serviceRangeMax = std::make_tuple(0, std::numeric_limits<uint32_t>::max()); | ||||||||
|
||||||||
private: | ||||||||
void DiscoverPeerServices(); | ||||||||
void StartDiscovery(); | ||||||||
void ClearUpdatedServices(); | ||||||||
uint32_t SearchRangeBegin() const; | ||||||||
uint32_t SearchRangeEnd() const; | ||||||||
|
||||||||
private: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MegaLinter] reported by reviewdog 🐶
Suggested change
|
||||||||
infra::BoundedVector<uint32_t>::WithMaxSize<10> services; | ||||||||
ServiceRange searchRange; | ||||||||
}; | ||||||||
} | ||||||||
|
||||||||
#endif |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
syntax = "proto3"; | ||
|
||
import "EchoAttributes.proto"; | ||
|
||
package service_discovery; | ||
option java_package = "com.philips.emil.protobufEcho"; | ||
option java_outer_classname = "ServiceDiscoveryProto"; | ||
|
||
message Uint32Value { | ||
uint32 value = 1; | ||
} | ||
|
||
message BoolValue | ||
{ | ||
bool value = 1; | ||
} | ||
|
||
message ServiceRange | ||
{ | ||
uint32 startServiceId = 1; | ||
uint32 endServiceId = 2; | ||
} | ||
|
||
service ServiceDiscovery | ||
{ | ||
option (service_id) = 1000; | ||
|
||
rpc FindFirstServiceInRange(ServiceRange) returns (Nothing) { option (method_id) = 1; } | ||
rpc NotifyServiceChanges(BoolValue) returns (Nothing) { option (method_id) = 2; } | ||
} | ||
|
||
service ServiceDiscoveryResponse | ||
{ | ||
option (service_id) = 1001; | ||
|
||
rpc FirstServiceSupported(Uint32Value) returns (Nothing) { option (method_id) = 1; } | ||
rpc NoServiceSupported(Nothing) returns (Nothing) { option (method_id) = 2; } | ||
rpc ServicesChanged(ServiceRange) returns (Nothing) { option (method_id) = 3; } | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MegaLinter] reported by reviewdog 🐶