pw_hdlc: Multibyte address support
This extends the HDLC encoder and decoder in both C++ and Python to use
HDLC extended addressing, where the address field is a one-terminated
LSB varint.
Classes using HDLC, such as the HDLC ChannelOutput and WirePacketParser,
are updated accordingly.
Change-Id: I6affc4443079628567d33939d809218f7b7bdf41
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/38020
Reviewed-by: Keir Mierle <keir@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
diff --git a/pw_hdlc/BUILD b/pw_hdlc/BUILD
index ad4a3af..9463dbc 100644
--- a/pw_hdlc/BUILD
+++ b/pw_hdlc/BUILD
@@ -27,7 +27,7 @@
"decoder.cc",
"encoder.cc",
"public/pw_hdlc/internal/encoder.h",
- "pw_hdlc_private/protocol.h",
+ "public/pw_hdlc/internal/protocol.h",
"rpc_packets.cc",
],
hdrs = [
@@ -43,6 +43,7 @@
"//pw_span",
"//pw_status",
"//pw_stream",
+ "//pw_varint",
],
)
diff --git a/pw_hdlc/BUILD.gn b/pw_hdlc/BUILD.gn
index 3f837f5..41e5e4b 100644
--- a/pw_hdlc/BUILD.gn
+++ b/pw_hdlc/BUILD.gn
@@ -30,14 +30,19 @@
]
}
+pw_source_set("common") {
+ public_configs = [ ":default_config" ]
+ public = [ "public/pw_hdlc/internal/protocol.h" ]
+ public_deps = [ dir_pw_varint ]
+ visibility = [ ":*" ]
+}
+
pw_source_set("decoder") {
public_configs = [ ":default_config" ]
public = [ "public/pw_hdlc/decoder.h" ]
- sources = [
- "decoder.cc",
- "pw_hdlc_private/protocol.h",
- ]
+ sources = [ "decoder.cc" ]
public_deps = [
+ ":common",
dir_pw_bytes,
dir_pw_checksum,
dir_pw_result,
@@ -53,9 +58,9 @@
sources = [
"encoder.cc",
"public/pw_hdlc/internal/encoder.h",
- "pw_hdlc_private/protocol.h",
]
public_deps = [
+ ":common",
dir_pw_bytes,
dir_pw_checksum,
dir_pw_status,
@@ -91,7 +96,6 @@
public_deps = [
":pw_hdlc",
"$dir_pw_router:packet_parser",
- dir_pw_assert,
]
deps = [
dir_pw_bytes,
diff --git a/pw_hdlc/decoder.cc b/pw_hdlc/decoder.cc
index 7c14cc1..c2e10aa 100644
--- a/pw_hdlc/decoder.cc
+++ b/pw_hdlc/decoder.cc
@@ -16,13 +16,27 @@
#include "pw_assert/assert.h"
#include "pw_bytes/endian.h"
-#include "pw_hdlc_private/protocol.h"
+#include "pw_hdlc/internal/protocol.h"
#include "pw_log/log.h"
+#include "pw_varint/varint.h"
using std::byte;
namespace pw::hdlc {
+Result<Frame> Frame::Parse(ConstByteSpan frame) {
+ uint64_t address;
+ size_t address_size = varint::Decode(frame, &address, kAddressFormat);
+ int data_size = frame.size() - address_size - kControlSize - kFcsSize;
+
+ if (address_size == 0 || data_size < 0) {
+ return Status::DataLoss();
+ }
+
+ return Frame(
+ address, frame[address_size], frame.subspan(address_size + 1, data_size));
+}
+
Result<Frame> Decoder::Process(const byte new_byte) {
switch (state_) {
case State::kInterFrame: {
@@ -48,7 +62,7 @@
Reset();
if (status.ok()) {
- return Frame(buffer_.first(completed_frame_size));
+ return Frame::Parse(buffer_.first(completed_frame_size));
}
return status;
}
diff --git a/pw_hdlc/decoder_test.cc b/pw_hdlc/decoder_test.cc
index bd17414..af842ea 100644
--- a/pw_hdlc/decoder_test.cc
+++ b/pw_hdlc/decoder_test.cc
@@ -19,7 +19,7 @@
#include "gtest/gtest.h"
#include "pw_bytes/array.h"
-#include "pw_hdlc_private/protocol.h"
+#include "pw_hdlc/internal/protocol.h"
namespace pw::hdlc {
namespace {
@@ -27,15 +27,43 @@
using std::byte;
TEST(Frame, Fields) {
- static constexpr auto kFrameData = bytes::String("1234\xa3\xe0\xe3\x9b");
- constexpr Frame frame(kFrameData);
+ static constexpr auto kFrameData =
+ bytes::String("\x05\xab\x42\x24\xf9\x54\xfb\x3d");
+ auto result = Frame::Parse(kFrameData);
+ ASSERT_TRUE(result.ok());
+ Frame& frame = result.value();
- static_assert(frame.address() == unsigned{'1'});
- static_assert(frame.control() == byte{'2'});
+ EXPECT_EQ(frame.address(), 2u);
+ EXPECT_EQ(frame.control(), byte{0xab});
- static_assert(frame.data().size() == 2u);
- static_assert(frame.data()[0] == byte{'3'});
- static_assert(frame.data()[1] == byte{'4'});
+ EXPECT_EQ(frame.data().size(), 2u);
+ EXPECT_EQ(frame.data()[0], byte{0x42});
+ EXPECT_EQ(frame.data()[1], byte{0x24});
+}
+
+TEST(Frame, MultibyteAddress) {
+ static constexpr auto kFrameData =
+ bytes::String("\x2c\xd9\x33\x01\x02\xaf\xc8\x77\x48");
+ auto result = Frame::Parse(kFrameData);
+ ASSERT_TRUE(result.ok());
+ Frame& frame = result.value();
+
+ EXPECT_EQ(frame.address(), 0b11011000010110u);
+ EXPECT_EQ(frame.control(), byte{0x33});
+
+ EXPECT_EQ(frame.data().size(), 2u);
+ EXPECT_EQ(frame.data()[0], byte{0x01});
+ EXPECT_EQ(frame.data()[1], byte{0x02});
+}
+
+TEST(Frame, MultibyteAddressTooLong) {
+ // 11-byte encoded address.
+ constexpr auto kLongAddress =
+ bytes::String("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01");
+ static constexpr auto kFrameData = bytes::Concat(
+ kLongAddress, bytes::String("\x33\x01\x02\xaf\xc8\x77\x48"));
+ auto result = Frame::Parse(kFrameData);
+ EXPECT_EQ(result.status(), Status::DataLoss());
}
TEST(Decoder, Clear) {
diff --git a/pw_hdlc/docs.rst b/pw_hdlc/docs.rst
index c8b574a..6791855 100644
--- a/pw_hdlc/docs.rst
+++ b/pw_hdlc/docs.rst
@@ -102,7 +102,7 @@
^^^
.. cpp:namespace:: pw
-.. cpp:function:: Status hdlc::WriteUIFrame(uint8_t address, ConstByteSpan data, stream::Writer& writer)
+.. cpp:function:: Status hdlc::WriteUIFrame(uint64_t address, ConstByteSpan data, stream::Writer& writer)
Writes a span of data to a :ref:`pw::stream::Writer <module-pw_stream>` and
returns the status. This implementation uses the :ref:`module-pw_checksum`
@@ -221,9 +221,8 @@
Roadmap
=======
- **Expanded protocol support** - ``pw_hdlc`` currently only supports
- unnumbered information frames with a single address byte and control byte.
- Support for different frame types and extended address or control fields may
- be added in the future.
+ unnumbered information frames. Support for different frame types and
+ extended control fields may be added in the future.
- **Higher performance** - We plan to improve the overall performance of the
decoder and encoder implementations by using SIMD/NEON.
diff --git a/pw_hdlc/encoder.cc b/pw_hdlc/encoder.cc
index b0d209e..9b1b027 100644
--- a/pw_hdlc/encoder.cc
+++ b/pw_hdlc/encoder.cc
@@ -22,16 +22,13 @@
#include "pw_bytes/endian.h"
#include "pw_hdlc/internal/encoder.h"
-#include "pw_hdlc_private/protocol.h"
+#include "pw_varint/varint.h"
using std::byte;
namespace pw::hdlc {
namespace internal {
-// Indicates this an information packet with sequence numbers set to 0.
-constexpr byte kUnusedControl = byte{0};
-
Status EscapeAndWrite(const byte b, stream::Writer& writer) {
if (b == kFlag) {
return writer.Write(kEscapedFlag);
@@ -42,28 +39,6 @@
return writer.Write(b);
}
-Status Encoder::StartInformationFrame(uint8_t address) {
- fcs_.clear();
- if (Status status = writer_.Write(kFlag); !status.ok()) {
- return status;
- }
-
- const byte address_and_control[] = {
- std::byte{address}, kUnusedControl, kUnusedControl};
- return WriteData(address_and_control);
-}
-
-Status Encoder::StartUnnumberedFrame(uint8_t address) {
- fcs_.clear();
- if (Status status = writer_.Write(kFlag); !status.ok()) {
- return status;
- }
-
- const byte address_and_control[] = {
- std::byte{address}, UFrameControl::UnnumberedInformation().data()};
- return WriteData(address_and_control);
-}
-
Status Encoder::WriteData(ConstByteSpan data) {
auto begin = data.begin();
while (true) {
@@ -92,19 +67,37 @@
return writer_.Write(kFlag);
}
-size_t Encoder::MaxEncodedSize(uint8_t address, ConstByteSpan payload) {
+size_t Encoder::MaxEncodedSize(uint64_t address, ConstByteSpan payload) {
constexpr size_t kFcsMaxSize = 8; // Worst case FCS: 0x7e7e7e7e.
- size_t encoded_address_size = NeedsEscaping(std::byte{address}) ? 2 : 1;
+ size_t max_encoded_address_size = varint::EncodedSize(address) * 2;
size_t encoded_payload_size =
payload.size() +
std::count_if(payload.begin(), payload.end(), NeedsEscaping);
- return encoded_address_size + encoded_payload_size + kFcsMaxSize;
+ return max_encoded_address_size + sizeof(kUnusedControl) +
+ encoded_payload_size + kFcsMaxSize;
+}
+
+Status Encoder::StartFrame(uint64_t address, std::byte control) {
+ fcs_.clear();
+ if (Status status = writer_.Write(kFlag); !status.ok()) {
+ return status;
+ }
+
+ std::array<std::byte, 16> metadata_buffer;
+ size_t metadata_size =
+ varint::Encode(address, metadata_buffer, kAddressFormat);
+ if (metadata_size == 0) {
+ return Status::InvalidArgument();
+ }
+
+ metadata_buffer[metadata_size++] = control;
+ return WriteData(std::span(metadata_buffer).first(metadata_size));
}
} // namespace internal
-Status WriteUIFrame(uint8_t address,
+Status WriteUIFrame(uint64_t address,
ConstByteSpan payload,
stream::Writer& writer) {
if (internal::Encoder::MaxEncodedSize(address, payload) >
diff --git a/pw_hdlc/encoder_test.cc b/pw_hdlc/encoder_test.cc
index ead3bfc..e5359a7 100644
--- a/pw_hdlc/encoder_test.cc
+++ b/pw_hdlc/encoder_test.cc
@@ -21,7 +21,7 @@
#include "gtest/gtest.h"
#include "pw_bytes/array.h"
#include "pw_hdlc/internal/encoder.h"
-#include "pw_hdlc_private/protocol.h"
+#include "pw_hdlc/internal/protocol.h"
#include "pw_stream/memory_stream.h"
using std::byte;
@@ -30,6 +30,7 @@
namespace {
constexpr uint8_t kAddress = 0x7B; // 123
+constexpr uint8_t kEncodedAddress = (kAddress << 1) | 1;
#define EXPECT_ENCODER_WROTE(...) \
do { \
@@ -54,39 +55,46 @@
TEST_F(WriteUnnumberedFrame, EmptyPayload) {
ASSERT_EQ(OkStatus(), WriteUIFrame(kAddress, std::span<byte>(), writer_));
EXPECT_ENCODER_WROTE(bytes::Concat(
- kFlag, kAddress, kUnnumberedControl, uint32_t{0x141BE378}, kFlag));
+ kFlag, kEncodedAddress, kUnnumberedControl, uint32_t{0x832d343f}, kFlag));
}
TEST_F(WriteUnnumberedFrame, OneBytePayload) {
ASSERT_EQ(OkStatus(), WriteUIFrame(kAddress, bytes::String("A"), writer_));
- EXPECT_ENCODER_WROTE(bytes::Concat(
- kFlag, kAddress, kUnnumberedControl, 'A', uint32_t{0x8D137C66}, kFlag));
+ EXPECT_ENCODER_WROTE(bytes::Concat(kFlag,
+ kEncodedAddress,
+ kUnnumberedControl,
+ 'A',
+ uint32_t{0x653c9e82},
+ kFlag));
}
TEST_F(WriteUnnumberedFrame, OneBytePayload_Escape0x7d) {
ASSERT_EQ(OkStatus(), WriteUIFrame(kAddress, bytes::Array<0x7d>(), writer_));
EXPECT_ENCODER_WROTE(bytes::Concat(kFlag,
- kAddress,
+ kEncodedAddress,
kUnnumberedControl,
kEscape,
byte{0x7d} ^ byte{0x20},
- uint32_t{0xA27C00E1},
+ uint32_t{0x4a53e205},
kFlag));
}
TEST_F(WriteUnnumberedFrame, OneBytePayload_Escape0x7E) {
ASSERT_EQ(OkStatus(), WriteUIFrame(kAddress, bytes::Array<0x7e>(), writer_));
EXPECT_ENCODER_WROTE(bytes::Concat(kFlag,
- kAddress,
+ kEncodedAddress,
kUnnumberedControl,
kEscape,
byte{0x7e} ^ byte{0x20},
- uint32_t{0x3B75515B},
+ uint32_t{0xd35ab3bf},
kFlag));
}
TEST_F(WriteUnnumberedFrame, AddressNeedsEscaping) {
- ASSERT_EQ(OkStatus(), WriteUIFrame(0x7d, bytes::String("A"), writer_));
+ // Becomes 0x7d when encoded.
+ constexpr uint8_t kEscapeRequiredAddress = 0x7d >> 1;
+ ASSERT_EQ(OkStatus(),
+ WriteUIFrame(kEscapeRequiredAddress, bytes::String("A"), writer_));
EXPECT_ENCODER_WROTE(bytes::Concat(kFlag,
kEscape,
byte{0x5d},
@@ -97,14 +105,15 @@
}
TEST_F(WriteUnnumberedFrame, Crc32NeedsEscaping) {
- ASSERT_EQ(OkStatus(), WriteUIFrame(kAddress, bytes::String("a"), writer_));
+ ASSERT_EQ(OkStatus(), WriteUIFrame(kAddress, bytes::String("aa"), writer_));
- // The CRC-32 is 0xB67D5CAE, so the 0x7D must be escaped.
- constexpr auto expected_crc32 = bytes::Array<0xae, 0x5c, 0x7d, 0x5d, 0xb6>();
+ // The CRC-32 of {kEncodedAddress, kUnnumberedControl, "aa"} is 0x7ee04473, so
+ // the 0x7e must be escaped.
+ constexpr auto expected_crc32 = bytes::Array<0x73, 0x44, 0xe0, 0x7d, 0x5e>();
EXPECT_ENCODER_WROTE(bytes::Concat(kFlag,
- kAddress,
+ kEncodedAddress,
kUnnumberedControl,
- bytes::String("a"),
+ bytes::String("aa"),
expected_crc32,
kFlag));
}
@@ -113,16 +122,16 @@
ASSERT_EQ(OkStatus(), WriteUIFrame(kAddress, bytes::String("ABC"), writer_));
ASSERT_EQ(OkStatus(), WriteUIFrame(kAddress, bytes::String("DEF"), writer_));
EXPECT_ENCODER_WROTE(bytes::Concat(kFlag,
- kAddress,
+ kEncodedAddress,
kUnnumberedControl,
bytes::String("ABC"),
- uint32_t{0x06575377},
+ uint32_t{0x72410ee4},
kFlag,
kFlag,
- kAddress,
+ kEncodedAddress,
kUnnumberedControl,
bytes::String("DEF"),
- uint32_t{0x3FB7F3D4},
+ uint32_t{0x4ba1ae47},
kFlag));
}
@@ -132,10 +141,21 @@
WriteUIFrame(kAddress, bytes::String("1995 toyota corolla"), writer_));
EXPECT_ENCODER_WROTE(bytes::Concat(kFlag,
- kAddress,
+ kEncodedAddress,
kUnnumberedControl,
bytes::String("1995 toyota corolla"),
- uint32_t{0x56560172},
+ uint32_t{0x53ee911c},
+ kFlag));
+}
+
+TEST_F(WriteUnnumberedFrame, MultibyteAddress) {
+ ASSERT_EQ(OkStatus(), WriteUIFrame(0x3fff, bytes::String("abc"), writer_));
+
+ EXPECT_ENCODER_WROTE(bytes::Concat(kFlag,
+ bytes::String("\xfe\xff"),
+ kUnnumberedControl,
+ bytes::String("abc"),
+ uint32_t{0x8cee2978},
kFlag));
}
@@ -147,11 +167,11 @@
writer_));
EXPECT_ENCODER_WROTE(bytes::Concat(
kFlag,
- kAddress,
+ kEncodedAddress,
kUnnumberedControl,
bytes::
Array<0x7D, 0x5E, 0x7B, 0x61, 0x62, 0x63, 0x7D, 0x5D, 0x7D, 0x5E>(),
- uint32_t{0x950257BD},
+ uint32_t{0x1563a4e6},
kFlag));
}
@@ -180,26 +200,26 @@
constexpr uint8_t kEscapeAddress = 0x7d;
TEST(Encoder, MaxEncodedSize_EmptyPayload) {
- EXPECT_EQ(9u, Encoder::MaxEncodedSize(kAddress, {}));
- EXPECT_EQ(10u, Encoder::MaxEncodedSize(kEscapeAddress, {}));
+ EXPECT_EQ(11u, Encoder::MaxEncodedSize(kAddress, {}));
+ EXPECT_EQ(11u, Encoder::MaxEncodedSize(kEscapeAddress, {}));
}
TEST(Encoder, MaxEncodedSize_PayloadWithoutEscapes) {
constexpr auto data = bytes::Array<0x00, 0x01, 0x02, 0x03>();
- EXPECT_EQ(13u, Encoder::MaxEncodedSize(kAddress, data));
- EXPECT_EQ(14u, Encoder::MaxEncodedSize(kEscapeAddress, data));
+ EXPECT_EQ(15u, Encoder::MaxEncodedSize(kAddress, data));
+ EXPECT_EQ(15u, Encoder::MaxEncodedSize(kEscapeAddress, data));
}
TEST(Encoder, MaxEncodedSize_PayloadWithOneEscape) {
constexpr auto data = bytes::Array<0x00, 0x01, 0x7e, 0x03>();
- EXPECT_EQ(14u, Encoder::MaxEncodedSize(kAddress, data));
- EXPECT_EQ(15u, Encoder::MaxEncodedSize(kEscapeAddress, data));
+ EXPECT_EQ(16u, Encoder::MaxEncodedSize(kAddress, data));
+ EXPECT_EQ(16u, Encoder::MaxEncodedSize(kEscapeAddress, data));
}
TEST(Encoder, MaxEncodedSize_PayloadWithAllEscapes) {
constexpr auto data = bytes::Initialized<8>(0x7e);
- EXPECT_EQ(25u, Encoder::MaxEncodedSize(kAddress, data));
- EXPECT_EQ(26u, Encoder::MaxEncodedSize(kEscapeAddress, data));
+ EXPECT_EQ(27u, Encoder::MaxEncodedSize(kAddress, data));
+ EXPECT_EQ(27u, Encoder::MaxEncodedSize(kEscapeAddress, data));
}
} // namespace
diff --git a/pw_hdlc/public/pw_hdlc/decoder.h b/pw_hdlc/public/pw_hdlc/decoder.h
index fd9b909..b956c0e 100644
--- a/pw_hdlc/public/pw_hdlc/decoder.h
+++ b/pw_hdlc/public/pw_hdlc/decoder.h
@@ -30,39 +30,39 @@
// flag bytes. Instances of Frame are only created when a full, valid frame has
// been read.
//
-// For now, the Frame class assumes single-byte address and control fields and a
-// 32-bit frame check sequence (FCS).
+// For now, the Frame class assumes a single-byte control field and a 32-bit
+// frame check sequence (FCS).
class Frame {
private:
- static constexpr size_t kAddressSize = 1;
+ static constexpr size_t kMinimumAddressSize = 1;
static constexpr size_t kControlSize = 1;
static constexpr size_t kFcsSize = sizeof(uint32_t);
public:
// The minimum size of a frame, excluding control bytes (flag or escape).
static constexpr size_t kMinSizeBytes =
- kAddressSize + kControlSize + kFcsSize;
+ kMinimumAddressSize + kControlSize + kFcsSize;
+ static Result<Frame> Parse(ConstByteSpan frame);
+
+ constexpr uint64_t address() const { return address_; }
+
+ constexpr std::byte control() const { return control_; }
+
+ constexpr ConstByteSpan data() const { return data_; }
+
+ private:
// Creates a Frame with the specified data. The data MUST be valid frame data
// with a verified frame check sequence.
- explicit constexpr Frame(ConstByteSpan data) : frame_(data) {
+ constexpr Frame(uint64_t address, std::byte control, ConstByteSpan data)
+ : data_(data), address_(address), control_(control) {
// TODO(pwbug/246): Use PW_DASSERT when available.
// PW_DASSERT(data.size() >= kMinSizeBytes);
}
- constexpr unsigned address() const {
- return std::to_integer<unsigned>(frame_[0]);
- }
-
- constexpr std::byte control() const { return frame_[kAddressSize]; }
-
- constexpr ConstByteSpan data() const {
- return frame_.subspan(kAddressSize + kControlSize,
- frame_.size() - kMinSizeBytes);
- }
-
- private:
- ConstByteSpan frame_;
+ ConstByteSpan data_;
+ uint64_t address_;
+ std::byte control_;
};
// The Decoder class facilitates decoding of data frames using the HDLC
diff --git a/pw_hdlc/public/pw_hdlc/encoder.h b/pw_hdlc/public/pw_hdlc/encoder.h
index e885c5e..8dcf66f 100644
--- a/pw_hdlc/public/pw_hdlc/encoder.h
+++ b/pw_hdlc/public/pw_hdlc/encoder.h
@@ -20,16 +20,16 @@
namespace pw::hdlc {
// Writes an HDLC unnumbered information frame (UI-frame) to the provided
-// writer. The frame contains the following:
+// writer. The complete frame contains the following:
//
// - HDLC flag byte (0x7e)
// - Address
-// - UI-frame control byte
-// - Data (0 or more bytes)
+// - UI-frame control (metadata) byte
+// - Payload (0 or more bytes)
// - Frame check sequence (CRC-32)
// - HDLC flag byte (0x7e)
//
-Status WriteUIFrame(uint8_t address,
+Status WriteUIFrame(uint64_t address,
ConstByteSpan payload,
stream::Writer& writer);
diff --git a/pw_hdlc/public/pw_hdlc/internal/encoder.h b/pw_hdlc/public/pw_hdlc/internal/encoder.h
index a7a61ad..2f3b4b0 100644
--- a/pw_hdlc/public/pw_hdlc/internal/encoder.h
+++ b/pw_hdlc/public/pw_hdlc/internal/encoder.h
@@ -14,6 +14,7 @@
#pragma once
#include "pw_checksum/crc32.h"
+#include "pw_hdlc/internal/protocol.h"
#include "pw_stream/stream.h"
namespace pw::hdlc::internal {
@@ -25,11 +26,15 @@
// Writes the header for an I-frame. After successfully calling
// StartInformationFrame, WriteData may be called any number of times.
- [[maybe_unused]] Status StartInformationFrame(uint8_t address);
+ Status StartInformationFrame(uint64_t address) {
+ return StartFrame(address, kUnusedControl);
+ }
// Writes the header for an U-frame. After successfully calling
// StartUnnumberedFrame, WriteData may be called any number of times.
- Status StartUnnumberedFrame(uint8_t address);
+ Status StartUnnumberedFrame(uint64_t address) {
+ return StartFrame(address, UFrameControl::UnnumberedInformation().data());
+ }
// Writes data for an ongoing frame. Must only be called after a successful
// StartInformationFrame call, and prior to a FinishFrame() call.
@@ -40,9 +45,14 @@
// Runs a pass through a payload, returning the worst-case encoded size for a
// frame containing it. Does not calculate CRC to improve efficiency.
- static size_t MaxEncodedSize(uint8_t address, ConstByteSpan payload);
+ static size_t MaxEncodedSize(uint64_t address, ConstByteSpan payload);
private:
+ // Indicates this an information packet with sequence numbers set to 0.
+ static constexpr std::byte kUnusedControl = std::byte{0};
+
+ Status StartFrame(uint64_t address, std::byte control);
+
stream::Writer& writer_;
checksum::Crc32 fcs_;
};
diff --git a/pw_hdlc/pw_hdlc_private/protocol.h b/pw_hdlc/public/pw_hdlc/internal/protocol.h
similarity index 93%
rename from pw_hdlc/pw_hdlc_private/protocol.h
rename to pw_hdlc/public/pw_hdlc/internal/protocol.h
index 1e6e210..f11b34f 100644
--- a/pw_hdlc/pw_hdlc_private/protocol.h
+++ b/pw_hdlc/public/pw_hdlc/internal/protocol.h
@@ -15,6 +15,8 @@
#include <cstddef>
+#include "pw_varint/varint.h"
+
namespace pw::hdlc {
inline constexpr std::byte kFlag = std::byte{0x7E};
@@ -26,6 +28,9 @@
inline constexpr std::array<std::byte, 2> kEscapedEscape = {kEscape,
std::byte{0x5D}};
+inline constexpr varint::Format kAddressFormat =
+ varint::Format::kOneTerminatedLeastSignificant;
+
constexpr bool NeedsEscaping(std::byte b) {
return (b == kFlag || b == kEscape);
}
diff --git a/pw_hdlc/public/pw_hdlc/rpc_channel.h b/pw_hdlc/public/pw_hdlc/rpc_channel.h
index 84e2a47..f2210cb 100644
--- a/pw_hdlc/public/pw_hdlc/rpc_channel.h
+++ b/pw_hdlc/public/pw_hdlc/rpc_channel.h
@@ -35,7 +35,7 @@
// a writer object to which will be used to write and send the bytes.
constexpr RpcChannelOutput(stream::Writer& writer,
std::span<std::byte> buffer,
- uint8_t address,
+ uint64_t address,
const char* channel_name)
: ChannelOutput(channel_name),
writer_(writer),
@@ -55,7 +55,7 @@
private:
stream::Writer& writer_;
const std::span<std::byte> buffer_;
- const uint8_t address_;
+ const uint64_t address_;
};
// RpcChannelOutput with its own buffer.
@@ -66,7 +66,7 @@
class RpcChannelOutputBuffer : public rpc::ChannelOutput {
public:
constexpr RpcChannelOutputBuffer(stream::Writer& writer,
- uint8_t address,
+ uint64_t address,
const char* channel_name)
: ChannelOutput(channel_name), writer_(writer), address_(address) {}
@@ -83,7 +83,7 @@
private:
stream::Writer& writer_;
std::array<std::byte, buffer_size> buffer_;
- const uint8_t address_;
+ const uint64_t address_;
};
} // namespace pw::hdlc
diff --git a/pw_hdlc/public/pw_hdlc/wire_packet_parser.h b/pw_hdlc/public/pw_hdlc/wire_packet_parser.h
index 7187773..cf83a0c 100644
--- a/pw_hdlc/public/pw_hdlc/wire_packet_parser.h
+++ b/pw_hdlc/public/pw_hdlc/wire_packet_parser.h
@@ -13,33 +13,31 @@
// the License.
#pragma once
-#include "pw_assert/light.h"
#include "pw_router/packet_parser.h"
namespace pw::hdlc {
// HDLC frame parser for routers that operates on wire-encoded frames.
//
-// Currently, this assumes 1-byte HDLC address fields. An optional address_bits
-// value can be provided to the constructor to use a smaller address size.
-class WirePacketParser final : public router::PacketParser {
+// This allows routing HDLC frames through Pigweed routers without having to
+// first decode them from their wire format.
+class WirePacketParser : public router::PacketParser {
public:
- constexpr WirePacketParser(uint8_t address_bits = 8)
- : address_(0), address_shift_(8 - address_bits) {
- PW_ASSERT(address_bits <= 8);
- }
+ constexpr WirePacketParser() : address_(0) {}
// Verifies and parses an HDLC frame. Packet passed in is expected to be a
// single, complete, wire-encoded frame, starting and ending with a flag.
bool Parse(ConstByteSpan packet) final;
- std::optional<uint32_t> GetDestinationAddress() const final {
+ std::optional<uint32_t> GetDestinationAddress() const override {
return address_;
}
+ protected:
+ constexpr uint64_t address() const { return address_; }
+
private:
- uint8_t address_;
- uint8_t address_shift_;
+ uint64_t address_;
};
} // namespace pw::hdlc
diff --git a/pw_hdlc/py/decode_test.py b/pw_hdlc/py/decode_test.py
index ff4b6b6..5aa8f90 100755
--- a/pw_hdlc/py/decode_test.py
+++ b/pw_hdlc/py/decode_test.py
@@ -21,10 +21,11 @@
from pw_build.generated_tests import parse_test_generation_args
from pw_hdlc.decode import Frame, FrameDecoder, FrameStatus, NO_ADDRESS
from pw_hdlc.protocol import frame_check_sequence as fcs
+from pw_hdlc.protocol import encode_address
def _encode(address: int, control: int, data: bytes) -> bytes:
- frame = bytearray([address, control]) + data
+ frame = encode_address(address) + bytes([control]) + data
frame += fcs(frame)
frame = frame.replace(b'\x7d', b'\x7d\x5d')
frame = frame.replace(b'\x7e', b'\x7d\x5e')
@@ -96,6 +97,10 @@
(_encode(0x7e, 0x7d, b'E'), [Expected(0x7e, b'\x7d', b'E')]),
(_encode(0x7d, 0x7e, b'F'), [Expected(0x7d, b'\x7e', b'F')]),
(_encode(0x7e, 0x7e, b'\x7e'), [Expected(0x7e, b'\x7e', b'\x7e')]),
+ 'Multibyte address',
+ (_encode(128, 0, b'big address'), [Expected(128, b'\0', b'big address')]),
+ (_encode(0xffffffff, 0, b'\0\0\1\0\0'),
+ [Expected(0xffffffff, b'\0', b'\0\0\1\0\0')]),
'Multiple frames separated by single flag',
(_encode(0, 0, b'A')[:-1] + _encode(1, 2, b'123'),
[Expected(0, b'\0', b'A'),
@@ -113,13 +118,16 @@
'Cannot escape flag',
(b'\x7e\xAA\x7d\x7e\xab\x00Hello' + fcs(b'\xab\0Hello') + b'\x7e', [
Expected.error(FrameStatus.FRAMING_ERROR),
- Expected(0xab, b'\0', b'Hello'),
+ Expected(0x55, b'\0', b'Hello'),
]),
_ESCAPED_FLAG_TEST_CASE,
'Frame too short',
(b'\x7e1\x7e', [Expected.error(FrameStatus.FRAMING_ERROR)]),
(b'\x7e12\x7e', [Expected.error(FrameStatus.FRAMING_ERROR)]),
(b'\x7e12345\x7e', [Expected.error(FrameStatus.FRAMING_ERROR)]),
+ 'Multibyte address too long',
+ (_encode(2 ** 100, 0, b'too long'),
+ [Expected.error(FrameStatus.BAD_ADDRESS)]),
'Incorrect frame check sequence',
(b'\x7e123456\x7e', [Expected.error(FrameStatus.FCS_MISMATCH)]),
(b'\x7e\1\2msg\xff\xff\xff\xff\x7e',
@@ -185,7 +193,9 @@
def _expected(frames: List[Frame]) -> Iterator[str]:
for i, frame in enumerate(frames, 1):
if frame.ok():
- yield f' Frame(kDecodedFrame{i:02}),'
+ yield f' Frame::Parse(kDecodedFrame{i:02}).value(),'
+ elif frame.status is FrameStatus.BAD_ADDRESS:
+ yield f' Frame::Parse(kDecodedFrame{i:02}).status(),'
else:
yield f' Status::DataLoss(), // Frame {i}'
@@ -219,7 +229,7 @@
yield f' static constexpr auto kData = bytes::String("{data_bytes}");\n'
for i, frame in enumerate(frames, 1):
- if frame.status is FrameStatus.OK:
+ if frame.ok() or frame.status is FrameStatus.BAD_ADDRESS:
frame_bytes = ''.join(rf'\x{byte:02x}'
for byte in frame.raw_decoded)
yield (f' static constexpr auto kDecodedFrame{i:02} = '
@@ -235,7 +245,7 @@
yield f"""\
DecoderBuffer<{decoder_size}> decoder;
- static constexpr std::array<std::variant<Frame, Status>, {len(frames)}> kExpected = {{
+ static std::array<std::variant<Frame, Status>, {len(frames)}> kExpected = {{
{expected}
}};
diff --git a/pw_hdlc/py/encode_test.py b/pw_hdlc/py/encode_test.py
index d09902a..fa48c58 100755
--- a/pw_hdlc/py/encode_test.py
+++ b/pw_hdlc/py/encode_test.py
@@ -31,24 +31,28 @@
"""Tests Encoding bytes with different arguments using a custom serial."""
def test_empty(self):
self.assertEqual(encode.ui_frame(0, b''),
- FLAG + _with_fcs(b'\0\x03') + FLAG)
+ FLAG + _with_fcs(b'\x01\x03') + FLAG)
self.assertEqual(encode.ui_frame(0x1a, b''),
- FLAG + _with_fcs(b'\x1a\x03') + FLAG)
+ FLAG + _with_fcs(b'\x35\x03') + FLAG)
def test_1byte(self):
self.assertEqual(encode.ui_frame(0, b'A'),
- FLAG + _with_fcs(b'\0\x03A') + FLAG)
+ FLAG + _with_fcs(b'\x01\x03A') + FLAG)
def test_multibyte(self):
self.assertEqual(encode.ui_frame(0, b'123456789'),
- FLAG + _with_fcs(b'\x00\x03123456789') + FLAG)
+ FLAG + _with_fcs(b'\x01\x03123456789') + FLAG)
+
+ def test_multibyte_address(self):
+ self.assertEqual(encode.ui_frame(128, b'123456789'),
+ FLAG + _with_fcs(b'\x00\x03\x03123456789') + FLAG)
def test_escape(self):
self.assertEqual(
- encode.ui_frame(0x7e, b'\x7d'),
- FLAG + b'\x7d\x5e\x03\x7d\x5d' + _fcs(b'\x7e\x03\x7d') + FLAG)
+ encode.ui_frame(0x3e, b'\x7d'),
+ FLAG + b'\x7d\x5d\x03\x7d\x5d' + _fcs(b'\x7d\x03\x7d') + FLAG)
self.assertEqual(
- encode.ui_frame(0x7d, b'A\x7e\x7dBC'),
+ encode.ui_frame(0x3e, b'A\x7e\x7dBC'),
FLAG + b'\x7d\x5d\x03A\x7d\x5e\x7d\x5dBC' +
_fcs(b'\x7d\x03A\x7e\x7dBC') + FLAG)
diff --git a/pw_hdlc/py/pw_hdlc/decode.py b/pw_hdlc/py/pw_hdlc/decode.py
index d727bc3..3c5b487 100644
--- a/pw_hdlc/py/pw_hdlc/decode.py
+++ b/pw_hdlc/py/pw_hdlc/decode.py
@@ -13,7 +13,6 @@
# the License.
"""Decoder class for decoding bytes using HDLC protocol"""
-from dataclasses import dataclass
import enum
import logging
from typing import Iterator, Optional
@@ -32,35 +31,41 @@
OK = 'OK'
FCS_MISMATCH = 'frame check sequence failure'
FRAMING_ERROR = 'invalid flag or escape characters'
+ BAD_ADDRESS = 'address field too long'
-@dataclass(frozen=True)
class Frame:
"""Represents an HDLC frame."""
+ def __init__(self,
+ raw_encoded: bytes,
+ raw_decoded: bytes,
+ status: FrameStatus = FrameStatus.OK):
+ """Parses fields from an HDLC frame.
- # The complete HDLC-encoded frame, excluding HDLC flag characters.
- raw_encoded: bytes
+ Arguments:
+ raw_encoded: The complete HDLC-encoded frame, excluding HDLC flag
+ characters.
+ raw_decoded: The complete decoded frame (address, control,
+ information, FCS).
+ status: Whether parsing the frame succeeded.
+ """
+ self.raw_encoded = raw_encoded
+ self.raw_decoded = raw_decoded
+ self.status = status
- # The complete decoded frame (address, control, information, FCS).
- raw_decoded: bytes
+ self.address: int = NO_ADDRESS
+ self.control: bytes = b''
+ self.data: bytes = b''
- # Whether parsing the frame succeeded.
- status: FrameStatus = FrameStatus.OK
+ if status == FrameStatus.OK:
+ address, address_length = protocol.decode_address(raw_decoded)
+ if address_length == 0:
+ self.status = FrameStatus.BAD_ADDRESS
+ return
- @property
- def address(self) -> int:
- """The frame's address field (assumes only one byte for now)."""
- return self.raw_decoded[0] if self.ok() else NO_ADDRESS
-
- @property
- def control(self) -> bytes:
- """The control byte (assumes only one byte for now)."""
- return self.raw_decoded[1:2] if self.ok() else b''
-
- @property
- def data(self) -> bytes:
- """The information field in the frame."""
- return self.raw_decoded[2:-4] if self.ok() else b''
+ self.address = address
+ self.control = raw_decoded[address_length:address_length + 1]
+ self.data = raw_decoded[address_length + 1:-4]
def ok(self) -> bool:
"""True if this represents a valid frame.
diff --git a/pw_hdlc/py/pw_hdlc/encode.py b/pw_hdlc/py/pw_hdlc/encode.py
index ea6ec06..ceede92 100644
--- a/pw_hdlc/py/pw_hdlc/encode.py
+++ b/pw_hdlc/py/pw_hdlc/encode.py
@@ -21,9 +21,8 @@
def ui_frame(address: int, data: bytes) -> bytes:
"""Encodes an HDLC UI-frame with a CRC-32 frame check sequence."""
- frame = bytearray([
- address
- ]) + protocol.UFrameControl.unnumbered_information().data + data
+ frame = protocol.encode_address(
+ address) + protocol.UFrameControl.unnumbered_information().data + data
frame += protocol.frame_check_sequence(frame)
frame = frame.replace(_ESCAPE_BYTE, b'\x7d\x5d')
frame = frame.replace(_FLAG_BYTE, b'\x7d\x5e')
diff --git a/pw_hdlc/py/pw_hdlc/protocol.py b/pw_hdlc/py/pw_hdlc/protocol.py
index a6dd83b..f12b79b 100644
--- a/pw_hdlc/py/pw_hdlc/protocol.py
+++ b/pw_hdlc/py/pw_hdlc/protocol.py
@@ -13,6 +13,8 @@
# the License.
"""Module for low-level HDLC protocol features."""
+from typing import Tuple
+
import zlib
# Special flag character for delimiting HDLC frames.
@@ -24,6 +26,9 @@
# Characters allowed after a 0x7d escape character.
VALID_ESCAPED_BYTES = 0x5D, 0x5E
+# Maximum allowed HDLC address (uint64_t in C++).
+MAX_ADDRESS = 2**64 - 1
+
def escape(byte: int) -> int:
"""Escapes or unescapes a byte, which should have been preceeded by 0x7d."""
@@ -34,6 +39,40 @@
return zlib.crc32(data).to_bytes(4, 'little')
+def encode_address(address: int) -> bytes:
+ """Encodes an HDLC address as a one-terminated LSB varint."""
+ result = bytearray()
+
+ while True:
+ result += bytes([(address & 0x7f) << 1])
+
+ address >>= 7
+ if address == 0:
+ break
+
+ result[-1] |= 0x1
+ return result
+
+
+def decode_address(frame: bytes) -> Tuple[int, int]:
+ """Decodes an HDLC address from a frame, returning it and its size."""
+ result = 0
+ length = 0
+
+ while length < len(frame):
+ byte = frame[length]
+ result |= (byte >> 1) << (length * 7)
+ length += 1
+
+ if byte & 0x1 == 0x1:
+ break
+
+ if result > MAX_ADDRESS:
+ return -1, 0
+
+ return result, length
+
+
class UFrameControl:
def __init__(self, frame_type: int):
self._data: bytes = bytes([0x03 | frame_type])
diff --git a/pw_hdlc/rpc_channel_test.cc b/pw_hdlc/rpc_channel_test.cc
index efc95a4..538f699 100644
--- a/pw_hdlc/rpc_channel_test.cc
+++ b/pw_hdlc/rpc_channel_test.cc
@@ -42,7 +42,8 @@
namespace {
constexpr byte kFlag = byte{0x7E};
-constexpr uint8_t kAddress = 0x7b; // 123
+constexpr uint8_t kAddress = 0x7b; // 123
+constexpr uint8_t kEncodedAddress = (kAddress << 1) | 1;
constexpr byte kControl = byte{0x3}; // UI-frame control sequence.
// Size of the in-memory buffer to use for this test.
@@ -60,7 +61,7 @@
std::memcpy(buffer.data(), &test_data, sizeof(test_data));
constexpr auto expected = bytes::Concat(
- kFlag, kAddress, kControl, 'A', uint32_t{0x8D137C66}, kFlag);
+ kFlag, kEncodedAddress, kControl, 'A', uint32_t{0x653c9e82}, kFlag);
EXPECT_EQ(OkStatus(),
output.SendAndReleaseBuffer(buffer.first(sizeof(test_data))));
@@ -84,11 +85,11 @@
std::memcpy(buffer.data(), test_data.data(), test_data.size());
constexpr auto expected = bytes::Concat(kFlag,
- kAddress,
+ kEncodedAddress,
kControl,
byte{0x7d},
byte{0x7d} ^ byte{0x20},
- uint32_t{0xA27C00E1},
+ uint32_t{0x4a53e205},
kFlag);
EXPECT_EQ(OkStatus(),
output.SendAndReleaseBuffer(buffer.first(test_data.size())));
@@ -111,7 +112,34 @@
std::memcpy(buffer.data(), &test_data, sizeof(test_data));
constexpr auto expected = bytes::Concat(
- kFlag, kAddress, kControl, 'A', uint32_t{0x8D137C66}, kFlag);
+ kFlag, kEncodedAddress, kControl, 'A', uint32_t{0x653c9e82}, kFlag);
+
+ EXPECT_EQ(OkStatus(),
+ output.SendAndReleaseBuffer(buffer.first(sizeof(test_data))));
+
+ ASSERT_EQ(memory_writer.bytes_written(), expected.size());
+ EXPECT_EQ(
+ std::memcmp(
+ memory_writer.data(), expected.data(), memory_writer.bytes_written()),
+ 0);
+}
+
+TEST(RpcChannelOutputBuffer, MultibyteAddress) {
+ stream::MemoryWriterBuffer<kSinkBufferSize> memory_writer;
+
+ RpcChannelOutputBuffer<kSinkBufferSize> output(
+ memory_writer, 0x3fff, "RpcChannelOutput");
+
+ constexpr byte test_data = byte{'A'};
+ auto buffer = output.AcquireBuffer();
+ std::memcpy(buffer.data(), &test_data, sizeof(test_data));
+
+ constexpr auto expected = bytes::Concat(kFlag,
+ bytes::String("\xfe\xff"),
+ kControl,
+ 'A',
+ uint32_t{0xd393a8a0},
+ kFlag);
EXPECT_EQ(OkStatus(),
output.SendAndReleaseBuffer(buffer.first(sizeof(test_data))));
diff --git a/pw_hdlc/wire_packet_parser.cc b/pw_hdlc/wire_packet_parser.cc
index 5ced2fb..992dfde 100644
--- a/pw_hdlc/wire_packet_parser.cc
+++ b/pw_hdlc/wire_packet_parser.cc
@@ -17,7 +17,7 @@
#include "pw_bytes/endian.h"
#include "pw_checksum/crc32.h"
#include "pw_hdlc/decoder.h"
-#include "pw_hdlc_private/protocol.h"
+#include "pw_hdlc/internal/protocol.h"
namespace pw::hdlc {
@@ -30,10 +30,9 @@
return false;
}
- // Partially decode into a buffer with space only for the first two bytes
- // (address and control) of the frame. The decoder will verify the frame's
- // FCS field.
- std::array<std::byte, 2> buffer = {};
+ // Partially decode into a buffer with space only for the address and control
+ // fields of the frame. The decoder will verify the frame's FCS field.
+ std::array<std::byte, 16> buffer = {};
Decoder decoder(buffer);
Status status = Status::Unknown();
@@ -41,8 +40,12 @@
status = result.status();
});
- // Address is the first byte in the packet.
- address_ = static_cast<uint8_t>(buffer[0]) >> address_shift_;
+ Result<Frame> result = Frame::Parse(buffer);
+ if (!result.ok()) {
+ return false;
+ }
+
+ address_ = result.value().address();
// RESOURCE_EXHAUSTED is expected as the buffer is too small for the packet.
return status.ok() || status.IsResourceExhausted();
diff --git a/pw_hdlc/wire_packet_parser_test.cc b/pw_hdlc/wire_packet_parser_test.cc
index 4602e85..809afd2 100644
--- a/pw_hdlc/wire_packet_parser_test.cc
+++ b/pw_hdlc/wire_packet_parser_test.cc
@@ -16,46 +16,64 @@
#include "gtest/gtest.h"
#include "pw_bytes/array.h"
-#include "pw_hdlc_private/protocol.h"
+#include "pw_hdlc/internal/protocol.h"
namespace pw::hdlc {
namespace {
constexpr uint8_t kAddress = 0x6a;
+constexpr uint8_t kEncodedAddress = (kAddress << 1) | 0x1;
constexpr uint8_t kControl = 0x03;
TEST(WirePacketParser, Parse_ValidPacket) {
WirePacketParser parser;
- EXPECT_TRUE(parser.Parse(bytes::Concat(
- kFlag, kAddress, kControl, bytes::String("hello"), 0xc40cefe0, kFlag)));
+ EXPECT_TRUE(parser.Parse(bytes::Concat(kFlag,
+ kEncodedAddress,
+ kControl,
+ bytes::String("hello"),
+ 0x1231d0a9,
+ kFlag)));
auto maybe_address = parser.GetDestinationAddress();
EXPECT_TRUE(maybe_address.has_value());
EXPECT_EQ(maybe_address.value(), kAddress);
}
+TEST(WirePacketParser, Parse_MultibyteAddress) {
+ WirePacketParser parser;
+ EXPECT_TRUE(parser.Parse(bytes::Concat(kFlag,
+ bytes::String("\xfe\xff"),
+ kControl,
+ bytes::String("hello"),
+ 0x6b53b014,
+ kFlag)));
+ auto maybe_address = parser.GetDestinationAddress();
+ EXPECT_TRUE(maybe_address.has_value());
+ EXPECT_EQ(maybe_address.value(), 0x3fffu);
+}
+
TEST(WirePacketParser, Parse_EscapedAddress) {
WirePacketParser parser;
EXPECT_TRUE(parser.Parse(bytes::Concat(kFlag,
kEscape,
- uint8_t{0x7e ^ 0x20},
+ uint8_t{0x7d ^ 0x20},
kControl,
bytes::String("hello"),
- 0x579d573d,
+ 0x66754da0,
kFlag)));
auto maybe_address = parser.GetDestinationAddress();
EXPECT_TRUE(maybe_address.has_value());
- EXPECT_EQ(maybe_address.value(), 0x7eu);
+ EXPECT_EQ(maybe_address.value(), 62u);
}
TEST(WirePacketParser, Parse_EscapedPayload) {
WirePacketParser parser;
EXPECT_TRUE(parser.Parse(bytes::Concat(kFlag,
- kAddress,
+ kEncodedAddress,
kControl,
bytes::String("hello"),
kEscapedEscape,
bytes::String("world"),
- 0x9a9b934a,
+ 0x1af88e47,
kFlag)));
auto maybe_address = parser.GetDestinationAddress();
EXPECT_TRUE(maybe_address.has_value());
@@ -64,13 +82,14 @@
TEST(WirePacketParser, Parse_EscapedFcs) {
WirePacketParser parser;
- EXPECT_TRUE(parser.Parse(bytes::Concat(kFlag,
- kAddress,
- kControl,
- bytes::String("qwertyu"),
- // FCS: e0 7d e9 3b
- bytes::String("\x3b\xe9\x7d\x5d\xe0"),
- kFlag)));
+ EXPECT_TRUE(
+ parser.Parse(bytes::Concat(kFlag,
+ kEncodedAddress,
+ kControl,
+ uint8_t{'b'},
+ // FCS: fc 92 7d 7e
+ bytes::String("\x7d\x5e\x7d\x5d\x92\xfc"),
+ kFlag)));
auto maybe_address = parser.GetDestinationAddress();
EXPECT_TRUE(maybe_address.has_value());
EXPECT_EQ(maybe_address.value(), kAddress);
@@ -79,47 +98,46 @@
TEST(WirePacketParser, Parse_MultipleEscapes) {
WirePacketParser parser;
EXPECT_TRUE(parser.Parse(bytes::Concat(kFlag,
- kEscapedFlag,
+ kEscapedEscape,
kControl,
kEscapedEscape,
kEscapedFlag,
kEscapedFlag,
- 0xc85df51d,
+ 0x8ffd8fcd,
kFlag)));
auto maybe_address = parser.GetDestinationAddress();
EXPECT_TRUE(maybe_address.has_value());
- EXPECT_EQ(maybe_address.value(), static_cast<uint32_t>(kFlag));
-}
-
-TEST(WirePacketParser, Parse_AddressBits) {
- WirePacketParser parser(4);
- EXPECT_TRUE(parser.Parse(bytes::Concat(kFlag,
- std::byte{0xab},
- kControl,
- bytes::String("hello"),
- 0xae667bdf,
- kFlag)));
- auto maybe_address = parser.GetDestinationAddress();
- EXPECT_TRUE(maybe_address.has_value());
- EXPECT_EQ(maybe_address.value(), 0xau);
+ EXPECT_EQ(maybe_address.value(), 62u);
}
TEST(WirePacketParser, Parse_BadFcs) {
WirePacketParser parser;
- EXPECT_FALSE(parser.Parse(bytes::Concat(
- kFlag, kAddress, kControl, bytes::String("hello"), 0x1badda7a, kFlag)));
+ EXPECT_FALSE(parser.Parse(bytes::Concat(kFlag,
+ kEncodedAddress,
+ kControl,
+ bytes::String("hello"),
+ 0x1badda7a,
+ kFlag)));
}
TEST(WirePacketParser, Parse_DoubleEscape) {
WirePacketParser parser;
- EXPECT_FALSE(parser.Parse(bytes::Concat(
- kFlag, kAddress, kControl, bytes::String("hello"), 0x01027d7d, kFlag)));
+ EXPECT_FALSE(parser.Parse(bytes::Concat(kFlag,
+ kEncodedAddress,
+ kControl,
+ bytes::String("hello"),
+ 0x01027d7d,
+ kFlag)));
}
TEST(WirePacketParser, Parse_FlagInFrame) {
WirePacketParser parser;
- EXPECT_FALSE(parser.Parse(bytes::Concat(
- kFlag, kAddress, kControl, bytes::String("he~lo"), 0xdbae98fe, kFlag)));
+ EXPECT_FALSE(parser.Parse(bytes::Concat(kFlag,
+ kEncodedAddress,
+ kControl,
+ bytes::String("he~lo"),
+ 0xdbae98fe,
+ kFlag)));
}
TEST(WirePacketParser, Parse_EmptyPacket) {
diff --git a/pw_varint/varint.cc b/pw_varint/varint.cc
index 648c1db..fcef43d 100644
--- a/pw_varint/varint.cc
+++ b/pw_varint/varint.cc
@@ -121,7 +121,7 @@
return count;
}
-// TODO(frolv): Remove this deprecated function.
+// TODO(frolv): Remove this deprecated alias.
extern "C" size_t pw_VarintEncode(uint64_t integer,
void* output,
size_t output_size) {
@@ -134,7 +134,7 @@
return pw_varint_Encode(ZigZagEncode(integer), output, output_size);
}
-// TODO(frolv): Remove this deprecated function.
+// TODO(frolv): Remove this deprecated alias.
extern "C" size_t pw_VarintDecode(const void* input,
size_t input_size,
uint64_t* output) {