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.

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

Address known limitation in service/client (iron) #108

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 12 additions & 2 deletions rmw_iceoryx_cpp/src/rmw_request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,25 @@ rmw_take_request(
}

// Hold the loaned request till we send the response in 'rmw_send_response'
iceoryx_server_abstraction->request_payload_ = iceoryx_request_payload;
auto result =
iceoryx_server_abstraction->request_payload_map_.insert(
{request_header->request_id.
sequence_number, iceoryx_request_payload});

if (result.second == false) {
*taken = false;
RMW_SET_ERROR_MSG("rmw_take_request: Could not store the sample pointer in the map!");
ret = RMW_RET_ERROR;
return;
}

*taken = true;
ret = RMW_RET_OK;
})
.or_else(
[&](auto &) {
*taken = false;
RMW_SET_ERROR_MSG("rmw_take_request error!");
RMW_SET_ERROR_MSG("rmw_take_request: Taking the sample failed!");
ret = RMW_RET_ERROR;
});

Expand Down
20 changes: 14 additions & 6 deletions rmw_iceoryx_cpp/src/rmw_response.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,23 @@ rmw_send_response(
}

rmw_ret_t ret = RMW_RET_ERROR;
auto & payload_ptr_map = iceoryx_server_abstraction->request_payload_map_;

if (!iceoryx_server_abstraction->request_payload_) {
if (payload_ptr_map.empty()) {
RMW_SET_ERROR_MSG("'rmw_take_request' needs to be called before 'rmw_send_response'!");
ret = RMW_RET_ERROR;
return ret;
}

auto * iceoryx_request_header = iox::popo::RequestHeader::fromPayload(
iceoryx_server_abstraction->request_payload_);
auto it = payload_ptr_map.find(request_header->sequence_number);

if (it == payload_ptr_map.end()) {
RMW_SET_ERROR_MSG("Could not find the payload pointer in the map");
ret = RMW_RET_ERROR;
return ret;
}

auto * iceoryx_request_header = iox::popo::RequestHeader::fromPayload((*it).second);

iceoryx_server->loan(
iceoryx_request_header, iceoryx_server_abstraction->response_size_,
Expand Down Expand Up @@ -186,9 +194,9 @@ rmw_send_response(
ret = RMW_RET_ERROR;
});

// Release the hold request
iceoryx_server->releaseRequest(iceoryx_server_abstraction->request_payload_);
iceoryx_server_abstraction->request_payload_ = nullptr;
// Release the hold request and delete the entry from the map
iceoryx_server->releaseRequest((*it).second);
payload_ptr_map.erase(request_header->sequence_number);

return ret;
}
Expand Down
15 changes: 9 additions & 6 deletions rmw_iceoryx_cpp/src/types/iceoryx_server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#ifndef TYPES__ICEORYX_SERVER_HPP_
#define TYPES__ICEORYX_SERVER_HPP_

#include <map>

#include "iceoryx_posh/popo/untyped_server.hpp"

#include "rmw/rmw.h"
Expand All @@ -31,17 +33,18 @@ struct IceoryxServer
iceoryx_server_(iceoryx_server),
is_fixed_size_(rmw_iceoryx_cpp::iceoryx_is_fixed_size(type_supports)),
response_size_(rmw_iceoryx_cpp::iceoryx_get_response_size(type_supports))
{}
{
}

rosidl_service_type_support_t type_supports_;
iox::popo::UntypedServer * const iceoryx_server_;
bool is_fixed_size_{false};
size_t response_size_{0};
/// @todo The request payload pointer could also be added to 'rmw_request_id_t' if accepeted in
/// ros2/rmw. For now, the limitation exists to always call 'rmw_take_request' followed by
/// 'rmw_send_response' and not call e.g. 2x times 'rmw_take_request' and then
/// 2x 'rmw_send_response'
const void * request_payload_{nullptr};
/// @brief The map stores the sequence numbers together with the corresponding
/// sample pointer pointing to the shared memory. This is due to the fact that
/// 'rmw_request_id_t' misses a place to store the sample pointer, which is not
/// typical with DDS implementations.
std::map<int64_t, const void *> request_payload_map_;
};

#endif // TYPES__ICEORYX_SERVER_HPP_
Loading