Skip to content

Commit 4aa92a7

Browse files
fix: resolve Reliability clang-tidy issues and Windows UDP test timeout
Apply const-correctness, direct includes, member initializers, and pointer-based vector assign in Reliability; increase UDP test timeout for slower Windows CI. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 045e3be commit 4aa92a7

3 files changed

Lines changed: 24 additions & 22 deletions

File tree

src/nuclearnet/Reliability.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@
2929
#include <cstdint>
3030
#include <cstring>
3131
#include <mutex>
32+
#include <utility>
3233
#include <vector>
3334

35+
#include "RTTEstimator.hpp"
3436
#include "wire_protocol.hpp"
3537

3638
namespace NUClear {
@@ -55,7 +57,7 @@ namespace network {
5557
tp.acked.resize(packet_count, false);
5658
tp.last_send = now;
5759

58-
TrackingKey key{target, packet_id};
60+
const TrackingKey key{target, packet_id};
5961

6062
const std::lock_guard<std::mutex> lock(tracking_mutex);
6163
tracked_packets[key] = std::move(tp);
@@ -67,7 +69,7 @@ namespace network {
6769
const uint8_t* ack_bitset,
6870
std::size_t bitset_size,
6971
std::chrono::steady_clock::time_point now) {
70-
TrackingKey key{source, packet_id};
72+
const TrackingKey key{source, packet_id};
7173

7274
const std::lock_guard<std::mutex> lock(tracking_mutex);
7375
auto it = tracked_packets.find(key);
@@ -95,8 +97,8 @@ namespace network {
9597
// Update acked bitset
9698
bool all_acked = true;
9799
for (uint16_t i = 0; i < packet_count && i < tp.acked.size(); ++i) {
98-
std::size_t byte_idx = i / 8;
99-
uint8_t bit_idx = i % 8;
100+
const std::size_t byte_idx = i / 8;
101+
const uint8_t bit_idx = static_cast<uint8_t>(i % 8);
100102
if (byte_idx < bitset_size && (ack_bitset[byte_idx] & (1u << bit_idx)) != 0) {
101103
tp.acked[i] = true;
102104
}
@@ -115,8 +117,8 @@ namespace network {
115117
uint16_t packet_count,
116118
const std::vector<bool>& received) {
117119
// Calculate bitset size: ceil(packet_count / 8)
118-
std::size_t bitset_bytes = (packet_count + 7) / 8;
119-
std::size_t total_size = sizeof(ACKPacket) - 1 + bitset_bytes; // -1 for the placeholder uint8_t
120+
const std::size_t bitset_bytes = (packet_count + 7) / 8;
121+
const std::size_t total_size = sizeof(ACKPacket) - 1 + bitset_bytes; // -1 for the placeholder uint8_t
120122

121123
std::vector<uint8_t> packet(total_size, 0);
122124

@@ -171,9 +173,9 @@ namespace network {
171173
req.hash = tp.hash;
172174

173175
// Extract the fragment data
174-
std::size_t offset = static_cast<std::size_t>(i) * packet_mtu;
175-
std::size_t length = std::min(static_cast<std::size_t>(packet_mtu), tp.payload.size() - offset);
176-
req.data.assign(tp.payload.begin() + offset, tp.payload.begin() + offset + length);
176+
const std::size_t offset = static_cast<std::size_t>(i) * packet_mtu;
177+
const std::size_t length = std::min(static_cast<std::size_t>(packet_mtu), tp.payload.size() - offset);
178+
req.data.assign(tp.payload.data() + offset, tp.payload.data() + offset + length);
177179

178180
retransmissions.push_back(std::move(req));
179181
}

src/nuclearnet/Reliability.hpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ namespace network {
5252

5353
/// Information about a fragment that needs retransmitting
5454
struct RetransmitRequest {
55-
sock_t target;
56-
uint16_t packet_id;
57-
uint16_t packet_no;
58-
uint16_t packet_count;
59-
uint8_t flags;
60-
uint64_t hash;
55+
sock_t target{};
56+
uint16_t packet_id{0};
57+
uint16_t packet_no{0};
58+
uint16_t packet_count{0};
59+
uint8_t flags{0};
60+
uint64_t hash{0};
6161
std::vector<uint8_t> data;
6262
};
6363

@@ -144,14 +144,14 @@ namespace network {
144144
private:
145145
/// State for a tracked reliable packet group
146146
struct TrackedPacket {
147-
sock_t target;
148-
uint16_t packet_id;
149-
uint16_t packet_count;
150-
uint64_t hash;
151-
uint8_t flags;
147+
sock_t target{};
148+
uint16_t packet_id{0};
149+
uint16_t packet_count{0};
150+
uint64_t hash{0};
151+
uint8_t flags{0};
152152
std::vector<uint8_t> payload;
153153
std::vector<bool> acked; ///< Which fragments have been ACKed
154-
std::chrono::steady_clock::time_point last_send;
154+
std::chrono::steady_clock::time_point last_send{};
155155
uint16_t retransmit_count{0};
156156
};
157157

tests/tests/dsl/UDP.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ class TestReactor : public test_util::TestBase<TestReactor> {
186186
}
187187

188188
TestReactor(std::unique_ptr<NUClear::Environment> environment, const std::vector<TestType>& active_tests_)
189-
: TestBase(std::move(environment), false), active_tests(active_tests_) {
189+
: TestBase(std::move(environment), false, test_util::TimeUnit(200)), active_tests(active_tests_) {
190190

191191
for (const auto& t : active_tests) {
192192
switch (t) {

0 commit comments

Comments
 (0)