Skip to content

Commit 5e14a28

Browse files
authored
Modify rclcpp_action::GoalUUID hashing algorithm (#2441)
* Add unit tests for hashing rclcpp_action::GoalUUID's * Use the FNV-1a hash algorithm for Goal UUID Signed-off-by: Mauro Passerino <[email protected]>
1 parent 51a4d2e commit 5e14a28

File tree

2 files changed

+40
-8
lines changed

2 files changed

+40
-8
lines changed

rclcpp_action/include/rclcpp_action/types.hpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,13 @@ struct hash<rclcpp_action::GoalUUID>
6969
{
7070
size_t operator()(const rclcpp_action::GoalUUID & uuid) const noexcept
7171
{
72-
// TODO(sloretz) Use someone else's hash function and cite it
73-
size_t result = 0;
74-
for (size_t i = 0; i < uuid.size(); ++i) {
75-
for (size_t b = 0; b < sizeof(size_t); ++b) {
76-
size_t part = uuid[i];
77-
part <<= CHAR_BIT * b;
78-
result ^= part;
79-
}
72+
// Using the FNV-1a hash algorithm
73+
constexpr size_t FNV_prime = 1099511628211u;
74+
size_t result = 14695981039346656037u;
75+
76+
for (const auto & byte : uuid) {
77+
result ^= byte;
78+
result *= FNV_prime;
8079
}
8180
return result;
8281
}

rclcpp_action/test/test_types.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <gtest/gtest.h>
1616

1717
#include <limits>
18+
#include <random>
1819
#include "rclcpp_action/types.hpp"
1920

2021
TEST(TestActionTypes, goal_uuid_to_string) {
@@ -59,3 +60,35 @@ TEST(TestActionTypes, rcl_action_goal_info_to_goal_uuid) {
5960
EXPECT_EQ(goal_info.goal_id.uuid[i], goal_id[i]);
6061
}
6162
}
63+
64+
TEST(TestActionTypes, goal_uuid_to_hashed_uuid_random) {
65+
// Use std::random_device to seed the generator of goal IDs.
66+
std::random_device rd;
67+
std::independent_bits_engine<
68+
std::default_random_engine, 8, decltype(rd())> random_bytes_generator(rd());
69+
70+
std::vector<size_t> hashed_guuids;
71+
constexpr size_t iterations = 1000;
72+
73+
for (size_t i = 0; i < iterations; i++) {
74+
rclcpp_action::GoalUUID goal_id;
75+
76+
// Generate random bytes for each element of the array
77+
for (auto & element : goal_id) {
78+
element = static_cast<uint8_t>(random_bytes_generator());
79+
}
80+
81+
size_t new_hashed_guuid = std::hash<rclcpp_action::GoalUUID>()(goal_id);
82+
83+
// Search for any prevoius hashed goal_id with the same value
84+
for (auto prev_hashed_guuid : hashed_guuids) {
85+
EXPECT_NE(prev_hashed_guuid, new_hashed_guuid);
86+
if (prev_hashed_guuid == new_hashed_guuid) {
87+
// Fail before the first occurrence of a collision
88+
GTEST_FAIL();
89+
}
90+
}
91+
92+
hashed_guuids.push_back(new_hashed_guuid);
93+
}
94+
}

0 commit comments

Comments
 (0)