From 7e591862725cd64c2edd2f2467580c3751c9bf2c Mon Sep 17 00:00:00 2001 From: Pete Sramek <2333452+petesramek@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:46:16 +0100 Subject: [PATCH 1/2] Saturate engine telemetry and validate ACK sequence Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/TinyLink.h | 61 +++++-- test/test_tinylink/test_main.cpp | 290 +++++++++++++++++++++++++++++++ 2 files changed, 333 insertions(+), 18 deletions(-) diff --git a/src/TinyLink.h b/src/TinyLink.h index 7003354..6d72312 100644 --- a/src/TinyLink.h +++ b/src/TinyLink.h @@ -145,6 +145,7 @@ class TinyLink { uint8_t _nextTransmitSequence; uint8_t _pendingTransmitSequence; uint8_t _lastReceivedSequence; + bool _hasReceivedSequence; unsigned long _lastHandshakeSent; unsigned long _lastTransmit; unsigned long _lastReceiveByte; @@ -155,6 +156,8 @@ class TinyLink { bool _hasNew; bool _outputDirty; + friend struct TinyLinkStatsAccess; + TinyLink(const TinyLink&) = delete; TinyLink& operator=(const TinyLink&) = delete; @@ -180,7 +183,7 @@ class TinyLink { plain, sizeof(plain)); if (packed.status != packet::PacketStatus::Success) { - ++_stats.packetEncodeFailures; + _stats.increment(TelemetryEvent::PacketEncodeFailed); return false; } @@ -189,7 +192,7 @@ class TinyLink { plain, packed.length, encoded, sizeof(encoded)); if (cobs.status != codec::CobsStatus::Success) { - ++_stats.cobsEncodeFailures; + _stats.increment(TelemetryEvent::CobsEncodeFailed); return false; } @@ -230,7 +233,7 @@ class TinyLink { if (encode_handshake_message( message, payload, sizeof(payload)) != HandshakeMessageCodecStatus::Success) { - ++_stats.invalidHandshakes; + _stats.increment(TelemetryEvent::InvalidHandshake); return false; } @@ -261,12 +264,12 @@ class TinyLink { if (result.status != packet::PacketStatus::Success) { if (result.status == packet::PacketStatus::ChecksumMismatch) { - ++_stats.checksumMismatches; + _stats.increment(TelemetryEvent::ChecksumMismatch); } else if (result.status == packet::PacketStatus::LengthMismatch || result.status == packet::PacketStatus::FrameTooShort) { - ++_stats.packetLengthMismatches; + _stats.increment(TelemetryEvent::PacketLengthMismatch); } else { - ++_stats.payloadSizeMismatches; + _stats.increment(TelemetryEvent::PayloadSizeMismatch); } return; } @@ -297,13 +300,19 @@ class TinyLink { decode_handshake_message(payload, payloadLength, message); if (status != HandshakeMessageCodecStatus::Success) { - ++_stats.invalidHandshakes; + _stats.increment(TelemetryEvent::InvalidHandshake); if (status == HandshakeMessageCodecStatus::UnsupportedVersion) { _connectionState = ConnectionState::Incompatible; + _hasReceivedSequence = false; } return; } + if (message.kind == HandshakeKind::Request) { + // A peer restarting its handshake makes the old frame stale. + _hasReceivedSequence = false; + } + if (_onHandshakeReceived) { _onHandshakeReceived(message); } @@ -329,13 +338,13 @@ class TinyLink { decode_ack_message(payload, payloadLength, result); if (status != AckMessageCodecStatus::Success) { - ++_stats.invalidAckResults; + _stats.increment(TelemetryEvent::InvalidAckResult); return; } if (_transmitState != TransmitState::AwaitingAck || sequence != _pendingTransmitSequence) { - ++_stats.unexpectedSequences; + _stats.increment(TelemetryEvent::UnexpectedSequence); return; } @@ -353,7 +362,7 @@ class TinyLink { if (decode_log_message(payload, payloadLength, message) != LogMessageCodecStatus::Success) { - ++_stats.payloadSizeMismatches; + _stats.increment(TelemetryEvent::PayloadSizeMismatch); return; } @@ -368,19 +377,20 @@ class TinyLink { size_t payloadLength) { if (!is_connected(_connectionState)) { - ++_stats.unexpectedMessageTypes; + _stats.increment(TelemetryEvent::UnexpectedMessageType); return; } if (payloadLength != sizeof(T)) { - ++_stats.payloadSizeMismatches; + _stats.increment(TelemetryEvent::PayloadSizeMismatch); return; } memcpy(&_mailbox, payload, sizeof(T)); _lastReceivedSequence = sequence; + _hasReceivedSequence = true; _hasNew = true; - ++_stats.packetsReceived; + _stats.increment(TelemetryEvent::PacketReceived); if (_onDataReceived) { _onDataReceived(_mailbox); @@ -406,7 +416,7 @@ class TinyLink { _receiveState = ReceiveState::WaitingForFrame; if (decoded.status != codec::CobsStatus::Success) { - ++_stats.cobsDecodeFailures; + _stats.increment(TelemetryEvent::CobsDecodeFailed); return; } @@ -434,7 +444,7 @@ class TinyLink { } if (_encodedReceiveLength >= sizeof(_encodedReceive)) { - ++_stats.receiveOverflows; + _stats.increment(TelemetryEvent::ReceiveOverflow); _encodedReceiveLength = 0; _receiveState = ReceiveState::DiscardingFrame; return; @@ -460,6 +470,7 @@ class TinyLink { _nextTransmitSequence(0), _pendingTransmitSequence(0), _lastReceivedSequence(0), + _hasReceivedSequence(false), _lastHandshakeSent(0), _lastTransmit(0), _lastReceiveByte(0), @@ -500,6 +511,7 @@ class TinyLink { { _handshakeEnabled = true; _connectionState = ConnectionState::Connecting; + _hasReceivedSequence = false; return send_handshake(HandshakeKind::Request); } @@ -515,6 +527,7 @@ class TinyLink { _nextTransmitSequence = 0; _pendingTransmitSequence = 0; _lastReceivedSequence = 0; + _hasReceivedSequence = false; } /** @brief Process timers and all currently available transport bytes. */ @@ -534,14 +547,14 @@ class TinyLink { if (_receiveState == ReceiveState::ReceivingFrame && now - _lastReceiveByte >= _timeout) { - ++_stats.interByteTimeouts; + _stats.increment(TelemetryEvent::InterByteTimeout); _encodedReceiveLength = 0; _receiveState = ReceiveState::WaitingForFrame; } if (_transmitState == TransmitState::AwaitingAck && now - _lastTransmit >= _timeout) { - ++_stats.ackTimeouts; + _stats.increment(TelemetryEvent::AckTimeout); _transmitState = TransmitState::Ready; } @@ -615,9 +628,21 @@ class TinyLink { return true; } - /** @brief Acknowledge the most recently accepted application frame. */ + /** + * @brief Acknowledge the most recently accepted application frame. + * + * Returns false without writing until an application frame has been + * accepted. The acknowledged sequence stays valid across callback + * delivery, read() and flush(). It is invalidated by construction, + * reset(), begin(), a peer handshake request (peer restart) and an + * unsupported handshake version (Incompatible). + */ bool sendAck(AckResult result = AckResult::Ok) { + if (!_hasReceivedSequence) { + return false; + } + uint8_t payload[ACK_MESSAGE_WIRE_SIZE] = {}; if (encode_ack_message(result, payload, sizeof(payload)) != AckMessageCodecStatus::Success) { diff --git a/test/test_tinylink/test_main.cpp b/test/test_tinylink/test_main.cpp index 6e8f13e..9099081 100644 --- a/test/test_tinylink/test_main.cpp +++ b/test/test_tinylink/test_main.cpp @@ -6,6 +6,16 @@ #include "TinyLinkTestHelpers.h" +namespace tinylink { + + // Test-only access declared as a friend by TinyLink to preload counters. + struct TinyLinkStatsAccess { + template + static Stats& stats(L& link) { return link._stats; } + }; + +} // namespace tinylink + namespace { struct Payload { @@ -72,6 +82,31 @@ namespace { return link.connected(); } + bool receiveApplication(FakeAdapter& adapter, Link& link, uint8_t sequence) + { + const Payload payload = payloadWith(0x10); + if (!tinylink_test::queueApplication( + adapter, 0x20, sequence, payload.bytes, sizeof(payload.bytes))) { + return false; + } + link.update(); + adapter.clearOutput(); + return true; + } + + uint8_t ackedSequence(const FakeAdapter& adapter) + { + uint8_t type = 0; + uint8_t sequence = 0; + uint8_t payload[tinylink::ACK_MESSAGE_WIRE_SIZE] = {}; + size_t length = 0; + TEST_ASSERT_TRUE(tinylink_test::decodeWrittenFrame( + adapter, 0, type, sequence, payload, sizeof(payload), length)); + TEST_ASSERT_EQUAL_UINT8( + tinylink::message_type_to_wire(tinylink::MessageType::Ack), type); + return sequence; + } + void testConstructionStartsInactiveAndReadyWithZeroTelemetry() { FakeAdapter adapter; @@ -1066,6 +1101,7 @@ namespace { FakeAdapter adapter; Link link(adapter); TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 5)); const size_t failed[] = { 0 }; scriptWrites(adapter, failed, 1); @@ -1151,6 +1187,7 @@ namespace { FakeAdapter adapter; Link link(adapter); TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 5)); const Payload payload = payloadWith(0x61); adapter.setOpen(false); TEST_ASSERT_FALSE(link.sendData(0x2D, payload)); @@ -1308,6 +1345,249 @@ namespace { TEST_ASSERT_TRUE(link.connected()); } + void testEngineCountersSaturateAtMaximumInsteadOfWrapping() + { + FakeAdapter adapter; + Link link(adapter); + tinylink::Stats& counters = + tinylink::TinyLinkStatsAccess::stats(link); + counters.packetsReceived = UINT32_MAX - 1u; + const Payload payload = payloadWith(0x30); + const size_t size = sizeof(payload.bytes); + + // Rejected before connection: exercises UnexpectedMessageType. + TEST_ASSERT_TRUE(tinylink_test::queueApplication( + adapter, 0x20, 1, payload.bytes, size)); + counters.unexpectedMessageTypes = UINT32_MAX; + link.update(); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, link.stats().unexpectedMessageTypes); + + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(tinylink_test::queueApplication( + adapter, 0x20, 2, payload.bytes, size)); + TEST_ASSERT_TRUE(tinylink_test::queueApplication( + adapter, 0x20, 3, payload.bytes, size)); + TEST_ASSERT_TRUE(tinylink_test::queueApplication( + adapter, 0x20, 4, payload.bytes, size)); + link.update(); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, link.stats().packetsReceived); + + counters.cobsDecodeFailures = UINT32_MAX; + counters.checksumMismatches = UINT32_MAX; + counters.packetLengthMismatches = UINT32_MAX; + counters.payloadSizeMismatches = UINT32_MAX; + counters.invalidAckResults = UINT32_MAX; + counters.unexpectedSequences = UINT32_MAX; + counters.invalidHandshakes = UINT32_MAX; + counters.receiveOverflows = UINT32_MAX; + counters.interByteTimeouts = UINT32_MAX; + counters.ackTimeouts = UINT32_MAX; + + const uint8_t malformed[] = { 0x04u, 0x11u, 0x00u }; + TEST_ASSERT_TRUE(adapter.queueBytes(malformed, sizeof(malformed))); + + uint8_t plain[tinylink::packet::PACKET_MAX_SIZE] = {}; + const tinylink::packet::PacketResult packed = tinylink::packet::pack( + 0x20, 1, payload.bytes, size, plain, sizeof(plain)); + TEST_ASSERT_EQUAL( + static_cast(tinylink::packet::PacketStatus::Success), + static_cast(packed.status)); + plain[packed.length - 1] ^= 0x01u; + TEST_ASSERT_TRUE(tinylink_test::queueFrame(adapter, plain, packed.length)); + + const uint8_t shortPlain[] = { 1, 2, 3 }; + TEST_ASSERT_TRUE(tinylink_test::queueFrame( + adapter, shortPlain, sizeof(shortPlain))); + + TEST_ASSERT_TRUE(tinylink_test::queueApplication( + adapter, 0x20, 5, payload.bytes, size - 1)); + TEST_ASSERT_TRUE(tinylink_test::queuePacket( + adapter, + tinylink::message_type_to_wire(tinylink::MessageType::Ack), + 0, nullptr, 0)); + TEST_ASSERT_TRUE(tinylink_test::queueAck( + adapter, 9, tinylink::AckResult::Ok)); + TEST_ASSERT_TRUE(tinylink_test::queuePacket( + adapter, + tinylink::message_type_to_wire(tinylink::MessageType::Handshake), + 0, nullptr, 0)); + + uint8_t overflow[ + tinylink::packet::PACKET_MAX_SIZE + + tinylink::packet::PACKET_MAX_SIZE / 254u + 2u]; + memset(overflow, 0x01, sizeof(overflow)); + TEST_ASSERT_TRUE(adapter.queueBytes(overflow, sizeof(overflow))); + TEST_ASSERT_TRUE(adapter.queueByte(0)); + link.update(); + + TEST_ASSERT_TRUE(adapter.queueByte(0x01u)); + link.update(); + adapter.advanceTime(1000); + link.update(); + + TEST_ASSERT_TRUE(link.sendData(0x2A, payload)); + adapter.advanceTime(1000); + link.update(); + + const tinylink::Stats stats = link.stats(); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.cobsDecodeFailures); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.checksumMismatches); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.packetLengthMismatches); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.payloadSizeMismatches); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.invalidAckResults); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.unexpectedSequences); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.invalidHandshakes); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.receiveOverflows); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.interByteTimeouts); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.ackTimeouts); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.unexpectedMessageTypes); + TEST_ASSERT_EQUAL_UINT32(UINT32_MAX, stats.packetsReceived); + } + + void testSendAckReturnsFalseBeforeAnyApplicationFrameIsAccepted() + { + FakeAdapter adapter; + Link link(adapter); + TEST_ASSERT_FALSE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT32(0, adapter.outputSize()); + + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_FALSE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT32(0, adapter.outputSize()); + } + + void testRejectedApplicationFramesDoNotValidateAckSequence() + { + FakeAdapter adapter; + Link link(adapter); + const Payload payload = payloadWith(0x40); + TEST_ASSERT_TRUE(tinylink_test::queueApplication( + adapter, 0x20, 7, payload.bytes, sizeof(payload.bytes))); + link.update(); + TEST_ASSERT_FALSE(link.sendAck()); + + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(tinylink_test::queueApplication( + adapter, 0x20, 8, payload.bytes, sizeof(payload.bytes) - 1)); + link.update(); + TEST_ASSERT_FALSE(link.available()); + TEST_ASSERT_FALSE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT32(0, adapter.outputSize()); + } + + void testAckSequenceRemainsValidAfterCallbackDelivery() + { + FakeAdapter adapter; + Link link(adapter); + link.onDataReceived(onData); + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 41)); + + TEST_ASSERT_EQUAL_UINT32(1, dataCallbackCount); + TEST_ASSERT_TRUE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT8(41, ackedSequence(adapter)); + } + + void testAckSequenceRemainsValidAfterRead() + { + FakeAdapter adapter; + Link link(adapter); + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 42)); + + static_cast(link.read()); + TEST_ASSERT_FALSE(link.available()); + TEST_ASSERT_TRUE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT8(42, ackedSequence(adapter)); + } + + void testAckSequenceRemainsValidAfterFlush() + { + FakeAdapter adapter; + Link link(adapter); + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 43)); + + link.flush(); + TEST_ASSERT_FALSE(link.available()); + TEST_ASSERT_TRUE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT8(43, ackedSequence(adapter)); + } + + void testResetInvalidatesAckSequence() + { + FakeAdapter adapter; + Link link(adapter); + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 44)); + TEST_ASSERT_TRUE(link.sendAck()); + adapter.clearOutput(); + + link.reset(); + TEST_ASSERT_FALSE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT32(0, adapter.outputSize()); + } + + void testBeginInvalidatesAckSequence() + { + FakeAdapter adapter; + Link link(adapter); + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 45)); + + TEST_ASSERT_TRUE(link.begin()); + adapter.clearOutput(); + TEST_ASSERT_FALSE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT32(0, adapter.outputSize()); + } + + void testPeerHandshakeRequestInvalidatesAckSequence() + { + FakeAdapter adapter; + Link link(adapter); + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 46)); + + TEST_ASSERT_TRUE(tinylink_test::queueHandshake( + adapter, tinylink::HandshakeKind::Request)); + link.update(); + adapter.clearOutput(); + TEST_ASSERT_TRUE(link.connected()); + TEST_ASSERT_FALSE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT32(0, adapter.outputSize()); + } + + void testIncompatibleHandshakeInvalidatesAckSequence() + { + FakeAdapter adapter; + Link link(adapter); + TEST_ASSERT_TRUE(connectPeer(adapter, link)); + TEST_ASSERT_TRUE(receiveApplication(adapter, link, 47)); + + const tinylink::HandshakeMessage supported = { + tinylink::HandshakeKind::Request, + TINYLINK_PROTOCOL_VERSION + }; + uint8_t wire[tinylink::HANDSHAKE_MESSAGE_WIRE_SIZE] = {}; + TEST_ASSERT_EQUAL( + static_cast(tinylink::HandshakeMessageCodecStatus::Success), + static_cast(tinylink::encode_handshake_message( + supported, wire, sizeof(wire)))); + wire[sizeof(wire) - 1] = static_cast( + TINYLINK_PROTOCOL_VERSION + 1); + TEST_ASSERT_TRUE(tinylink_test::queuePacket( + adapter, + tinylink::message_type_to_wire(tinylink::MessageType::Handshake), + 0, wire, sizeof(wire))); + link.update(); + + TEST_ASSERT_EQUAL( + static_cast(tinylink::ConnectionState::Incompatible), + static_cast(link.connectionState())); + TEST_ASSERT_FALSE(link.sendAck()); + TEST_ASSERT_EQUAL_UINT32(0, adapter.outputSize()); + } + // Positive compile contracts: valid payloads, including the maximum size. static_assert(tinylink::is_payload_copyable::value, "plain byte-array payload must be accepted"); @@ -1383,6 +1663,16 @@ int main() RUN_TEST(testResetClearsRuntimeStateButPreservesCallbacksTimeoutAndStats); RUN_TEST(testResetCancelsPendingAckAndDiscardsPartialReceiveFrame); RUN_TEST(testClearStatsResetsCountersWithoutChangingConnectionState); + RUN_TEST(testEngineCountersSaturateAtMaximumInsteadOfWrapping); + RUN_TEST(testSendAckReturnsFalseBeforeAnyApplicationFrameIsAccepted); + RUN_TEST(testRejectedApplicationFramesDoNotValidateAckSequence); + RUN_TEST(testAckSequenceRemainsValidAfterCallbackDelivery); + RUN_TEST(testAckSequenceRemainsValidAfterRead); + RUN_TEST(testAckSequenceRemainsValidAfterFlush); + RUN_TEST(testResetInvalidatesAckSequence); + RUN_TEST(testBeginInvalidatesAckSequence); + RUN_TEST(testPeerHandshakeRequestInvalidatesAckSequence); + RUN_TEST(testIncompatibleHandshakeInvalidatesAckSequence); return UNITY_END(); } From 076532c3941c98c37d3808895d1d12f54649a7c0 Mon Sep 17 00:00:00 2001 From: Pete Sramek <2333452+petesramek@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:46:33 +0100 Subject: [PATCH 2/2] docs: changelog for stats saturation and ACK validity Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8b9e18..ee15b79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 All registered in `test/test_runner.cpp`. ### Changed +- `TinyLink` telemetry now uses the saturating `Stats::increment(TelemetryEvent)` API for every + counter, so engine counters never wrap. `sendAck()` now returns `false` (writing nothing) until + an application frame has been accepted; the acknowledged sequence survives callback delivery, + `read()` and `flush()` and is invalidated by construction, `reset()`, `begin()`, a peer handshake + request, and an unsupported-version handshake. - `TinyLink` now requires payload type `T` to be trivially copyable, enforced with a `static_assert` on every toolchain including AVR (compiler intrinsic where `` is unavailable). Trivially copyable is only a minimum object-safety requirement; raw object memory