Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Iox #415 Make ServiceRegistry aware of complete ServiceDescription #860

Merged
merged 33 commits into from
Aug 30, 2021
Merged

Iox #415 Make ServiceRegistry aware of complete ServiceDescription #860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
a51eeff
iox-#415 Draft two attempts to access service registry database effic…
mossmaurice Jun 30, 2021
88fea8f
iox-#415 Implement first version of new ServiceRegistry
mossmaurice Jun 30, 2021
02071d4
iox-#415 Remove obsolete comments
mossmaurice Jun 18, 2021
973b166
iox-#415 Send real ServiceDescription over IPC channel and write down…
mossmaurice Jun 30, 2021
e20c118
iox-#415 Fix RoudiFindService_test.StopOfferMultiMethodServiceSingleI…
mossmaurice Jun 21, 2021
5105289
iox-#415 Fix RoudiFindService_test.InterfacePort
mossmaurice Jun 30, 2021
ea0ed42
iox-#415 Add asserts to RoudiFindService_test
mossmaurice Jun 30, 2021
fb20956
iox-#415 Add copyright header and doxygen
mossmaurice Jun 30, 2021
b7c3f97
iox-#415 Update some comments
mossmaurice Jun 30, 2021
5e0f5b0
iox-#415 Write down some more ServiceRegistry test cases
mossmaurice Jun 30, 2021
8e24029
iox-#415 Add noexcept to ServiceRegistry and rename ServiceRegistry::…
mossmaurice Jun 30, 2021
486a008
iox-#415 Add todo for new discovery API and remove obsolete forward d…
mossmaurice Jun 28, 2021
d2c0aaf
iox-#415 Add new error in case adding entry to ServiceRegistry fails
mossmaurice Jun 30, 2021
cef63b5
iox-#415 Fix PoshRuntime and FindService tests
mossmaurice Jun 30, 2021
2fec024
iox-#415 Implement out-commented tests for ServiceRegistry
mossmaurice Jul 1, 2021
479ea0b
iox-#415 Add validity check to ServiceRegistry::{add,remove}
mossmaurice Jul 1, 2021
10b3658
iox-#415 Rename Any_t to Wildcard_t
mossmaurice Jul 1, 2021
fcb415a
iox-#415 Simplify ServiceRegistry::remove
mossmaurice Jul 1, 2021
4ffa025
iox-#415 Move else case in ServiceRegistry::find one level up
mossmaurice Jul 1, 2021
7cc5ee8
iox-#415 Rename instanceContainer to serviceContainer in findService …
mossmaurice Jul 1, 2021
6493ba5
iox-#415 Reorder ServiceRegistry test cases
mossmaurice Jul 1, 2021
cb08485
iox-#415 Add preliminary changelog entry about new DiscoveryInfo API
mossmaurice Jul 1, 2021
e2ed8c3
iox-#415 Fix bug in ServiceRegistry::remove
mossmaurice Jul 2, 2021
b7e7e9c
iox-#415 Update example in changelog
mossmaurice Jul 30, 2021
93d5a41
iox-#415 Revert changes in ServiceDescription::operator==() and allow…
mossmaurice Aug 3, 2021
27271df
iox-#415 Remove Error::SERVICE_DESCRIPTION_ALREADY_ADDED
mossmaurice Aug 3, 2021
43a4f7f
iox-#415 Create temp variable serviceIndex
mossmaurice Aug 3, 2021
f862b44
iox-#415 Take the reference counter into account when removing a Serv…
mossmaurice Aug 4, 2021
58dfb50
Merge remote-tracking branch 'origin/master' into iox-#415-make-servi…
mossmaurice Aug 4, 2021
f8b3ff6
iox-#415 Use ServiceDescriptionEntry instead of cxx::pair
mossmaurice Aug 9, 2021
41b8294
iox-#415 Fix some typos
mossmaurice Aug 9, 2021
bb8a348
iox-#415 Fix U's and ServiceRegistry::remove
mossmaurice Aug 9, 2021
78d5952
iox-#415 Fix code formatting
mossmaurice Aug 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,20 @@ ServiceDescription("First", "Second", "DontCare") myServiceDescription2;
ServiceDescription("Foo", "Bar", "Baz") myServiceDescription3;
```

The service-related methods have been moved from `PoshRuntime` to a separate class (TBD):

```cpp
// before
poshRuntime.offerService(myServiceDescription);
poshRuntime.stopOfferService(myServiceDescription);
poshRuntime.findService({"ServiceA", iox::capro::AnyInstanceString});

