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

Add sequence numbers to message info structure #587

Merged
merged 6 commits into from
Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions rmw_fastrtps_cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ add_library(rmw_fastrtps_cpp
src/rmw_compare_gids_equal.cpp
src/rmw_count.cpp
src/rmw_event.cpp
src/rmw_features.cpp
src/rmw_get_gid_for_publisher.cpp
src/rmw_get_implementation_identifier.cpp
src/rmw_get_serialization_format.cpp
Expand Down
27 changes: 27 additions & 0 deletions rmw_fastrtps_cpp/src/rmw_features.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2022 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "rmw/features.h"

#include "rmw_fastrtps_shared_cpp/rmw_common.hpp"

extern "C"
{

bool
rmw_feature_supported(rmw_feature_t feature)
{
return rmw_fastrtps_shared_cpp::__rmw_feature_supported(feature);
}
} // extern "C"
1 change: 1 addition & 0 deletions rmw_fastrtps_dynamic_cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ add_library(rmw_fastrtps_dynamic_cpp
src/rmw_compare_gids_equal.cpp
src/rmw_count.cpp
src/rmw_event.cpp
src/rmw_features.cpp
src/rmw_get_gid_for_publisher.cpp
src/rmw_get_implementation_identifier.cpp
src/rmw_get_serialization_format.cpp
Expand Down
27 changes: 27 additions & 0 deletions rmw_fastrtps_dynamic_cpp/src/rmw_features.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2022 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "rmw/features.h"

#include "rmw_fastrtps_shared_cpp/rmw_common.hpp"

extern "C"
{

bool
rmw_feature_supported(rmw_feature_t feature)
{
return rmw_fastrtps_shared_cpp::__rmw_feature_supported(feature);
}
} // extern "C"
1 change: 1 addition & 0 deletions rmw_fastrtps_shared_cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ add_library(rmw_fastrtps_shared_cpp
src/rmw_compare_gids_equal.cpp
src/rmw_count.cpp
src/rmw_event.cpp
src/rmw_features.cpp
src/rmw_get_endpoint_network_flow.cpp
src/rmw_get_gid_for_publisher.cpp
src/rmw_get_topic_endpoint_info.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "rmw/error_handling.h"
#include "rmw/event.h"
#include "rmw/features.h"
#include "rmw/rmw.h"
#include "rmw/topic_endpoint_info_array.h"
#include "rmw/types.h"
Expand Down Expand Up @@ -514,6 +515,10 @@ __rmw_event_set_callback(
rmw_event_callback_t callback,
const void * user_data);

RMW_FASTRTPS_SHARED_CPP_PUBLIC
bool
__rmw_feature_supported(rmw_feature_t feature);

} // namespace rmw_fastrtps_shared_cpp

#endif // RMW_FASTRTPS_SHARED_CPP__RMW_COMMON_HPP_
26 changes: 26 additions & 0 deletions rmw_fastrtps_shared_cpp/src/rmw_features.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2022 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "rmw/features.h"

#include "rmw_fastrtps_shared_cpp/rmw_common.hpp"

bool
rmw_fastrtps_shared_cpp::__rmw_feature_supported(rmw_feature_t feature)
{
if (feature == RMW_FEATURE_MESSAGE_INFO_PUBLICATION_SEQUENCE_NUMBER) {
return true;
}
return false;
}
4 changes: 4 additions & 0 deletions rmw_fastrtps_shared_cpp/src/rmw_take.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ _assign_message_info(
{
message_info->source_timestamp = sinfo->source_timestamp.to_ns();
message_info->received_timestamp = sinfo->reception_timestamp.to_ns();
auto fastdds_sn = sinfo->sample_identity.sequence_number();
message_info->publication_sequence_number = (static_cast<uint64_t>(fastdds_sn.high)) <<
32 | static_cast<uint64_t>(fastdds_sn.low);
ivanpauno marked this conversation as resolved.
Show resolved Hide resolved
message_info->reception_sequence_number = RMW_MESSAGE_INFO_SEQUENCE_NUMBER_UNSUPPORTED;
Copy link
Member

Choose a reason for hiding this comment

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

Should we just implement this by taking a timestamp here?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally we'd take the timestamp when it is received, not as it is being taken. So maybe leaving it as-is would be best. I'm not sure. Can we ask if it's possible to take a timestamp when that occurs?

I was actually just using this timestamp to debug an issue, because it's useful to know the time between the middleware receiving the message (ready to deliver) and the user code actually taking it, as it tells you something about the latency introduced by waiting or the executor or listener-style callbacks, which ever you are using.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we just implement this by taking a timestamp here?

The reception sequence number?
If we want to manually implement it, I would use a counter, not a timestamp.
I would expect the reception sequence number to not have gaps, that's why it would be weird to use a timestamp IMO.

I was actually just using this timestamp to debug an issue, because it's useful to know the time between the middleware receiving the message (ready to deliver) and the user code actually taking it, as it tells you something about the latency introduced by waiting or the executor or listener-style callbacks, which ever you are using.

Maybe tracing is the correct way to measure that.
We probably need just to add a couple more tracepoints.

Copy link
Member

Choose a reason for hiding this comment

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

Doh, I'm just confused. I read reception and just auto-completed in my brain to timestamp, lol.

We already have the reception timestamp. You're right it should just be a counter.

So, original question, should we ask about how to implement that? We'd need to increment and store the counter when receiving the message, not taking it, so it'd have to be in a different part of the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, original question, should we ask about how to implement that? We'd need to increment and store the counter when receiving the message, not taking it, so it'd have to be in a different part of the code.

I guess it's maybe possible to do that in a listener, but I guess you could miss a message that way (because many were got together and the queue was overridden), so maybe it's better to leave it unimplemented until there's actual support for it (maybe there's support already and I haven't found it).

@MiguelCompany @richiprosima is the reception sequence number available in FastDDS?

rmw_gid_t * sender_gid = &message_info->publisher_gid;
sender_gid->implementation_identifier = identifier;
memset(sender_gid->data, 0, RMW_GID_STORAGE_SIZE);
Expand Down