// after
discoveryInfo.offerService(myServiceDescription);
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
discoveryInfo.stopOfferService(myServiceDescription);
discoveryInfo.findService("ServiceA", Wildcard);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that we now call findService, provide a service ID and what will be returned is a container with full CaProServiceDescription? The Service IDs in this Service Description would be some kind of redundant information as hopefully you only get CaPro IDs that match the requested service ID

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. Service IDs in this case would all be "ServiceA". IMHO this is beneficial to the old design as the user will now be able to read all the other members of a ServiceDescription. This could in future be also e.g. QoS settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be precise: The return value is a vector wrapped in an expected cxx::expected<ServiceContainer, FindServiceError> and using ServiceContainer = iox::cxx::vector<capro::ServiceDescription, MAX_NUMBER_OF_SERVICES>;

```

## [v1.0.1](https://github.com/eclipse-iceoryx/iceoryx/tree/v1.0.0) (2021-06-15)

[Full Changelog](https://github.com/eclipse-iceoryx/iceoryx/compare/v1.0.0...v1.0.1)
Expand Down
1 change: 1 addition & 0 deletions iceoryx_hoofs/include/iceoryx_hoofs/error_handling/error_handling.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ namespace iox
error(POSH__RUNTIME_NAME_EMPTY) \
error(POSH__RUNTIME_LEADING_SLASH_PROVIDED) \
error(POSH__PORT_MANAGER_PUBLISHERPORT_NOT_UNIQUE) \
error(POSH__PORT_MANAGER_COULD_NOT_ADD_SERVICE_TO_REGISTRY) \
error(POSH__MEMPOOL_POSSIBLE_DOUBLE_FREE) \
error(POSH__RECEIVERPORT_DELIVERYFIFO_OVERFLOW) \
error(POSH__SENDERPORT_SAMPLE_SIZE_CHANGED_FOR_ACTIVE_PORT) \
Expand Down
11 changes: 8 additions & 3 deletions iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ class PublisherPortUser;
class SubscriberPortRouDi;
class SubscriberPortUser;
} // namespace popo
namespace capro
{
class ServiceDescription;
}

using PublisherPortRouDiType = iox::popo::PublisherPortRouDi;
using PublisherPortUserType = iox::popo::PublisherPortUser;
Expand Down Expand Up @@ -131,10 +135,11 @@ constexpr uint32_t APP_MESSAGE_SIZE = 512U;

// Processes
constexpr uint32_t MAX_PROCESS_NUMBER = 300U;
/// Maximum number of instances of a given service, which can be found.
/// Maximum number of services, which can be found.
/// This limitation is coming due to the fixed capacity of the cxx::vector (This doesn't limit the offered number of
/// instances)
constexpr uint32_t MAX_NUMBER_OF_INSTANCES = 50U;
/// @todo #415 increase number back to 50 once service registry is available via shared memory
constexpr uint32_t MAX_NUMBER_OF_SERVICES = 10U;

// Nodes
constexpr uint32_t MAX_NODE_NUMBER = 1000U;
Expand Down Expand Up @@ -249,7 +254,7 @@ using TimePointNs_t = std::chrono::time_point<BaseClock_t, DurationNs_t>;

namespace runtime
{
using InstanceContainer = iox::cxx::vector<capro::IdString_t, MAX_NUMBER_OF_INSTANCES>;
using ServiceContainer = iox::cxx::vector<capro::ServiceDescription, MAX_NUMBER_OF_SERVICES>;
using namespace units::duration_literals;
constexpr units::Duration PROCESS_WAITING_FOR_ROUDI_TIMEOUT = 60_s;
constexpr units::Duration PROCESS_KEEP_ALIVE_INTERVAL = 3 * roudi::DISCOVERY_INTERVAL; // > DISCOVERY_INTERVAL
Expand Down
4 changes: 2 additions & 2 deletions iceoryx_posh/include/iceoryx_posh/internal/roudi/port_manager.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ class PortManager

void sendToAllMatchingInterfacePorts(const capro::CaproMessage& message) noexcept;

void addEntryToServiceRegistry(const capro::IdString_t& service, const capro::IdString_t& instance) noexcept;
void removeEntryFromServiceRegistry(const capro::IdString_t& service, const capro::IdString_t& instance) noexcept;
void addEntryToServiceRegistry(const capro::ServiceDescription& service) noexcept;
void removeEntryFromServiceRegistry(const capro::ServiceDescription& service) noexcept;

template <typename T, std::enable_if_t<std::is_same<T, iox::build::OneToManyPolicy>::value>* = nullptr>
cxx::optional<RuntimeName_t>
Expand Down
54 changes: 41 additions & 13 deletions iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@
#ifndef IOX_POSH_ROUDI_SERVICE_REGISTRY_HPP
#define IOX_POSH_ROUDI_SERVICE_REGISTRY_HPP

#include "iceoryx_hoofs/cxx/expected.hpp"
#include "iceoryx_hoofs/cxx/vector.hpp"
#include "iceoryx_hoofs/internal/cxx/set.hpp"
#include "iceoryx_posh/capro/service_description.hpp"

#include <cstdint>
#include <map>
#include <utility>

namespace iox
{
Expand All @@ -32,23 +33,50 @@ static const capro::IdString_t Wildcard{"*"};
class ServiceRegistry
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
{
public:
static constexpr uint32_t MAX_INSTANCES_PER_SERVICE = 100u;
using InstanceSet_t = cxx::vector<capro::IdString_t, MAX_INSTANCES_PER_SERVICE>;
struct instance_t
enum class Error
{
InstanceSet_t instanceSet;
INVALID_STATE,
SERVICE_REGISTRY_FULL,
};
using serviceMap_t = std::map<capro::IdString_t, instance_t>;

void add(const capro::IdString_t& service, const capro::IdString_t& instance);
void remove(const capro::IdString_t& service, const capro::IdString_t& instance);
void find(InstanceSet_t& instances,
const capro::IdString_t& service,
const capro::IdString_t& instance = Wildcard) const;
const serviceMap_t& getServiceMap() const;
using ReferenceCounter_t = uint64_t;
struct ServiceDescriptionEntry
{
capro::ServiceDescription serviceDescription{};
ReferenceCounter_t referenceCounter = 0U;
};

/// @todo #415 should be connected with iox::MAX_NUMBER_OF_SERVICES
static constexpr uint32_t MAX_SERVICE_DESCRIPTIONS = 100U;
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
using ServiceDescriptionVector_t = cxx::vector<ServiceDescriptionEntry, MAX_SERVICE_DESCRIPTIONS>;

/// @brief Adds given service description to registry
/// @param[in] serviceDescription, service to be added
/// @return ServiceRegistryError, error wrapped in cxx::expected
cxx::expected<Error> add(const capro::ServiceDescription& serviceDescription) noexcept;

/// @brief Removes given service description from registry
/// @param[in] serviceDescription, service to be removed
/// @return true, if service description was removed, false otherwise
bool remove(const capro::ServiceDescription& serviceDescription) noexcept;

/// @brief Removes given service description from registry
/// @param[in] searchResult, reference to the vector which will be filled with the results
/// @param[in] service, string or wildcard to search for
/// @param[in] instance, string or wildcard to search for
void find(ServiceDescriptionVector_t& searchResult,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API feels like the old C APIs and it is unnecessary to add the searchResult as a reference to find in here.
Please adjust it to

ServiceDescriptionVector_t find(
const CaproIdString_t& service = capro::AnyServiceString, const CaproIdString_t instinstanceance = capro::AnyInstanceString, const CaproIdString_t & event) const;

This can be used much easier, see

auto result = registry.find("bla", "blubb", "fuu");
// instead of
ServiceRegistry::ServiceDescriptionVector_t result;
registry.find(result, "bla", "blubb", "fuu");

Copy link
Contributor

@MatthiasKillat MatthiasKillat Jun 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be easier more natural to use like this, but consider the case where we can store 1000 services or more.

There is a reason to have an API like this with our memory constraints, and this is that the return type would always need to support maximum number of elements in the returned container. We have no truly dynamic containers and cannot easily (or at all) define those. It is possible that the user may define the return type as a template parameter, but this is also bad to use for return values (cannot be inferred, needs to be specified explicitly).

Therefore the more natural API would be less efficient memory wise. Runtime wise it would likely be ok, if the unused container elements are not actually initialized with anything (nor memory set to 0).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @MatthiasKillat in general I agree with you @elfenpiff a auto result = registry.find("bla", "blubb", "fuu") is much nicer. A typical ADAS or robotics system will for sure have something around 1000 services. We would not let all the services be findable, but let's assume a size of 200. That's a already a huge number and it will have some runtime effect as we would return lots of cxx::string's via copy here. I'd propose to do a measurement once all building blocks are in place for the new discovery API. This can easily be changed as it is no user-facing API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do this later. There are all kinds of trade-off. Returning pointers vs. values, creating the container vs. letting the user pass it to be filled, defining the container type vs. leaving it generic as a template parameter etc.

One could even use a functional approach, letting the user pass a function which specifies what to do with the found service.

Hard to tell what is the best approach here, depends on what is the priority: generality, usability, efficiency...?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the approach with the lambda. That's quite modern and gives the user maximum freedom

const capro::IdString_t& service = Wildcard,
const capro::IdString_t& instance = Wildcard) const noexcept;

/// @brief Returns all service descriptions as copy
/// @return ServiceDescriptionVector_t, copy of complete service registry
const ServiceDescriptionVector_t getServices() const noexcept;

private:
mutable serviceMap_t m_serviceMap;
/// @todo #859 replace std::multimap with prefix tree
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this does not work out for whatever reason an unordered multimap based on hashing is also suitable here.

The yet to be disclosed implementation of the tree is fine though, the only problem is the worst case space and that is not easy to fix. We will see when we replace it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that with static memory space is a problem almost regardless of data structures, just for some it is more problematic.

::std::multimap<capro::IdString_t, uint64_t> m_serviceMap;
::std::multimap<capro::IdString_t, uint64_t> m_instanceMap;
ServiceDescriptionVector_t m_serviceDescriptionVector;
};
} // namespace roudi
} // namespace iox
Expand Down
6 changes: 3 additions & 3 deletions iceoryx_posh/include/iceoryx_posh/internal/runtime/posh_runtime_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ class PoshRuntimeImpl : public PoshRuntime
virtual ~PoshRuntimeImpl() noexcept;

/// @copydoc PoshRuntime::findService
cxx::expected<InstanceContainer, FindServiceError>
findService(const cxx::variant<Any_t, capro::IdString_t> service,
const cxx::variant<Any_t, capro::IdString_t> instance) noexcept override;
cxx::expected<ServiceContainer, FindServiceError>
findService(const cxx::variant<Wildcard_t, capro::IdString_t> service,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we had it as a type this would be a special case for the either monad. (Also cxx::expected is a special case as well, where the types have specific meaning).

The reason I state this is that I think that the variant has more overhead for tracking its actual type than actually needed in a purely binary choice between types. Just optimization of course, but the building block is worth thinking about some time.

const cxx::variant<Wildcard_t, capro::IdString_t> instance) noexcept override;

/// @copydoc PoshRuntime::offerService
bool offerService(const capro::ServiceDescription& serviceDescription) noexcept override;
Expand Down
18 changes: 9 additions & 9 deletions iceoryx_posh/include/iceoryx_posh/runtime/posh_runtime.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ class NodeData;
enum class FindServiceError
{
INVALID_STATE,
UNABLE_TO_WRITE_TO_ROUDI_CHANNEL,
INSTANCE_CONTAINER_OVERFLOW
UNABLE_TO_WRITE_TO_ROUDI_CHANNEL, /// @todo #415 remove as IPC channel won't be used
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
INSTANCE_CONTAINER_OVERFLOW /// @todo #415 set container to iox::MAX_NUMBER_OF_SERVICES and remove error
};

/// @brief Used to search for any string (wildcard)
struct Any_t
/// @brief Used to search for any string
struct Wildcard_t
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
{
};

Expand Down Expand Up @@ -89,12 +89,12 @@ class PoshRuntime
/// @brief find all services that match the provided service description
/// @param[in] service service string to search for (wildcards allowed)
/// @param[in] instance instance string to search for (wildcards allowed)
/// @return cxx::expected<InstanceContainer, FindServiceError>
/// InstanceContainer: on success, container that is filled with all matching instances
/// @return cxx::expected<ServiceContainer, FindServiceError>
/// ServiceContainer: on success, container that is filled with all matching instances
/// FindServiceError: if any, encountered during the operation
virtual cxx::expected<InstanceContainer, FindServiceError>
findService(const cxx::variant<Any_t, capro::IdString_t> service,
const cxx::variant<Any_t, capro::IdString_t> instance) noexcept = 0;
virtual cxx::expected<ServiceContainer, FindServiceError>
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
findService(const cxx::variant<Wildcard_t, capro::IdString_t> service,
const cxx::variant<Wildcard_t, capro::IdString_t> instance) noexcept = 0;

/// @brief offer the provided service, sends the offer from application to RouDi daemon
/// @param[in] service valid ServiceDescription to offer
Expand Down
5 changes: 0 additions & 5 deletions iceoryx_posh/source/capro/service_description.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,6 @@ ServiceDescription::ServiceDescription(const IdString_t& service,

bool ServiceDescription::operator==(const ServiceDescription& rhs) const
{
if (!isValid() || !rhs.isValid())
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
{
return false;
}

if (m_serviceString != rhs.m_serviceString)
{
return false;
Expand Down
59 changes: 26 additions & 33 deletions iceoryx_posh/source/roudi/port_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,11 @@ void PortManager::doDiscoveryForPublisherPort(PublisherPortRouDiType& publisherP
m_portIntrospection.reportMessage(caproMessage);
if (capro::CaproMessageType::OFFER == caproMessage.m_type)
{
this->addEntryToServiceRegistry(caproMessage.m_serviceDescription.getServiceIDString(),
caproMessage.m_serviceDescription.getInstanceIDString());
this->addEntryToServiceRegistry(caproMessage.m_serviceDescription);
}
else if (capro::CaproMessageType::STOP_OFFER == caproMessage.m_type)
{
this->removeEntryFromServiceRegistry(caproMessage.m_serviceDescription.getServiceIDString(),
caproMessage.m_serviceDescription.getInstanceIDString());
this->removeEntryFromServiceRegistry(caproMessage.m_serviceDescription);
}
else
{
Expand Down Expand Up @@ -268,21 +266,19 @@ void PortManager::handleInterfaces() noexcept
}
}
// also forward services from service registry
auto serviceMap = m_serviceRegistry.getServiceMap();
/// @todo #415 do we still need this? yes but return a copy here to be stored in shared memory via new
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
/// StatusPort's
auto serviceVector = m_serviceRegistry.getServices();

caproMessage.m_subType = capro::CaproMessageSubType::SERVICE;

for (auto const& x : serviceMap)
for (auto const& element : serviceVector)
{
for (auto& instance : x.second.instanceSet)
{
caproMessage.m_serviceDescription = capro::ServiceDescription(x.first, instance, roudi::Wildcard);
caproMessage.m_serviceDescription = element.serviceDescription;

for (auto& interfacePortData : interfacePortsForInitialForwarding)
{
auto interfacePort = popo::InterfacePort(interfacePortData);
interfacePort.dispatchCaProMessage(caproMessage);
}
for (auto& interfacePortData : interfacePortsForInitialForwarding)
{
popo::InterfacePort(interfacePortData).dispatchCaProMessage(caproMessage);
}
}
}
Expand All @@ -306,14 +302,12 @@ void PortManager::handleApplications() noexcept
{
case capro::CaproMessageType::OFFER:
{
addEntryToServiceRegistry(serviceDescription.getServiceIDString(),
serviceDescription.getInstanceIDString());
addEntryToServiceRegistry(caproMessage.m_serviceDescription);
break;
}
case capro::CaproMessageType::STOP_OFFER:
{
removeEntryFromServiceRegistry(serviceDescription.getServiceIDString(),
serviceDescription.getInstanceIDString());
removeEntryFromServiceRegistry(caproMessage.m_serviceDescription);
break;
}
default:
Expand Down Expand Up @@ -550,8 +544,7 @@ void PortManager::destroyPublisherPort(PublisherPortRouDiType::MemberType_t* con
cxx::Ensures(caproMessage.m_type == capro::CaproMessageType::STOP_OFFER);

m_portIntrospection.reportMessage(caproMessage);
this->removeEntryFromServiceRegistry(caproMessage.m_serviceDescription.getServiceIDString(),
caproMessage.m_serviceDescription.getInstanceIDString());
this->removeEntryFromServiceRegistry(caproMessage.m_serviceDescription);
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
this->sendToAllMatchingSubscriberPorts(caproMessage, publisherPortRoudi);
this->sendToAllMatchingInterfacePorts(caproMessage);
});
Expand Down Expand Up @@ -600,17 +593,16 @@ runtime::IpcMessage PortManager::findService(const capro::IdString_t& service,
interfacePort.dispatchCaProMessage(caproMessage);
}

// add all found instances to instanceString
runtime::IpcMessage instanceMessage;
runtime::IpcMessage response;

ServiceRegistry::InstanceSet_t instances;
m_serviceRegistry.find(instances, service, instance);
for (auto& instance : instances)
ServiceRegistry::ServiceDescriptionVector_t searchResult;
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
m_serviceRegistry.find(searchResult, service, instance);
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
for (auto& service : searchResult)
{
instanceMessage << instance;
response << static_cast<cxx::Serialization>(service.serviceDescription).toString();
}

return instanceMessage;
return response;
}

const std::atomic<uint64_t>* PortManager::serviceRegistryChangeCounter() noexcept
Expand Down Expand Up @@ -721,17 +713,18 @@ popo::ApplicationPortData* PortManager::acquireApplicationPortData(const Runtime
}
}

void PortManager::addEntryToServiceRegistry(const capro::IdString_t& service,
const capro::IdString_t& instance) noexcept
void PortManager::addEntryToServiceRegistry(const capro::ServiceDescription& service) noexcept
{
m_serviceRegistry.add(service, instance);
m_serviceRegistry.add(service).or_else([&](auto&) {
LogWarn() << "Could not add service " << service.getServiceIDString() << " to service registry!";
errorHandler(Error::kPOSH__PORT_MANAGER_COULD_NOT_ADD_SERVICE_TO_REGISTRY, nullptr, ErrorLevel::MODERATE);
});
m_portPool->serviceRegistryChangeCounter()->fetch_add(1, std::memory_order_relaxed);
}

void PortManager::removeEntryFromServiceRegistry(const capro::IdString_t& service,
const capro::IdString_t& instance) noexcept
void PortManager::removeEntryFromServiceRegistry(const capro::ServiceDescription& service) noexcept
{
m_serviceRegistry.remove(service, instance);
m_serviceRegistry.remove(service);
m_portPool->serviceRegistryChangeCounter()->fetch_add(1, std::memory_order_relaxed);
}

Expand Down
7 changes: 3 additions & 4 deletions iceoryx_posh/source/roudi/process_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,10 @@ void ProcessManager::findServiceForProcess(const RuntimeName_t& name,
searchForProcessAndThen(
name,
[&](Process& process) {
runtime::IpcMessage instanceString({m_portManager.findService(service, instance)});
process.sendViaIpcChannel(instanceString);
LogDebug() << "Sent InstanceString to application " << name;
process.sendViaIpcChannel({m_portManager.findService(service, instance)});
LogDebug() << "Sent all found services to application " << name;
},
[&]() { LogWarn() << "Unknown process " << name << " requested an InstanceString."; });
[&]() { LogWarn() << "Unknown process " << name << " requested to find services."; });
}

void ProcessManager::addInterfaceForProcess(const RuntimeName_t& name,
Expand Down
Loading