From e1f394e90cc9b851ef0d190adf47a97093a3e42b Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 03:39:54 +0000 Subject: [PATCH 01/21] Starting a generic class to write parameter groups more simply. --- isobus/src/can_parameter_group_builder.hpp | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 isobus/src/can_parameter_group_builder.hpp diff --git a/isobus/src/can_parameter_group_builder.hpp b/isobus/src/can_parameter_group_builder.hpp new file mode 100644 index 000000000..0c891cdeb --- /dev/null +++ b/isobus/src/can_parameter_group_builder.hpp @@ -0,0 +1,10 @@ +// Note that this class currently only works for packets eight bytes or fewer. +class GroupBuilder +{ +private: + int bitOffset = 0; + int bytesUsed = 0; + char data[8]; + +}; + From 457aa1237422c19fccdc22115dda23ce334cacd9 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 04:08:39 +0000 Subject: [PATCH 02/21] CAN parameter group builder. --- isobus/src/can_parameter_group_builder.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 isobus/src/can_parameter_group_builder.cpp diff --git a/isobus/src/can_parameter_group_builder.cpp b/isobus/src/can_parameter_group_builder.cpp new file mode 100644 index 000000000..174c511ad --- /dev/null +++ b/isobus/src/can_parameter_group_builder.cpp @@ -0,0 +1,20 @@ +namespace isobus +{ + // Note that this class currently only works for packets eight bytes or fewer, because that is + // all the use-cases I needed. The spec has a whole section on how data can span byte + // boundaries, and how to align the bits when a data-type that is not a multiple of eight bits + // crosses said boundaries. Then every single parameter group uses padding to avoid those + // cases! The five-bit tractor command types have three bits of padding to make them exactly + // one byte. Why even specify the scheme if it isn't used? + // + // I wrote a huge amount of this file once, then somehow it vanished. I don't know how! + class GroupBuilder + { + private: + int bitOffset = 0; + int bytesUsed = 0; + char data[8]; + + }; +} + From f56f491c18b4b02d6a1d5c09abf292255847bf6c Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 05:45:28 +0000 Subject: [PATCH 03/21] Re-do the code I seemed to lose. --- isobus/src/can_parameter_group_builder.cpp | 160 ++++++++++++++++++++- isobus/src/can_parameter_group_builder.hpp | 24 +++- 2 files changed, 176 insertions(+), 8 deletions(-) diff --git a/isobus/src/can_parameter_group_builder.cpp b/isobus/src/can_parameter_group_builder.cpp index 174c511ad..b53ef003b 100644 --- a/isobus/src/can_parameter_group_builder.cpp +++ b/isobus/src/can_parameter_group_builder.cpp @@ -12,9 +12,167 @@ namespace isobus { private: int bitOffset = 0; - int bytesUsed = 0; char data[8]; + unsigned int get_byte_offset() const + { + // Which byte to write to. + return bitOffset / 8; + } + + unsigned int get_bit_offset() const + { + // Which bit to write to in the current byte. + return bitOffset % 8; + } + + bool write_bits(unsigned char const * data, unsigned int bits) + { + // First make a backup of the current write position, so that if there is an error + // writing some data we can roll back the entire change. This is replicated in the + // string writing code because it has to roll back the entire string, not just the last + // character. + unsigned int revert = bitOffset; + + return true; + } + + public: + template + bool write(T const & data) + { + return write_bits((unsigned char const *)&data, sizeof (T) * 8); + } + + template + bool write(T const & data, unsigned int bits) + { + return write_bits((unsigned char const *)&data, bits); + } + + template <> + bool write(bool const & data) + { + if (data) + { + unsigned char one = 255; + return write_bits(&one, 1); + } + else + { + unsigned char naught = 0; + return write_bits(&naught, 1); + } + } + + template <> + bool write(bool const & data) + { + // Only use a single bit for booleans. + if (data) + { + unsigned char one = 255; + return write_bits(&one, 1); + } + else + { + unsigned char naught = 0; + return write_bits(&naught, 1); + } + } + + template <> + bool write(char const * const & data) + { + // What should the default for including NULL be? + return write((unsigned char const *)data, false); + } + + template <> + bool write(char * const & data) + { + // What should the default for including NULL be? + return write((unsigned char const *)data, false); + } + + template <> + bool write(unsigned char const * const & data) + { + // What should the default for including NULL be? + return write((unsigned char const *)data, false); + } + + template <> + bool write(unsigned char * const & data) + { + // What should the default for including NULL be? + return write((unsigned char const *)data, false); + } + + bool write(char const * data, bool includeNull) + { + return write((unsigned char const *)data, includeNull); + } + + bool write(char * data, bool includeNull) + { + return write((unsigned char const *)data, includeNull); + } + + bool write(unsigned char const * data, bool includeNull) + { + // Base case. Write each byte separately so they don't get put in little-endian, which + // makes no sense for strings. + unsigned int revert = bitOffset; + while (*data) + { + if (!write_bits(data, 8)) + { + bitOffset = revert; + return false; + } + } + if (includeNull) + { + unsigned char naught = 0; + if (!write_bits(&naught, 8)) + { + bitOffset = revert; + return false; + } + } + return true; + } + + bool write(unsigned char * data, bool includeNull) + { + return write((unsigned char const *)data, includeNull); + } + + bool pad(unsigned int bits, bool value = true) + { + // Only use a single bit for booleans. + unsigned int revert = bitOffset; + unsigned char data = value ? 255 : 0; + while (bits > 8) + { + if (!write_bits(&data, 8)) + { + bitOffset = revert; + return false; + } + bits -= 8; + } + if (bits) + { + if (!write_bits(&data, bits)) + { + bitOffset = revert; + return false; + } + } + return true; + } }; } diff --git a/isobus/src/can_parameter_group_builder.hpp b/isobus/src/can_parameter_group_builder.hpp index 0c891cdeb..174c511ad 100644 --- a/isobus/src/can_parameter_group_builder.hpp +++ b/isobus/src/can_parameter_group_builder.hpp @@ -1,10 +1,20 @@ -// Note that this class currently only works for packets eight bytes or fewer. -class GroupBuilder +namespace isobus { -private: - int bitOffset = 0; - int bytesUsed = 0; - char data[8]; + // Note that this class currently only works for packets eight bytes or fewer, because that is + // all the use-cases I needed. The spec has a whole section on how data can span byte + // boundaries, and how to align the bits when a data-type that is not a multiple of eight bits + // crosses said boundaries. Then every single parameter group uses padding to avoid those + // cases! The five-bit tractor command types have three bits of padding to make them exactly + // one byte. Why even specify the scheme if it isn't used? + // + // I wrote a huge amount of this file once, then somehow it vanished. I don't know how! + class GroupBuilder + { + private: + int bitOffset = 0; + int bytesUsed = 0; + char data[8]; -}; + }; +} From ba0dfbddeaca7e957bef05070a80d3471f42539b Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 08:54:16 +0000 Subject: [PATCH 04/21] Write bits. --- isobus/src/can_parameter_group_builder.cpp | 147 +++++++++++++++++++-- 1 file changed, 134 insertions(+), 13 deletions(-) diff --git a/isobus/src/can_parameter_group_builder.cpp b/isobus/src/can_parameter_group_builder.cpp index b53ef003b..815cb75d4 100644 --- a/isobus/src/can_parameter_group_builder.cpp +++ b/isobus/src/can_parameter_group_builder.cpp @@ -11,33 +11,154 @@ namespace isobus class GroupBuilder { private: - int bitOffset = 0; - char data[8]; + int writeOffset = 0; + int readOffset = 0; + unsigned char buffer[8]; - unsigned int get_byte_offset() const + unsigned int get_write_byte_offset() const { // Which byte to write to. - return bitOffset / 8; + return writeOffset / 8; } - unsigned int get_bit_offset() const + unsigned int get_write_bit_offset() const { // Which bit to write to in the current byte. - return bitOffset % 8; + return writeOffset % 8; } bool write_bits(unsigned char const * data, unsigned int bits) { + if (bits == 0) + { + // Err, OK, I guess... + return true; + } // First make a backup of the current write position, so that if there is an error // writing some data we can roll back the entire change. This is replicated in the // string writing code because it has to roll back the entire string, not just the last // character. - unsigned int revert = bitOffset; + unsigned int revert = writeOffset; + unsigned int byte = get_write_byte_offset(); + unsigned int bit = get_write_bit_offset(); + // Adjust first, and revert later. + writeOffset += bits; + + // ------------------------------- + // Stage naught - trivial cases. + // ------------------------------- + + // How much space is there left in this byte? + unsigned int remaining = 8 - bit; + if (remaining >= bits) + { + if (byte == 8) + { + // Out of space for an eight byte packet. + writeOffset = revert; + return false; + } + // Everything will fit in the current byte, which must mean there's at most one byte + // of data to write. Hence we put this version first because it covers more single + // byte cases. + unsigned char mask = (1 << bit) - 1; + buffer[byte] = (buffer[byte] & mask) | (*data << bit); + return true; + } + + // Whole bytes (one byte is often handled above.) + if (bit == 0 && bits % 8 == 0) + { + // Whole bytes, which are aligned. + do + { + if (byte == 8) + { + // Out of space for an eight byte packet. + writeOffset = revert; + return false; + } + buffer[byte] = data++; + byte += 1; + bits -= 8; + } + while (bits); + return true; + } + + // ---------------------------------------- + // Stage one - fill out the current byte. + // ---------------------------------------- + + // All this data can come from the first byte of the input. + if (byte == 8) + { + // Out of space for an eight byte packet. + writeOffset = revert; + return false; + } + // Everything will fit in the current byte, which must mean there's at most one byte + // of data to write. Hence we put this version first because it covers more single + // byte cases. + unsigned char mask = (1 << bit) - 1; + buffer[byte] = (buffer[byte] & mask) | (*data << bit); + bits -= remaining; + ++byte; + + // ------------------------------- + // Stage two - copy whole bytes. + // ------------------------------- + + while (bits > 8) + { + if (byte == 8) + { + // Out of space for an eight byte packet. + writeOffset = revert; + return false; + } + buffer[byte] = (*data >> remaining); + ++data; + buffer[byte] = (buffer[byte] & mask) | (*data << bit); + bits -= 8; + ++byte; + } + + // ------------------------------- + // Stage three - copy whole bytes. + // ------------------------------- + + // `bits` is number of bits left. It may or may not span a byte boundary in the input. + if (bits == 0) + { + // Nothing left to copy. + return true; + } + + // I'm sure this code can be compressed with the code above. + if (byte == 8) + { + // Out of space for an eight byte packet. + writeOffset = revert; + return false; + } + buffer[byte] = (*data >> remaining); + if (bit < bits) + { + // The final output spans two bytes of input. + ++data; + buffer[byte] = (buffer[byte] & mask) | (*data << bit); + } return true; } public: + unsigned int get_written_bits() const + { + return writeOffset; + } + template bool write(T const & data) { @@ -123,12 +244,12 @@ namespace isobus { // Base case. Write each byte separately so they don't get put in little-endian, which // makes no sense for strings. - unsigned int revert = bitOffset; + unsigned int revert = writeOffset; while (*data) { if (!write_bits(data, 8)) { - bitOffset = revert; + writeOffset = revert; return false; } } @@ -137,7 +258,7 @@ namespace isobus unsigned char naught = 0; if (!write_bits(&naught, 8)) { - bitOffset = revert; + writeOffset = revert; return false; } } @@ -152,13 +273,13 @@ namespace isobus bool pad(unsigned int bits, bool value = true) { // Only use a single bit for booleans. - unsigned int revert = bitOffset; + unsigned int revert = writeOffset; unsigned char data = value ? 255 : 0; while (bits > 8) { if (!write_bits(&data, 8)) { - bitOffset = revert; + writeOffset = revert; return false; } bits -= 8; @@ -167,7 +288,7 @@ namespace isobus { if (!write_bits(&data, bits)) { - bitOffset = revert; + writeOffset = revert; return false; } } From fd431936d1dcecaa52b0bd1f80d56e43aa50d339 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 09:00:02 +0000 Subject: [PATCH 05/21] Add ones-padding to written bytes, don't rely on rubbish data. --- isobus/src/can_parameter_group_builder.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/isobus/src/can_parameter_group_builder.cpp b/isobus/src/can_parameter_group_builder.cpp index 815cb75d4..ce67c5c93 100644 --- a/isobus/src/can_parameter_group_builder.cpp +++ b/isobus/src/can_parameter_group_builder.cpp @@ -63,6 +63,11 @@ namespace isobus // byte cases. unsigned char mask = (1 << bit) - 1; buffer[byte] = (buffer[byte] & mask) | (*data << bit); + if (writeOffset % 8 != 0) + { + // Mask the top bits to hide excess data. + buffer[byte] |= 255 << (writeOffset % 8); + } return true; } @@ -83,6 +88,7 @@ namespace isobus bits -= 8; } while (bits); + // No additional masking required in this case. return true; } @@ -132,6 +138,11 @@ namespace isobus if (bits == 0) { // Nothing left to copy. + if (writeOffset % 8 != 0) + { + // Mask the top bits to hide excess data. + buffer[byte] |= 255 << (writeOffset % 8); + } return true; } @@ -149,6 +160,11 @@ namespace isobus ++data; buffer[byte] = (buffer[byte] & mask) | (*data << bit); } + if (writeOffset % 8 != 0) + { + // Mask the top bits to hide excess data. + buffer[byte] |= 255 << (writeOffset % 8); + } return true; } From aafc32858d1c4ec92799c569d8072282366d54d9 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 09:00:38 +0000 Subject: [PATCH 06/21] Get the number of bytes written to, at least partially. --- isobus/src/can_parameter_group_builder.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/isobus/src/can_parameter_group_builder.cpp b/isobus/src/can_parameter_group_builder.cpp index ce67c5c93..0da1fe3a1 100644 --- a/isobus/src/can_parameter_group_builder.cpp +++ b/isobus/src/can_parameter_group_builder.cpp @@ -175,6 +175,11 @@ namespace isobus return writeOffset; } + unsigned int get_written_bytes() const + { + return (writeOffset + 7) / 8; + } + template bool write(T const & data) { From 7e3b605ec2cdd61c27376fd0839ba96d9b80ff1a Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 09:17:38 +0000 Subject: [PATCH 07/21] Basic `read` API. --- isobus/src/can_parameter_group_builder.cpp | 164 ++++++++++++++++----- 1 file changed, 128 insertions(+), 36 deletions(-) diff --git a/isobus/src/can_parameter_group_builder.cpp b/isobus/src/can_parameter_group_builder.cpp index 0da1fe3a1..8d6244160 100644 --- a/isobus/src/can_parameter_group_builder.cpp +++ b/isobus/src/can_parameter_group_builder.cpp @@ -20,13 +20,25 @@ namespace isobus // Which byte to write to. return writeOffset / 8; } - + unsigned int get_write_bit_offset() const { // Which bit to write to in the current byte. return writeOffset % 8; } + unsigned int get_read_byte_offset() const + { + // Which byte to read from. + return readOffset / 8; + } + + unsigned int get_read_bit_offset() const + { + // Which bit to read from in the current byte. + return readOffset % 8; + } + bool write_bits(unsigned char const * data, unsigned int bits) { if (bits == 0) @@ -70,7 +82,7 @@ namespace isobus } return true; } - + // Whole bytes (one byte is often handled above.) if (bit == 0 && bits % 8 == 0) { @@ -91,11 +103,11 @@ namespace isobus // No additional masking required in this case. return true; } - + // ---------------------------------------- // Stage one - fill out the current byte. // ---------------------------------------- - + // All this data can come from the first byte of the input. if (byte == 8) { @@ -110,11 +122,11 @@ namespace isobus buffer[byte] = (buffer[byte] & mask) | (*data << bit); bits -= remaining; ++byte; - + // ------------------------------- // Stage two - copy whole bytes. // ------------------------------- - + while (bits > 8) { if (byte == 8) @@ -133,7 +145,7 @@ namespace isobus // ------------------------------- // Stage three - copy whole bytes. // ------------------------------- - + // `bits` is number of bits left. It may or may not span a byte boundary in the input. if (bits == 0) { @@ -165,7 +177,7 @@ namespace isobus // Mask the top bits to hide excess data. buffer[byte] |= 255 << (writeOffset % 8); } - + return true; } @@ -174,12 +186,22 @@ namespace isobus { return writeOffset; } - + unsigned int get_written_bytes() const { return (writeOffset + 7) / 8; } - + + unsigned int get_read_bits() const + { + return readOffset; + } + + unsigned int get_read_bytes() const + { + return (readOffset + 7) / 8; + } + template bool write(T const & data) { @@ -195,32 +217,8 @@ namespace isobus template <> bool write(bool const & data) { - if (data) - { - unsigned char one = 255; - return write_bits(&one, 1); - } - else - { - unsigned char naught = 0; - return write_bits(&naught, 1); - } - } - - template <> - bool write(bool const & data) - { - // Only use a single bit for booleans. - if (data) - { - unsigned char one = 255; - return write_bits(&one, 1); - } - else - { - unsigned char naught = 0; - return write_bits(&naught, 1); - } + unsigned char bits = data ? 255 : 0; + return write_bits(&bits, 1); } template <> @@ -273,6 +271,7 @@ namespace isobus writeOffset = revert; return false; } + ++data; } if (includeNull) { @@ -315,6 +314,99 @@ namespace isobus } return true; } + + template + bool read(T & data) + { + return read_bits((unsigned char *)&data, sizeof (T) * 8); + } + + template + bool read(T & data, unsigned int bits) + { + return read_bits((unsigned char *)&data, bits); + } + + template <> + bool read(bool & data) + { + unsigned char bits = 0; + if (read_bits(&bits, 1)) + { + data = !!bits; + return true; + } + return false; + } + + template <> + bool read(char * & data) + { + // Read until NULL. + return read((unsigned char *)data); + } + + template <> + bool read(unsigned char * & data) + { + // Read until NULL. + unsigned int revert = readOffset; + // Don't modify `data`! + unsigned char * ptr = data; + for ( ; ; ) + { + if (!read_bits(ptr, 8)) + { + readOffset = revert; + return false; + } + if (*ptr == 0) + { + // Found a NULL byte. + break; + } + ++ptr; + } + return true; + } + + template <> + bool read(char * & data, unsigned int bits) + { + // It is a bit awkward to specify how much of a string to read. + return read((unsigned char *)data, bits); + } + + template <> + bool read(unsigned char * & data, unsigned int bits) + { + if (bits % 8 != 0) + { + // This requires normal string sizes. + return false; + } + // Don't modify `data`! + unsigned char * ptr = data; + // Don't write NULL, just assume the caller handles that. + unsigned int revert = readOffset; + while (bits) + { + if (!read_bits(ptr, 8)) + { + readOffset = revert; + return false; + } + bits -= 8; + ++ptr; + } + return true; + } + + bool skip(unsigned int bits) + { + // Easy! + readOffset += bits; + } }; } From f5e5a6a95524c79737d2adc62a7d765716e23764 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 09:27:05 +0000 Subject: [PATCH 08/21] Inline padding code. --- isobus/src/can_parameter_group_builder.cpp | 35 +++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/isobus/src/can_parameter_group_builder.cpp b/isobus/src/can_parameter_group_builder.cpp index 8d6244160..f3a4bf8d0 100644 --- a/isobus/src/can_parameter_group_builder.cpp +++ b/isobus/src/can_parameter_group_builder.cpp @@ -292,24 +292,39 @@ namespace isobus bool pad(unsigned int bits, bool value = true) { - // Only use a single bit for booleans. unsigned int revert = writeOffset; unsigned char data = value ? 255 : 0; - while (bits > 8) + unsigned int byte = get_write_byte_offset(); + unsigned int bit = get_write_bit_offset(); + unsigned int remaining = 8 - bit; + unsigned char mask = 255 >> remaining; + writeOffset += bits; + for ( ; ; ) { - if (!write_bits(&data, 8)) + if (byte == 8) { writeOffset = revert; return false; } - bits -= 8; - } - if (bits) - { - if (!write_bits(&data, bits)) + buffer[byte] = (buffer[byte] & mask) | (data << bit); + if (remaining > bits) { - writeOffset = revert; - return false; + // Weirdly we need a second masking here, to keep untouched bits as `1`. + buffer[byte] |= 255 << (bits + bit); + break; + } + else if (remaining == bits) + { + // Done. + break; + } + else + { + bits -= remaining; + bit = 0; + remaining = 8; + mask = 0; + ++byte; } } return true; From 079b104fddb4db2a0394eec1a97f573fd6cb1318 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 13:06:10 +0000 Subject: [PATCH 09/21] Move to includes and flesh out reading. --- .../isobus/can_parameter_group_builder.hpp} | 119 +++++++++++++++--- isobus/src/can_parameter_group_builder.hpp | 20 --- 2 files changed, 104 insertions(+), 35 deletions(-) rename isobus/{src/can_parameter_group_builder.cpp => include/isobus/isobus/can_parameter_group_builder.hpp} (80%) delete mode 100644 isobus/src/can_parameter_group_builder.hpp diff --git a/isobus/src/can_parameter_group_builder.cpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp similarity index 80% rename from isobus/src/can_parameter_group_builder.cpp rename to isobus/include/isobus/isobus/can_parameter_group_builder.hpp index f3a4bf8d0..0eb51f97c 100644 --- a/isobus/src/can_parameter_group_builder.cpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -52,7 +52,7 @@ namespace isobus // character. unsigned int revert = writeOffset; unsigned int byte = get_write_byte_offset(); - unsigned int bit = get_write_bit_offset(); + unsigned int offset = get_write_bit_offset(); // Adjust first, and revert later. writeOffset += bits; @@ -61,7 +61,7 @@ namespace isobus // ------------------------------- // How much space is there left in this byte? - unsigned int remaining = 8 - bit; + unsigned int remaining = 8 - offset; if (remaining >= bits) { if (byte == 8) @@ -73,8 +73,8 @@ namespace isobus // Everything will fit in the current byte, which must mean there's at most one byte // of data to write. Hence we put this version first because it covers more single // byte cases. - unsigned char mask = (1 << bit) - 1; - buffer[byte] = (buffer[byte] & mask) | (*data << bit); + unsigned char mask = (1 << offset) - 1; + buffer[byte] = (buffer[byte] & mask) | (*data << offset); if (writeOffset % 8 != 0) { // Mask the top bits to hide excess data. @@ -84,7 +84,7 @@ namespace isobus } // Whole bytes (one byte is often handled above.) - if (bit == 0 && bits % 8 == 0) + if (offset == 0 && bits % 8 == 0) { // Whole bytes, which are aligned. do @@ -118,8 +118,8 @@ namespace isobus // Everything will fit in the current byte, which must mean there's at most one byte // of data to write. Hence we put this version first because it covers more single // byte cases. - unsigned char mask = (1 << bit) - 1; - buffer[byte] = (buffer[byte] & mask) | (*data << bit); + unsigned char mask = (1 << offset) - 1; + buffer[byte] = (buffer[byte] & mask) | (*data << offset); bits -= remaining; ++byte; @@ -137,7 +137,7 @@ namespace isobus } buffer[byte] = (*data >> remaining); ++data; - buffer[byte] = (buffer[byte] & mask) | (*data << bit); + buffer[byte] = (buffer[byte] & mask) | (*data << offset); bits -= 8; ++byte; } @@ -166,11 +166,11 @@ namespace isobus return false; } buffer[byte] = (*data >> remaining); - if (bit < bits) + if (offset < bits) { // The final output spans two bytes of input. ++data; - buffer[byte] = (buffer[byte] & mask) | (*data << bit); + buffer[byte] = (buffer[byte] & mask) | (*data << offset); } if (writeOffset % 8 != 0) { @@ -180,6 +180,95 @@ namespace isobus return true; } + + bool read_bits(unsigned char * data, unsigned int bits) + { + unsigned int revert = readOffset; + unsigned int byte = get_read_byte_offset(); + unsigned int input = get_read_bit_offset(); + unsigned int remaining = 8 - input; + unsigned int output = 0; + unsigned int space = 8 - output; + // Mark as read, even though we actually haven't yet. + readOffset += bits; + // Initialise the current destination byte. + *data = 0; + while (bits) + { + if (byte == 8) + { + // Ran out of data. + readOffset = revert; + return false; + } + // Move in the new data. + *data = (*data & ~(255 << output)) | ((buffer[byte] >> input) << output); + if (space > remaining) + { + if (remaining >= bits) + { + bits = 0; + } + else + { + bits -= remaining; + } + space -= remaining; + output = 8 - space; + remaining = 0; + input = 0; + ++byte; + } + else if (remaining > space) + { + if (space >= bits) + { + bits = 0; + } + else + { + bits -= space; + } + remaining -= space; + input = 8 - remaining; + space = 8; + output = 0; + ++data; + if (bits) + { + *data = 0; + } + } + else + { + if (remaining >= bits) + { + bits = 0; + } + else + { + bits -= space; + } + // Perfectly matched. + remaining = 8; + input = 0; + space = 8; + output = 0; + ++data; + if (bits) + { + *data = 0; + } + ++byte; + } + } + if (output) + { + // Bits left to mask. + *data = *data & ~(255 << output); + } + return true; + } public: unsigned int get_written_bits() const @@ -295,8 +384,8 @@ namespace isobus unsigned int revert = writeOffset; unsigned char data = value ? 255 : 0; unsigned int byte = get_write_byte_offset(); - unsigned int bit = get_write_bit_offset(); - unsigned int remaining = 8 - bit; + unsigned int offset = get_write_bit_offset(); + unsigned int remaining = 8 - offset; unsigned char mask = 255 >> remaining; writeOffset += bits; for ( ; ; ) @@ -306,11 +395,11 @@ namespace isobus writeOffset = revert; return false; } - buffer[byte] = (buffer[byte] & mask) | (data << bit); + buffer[byte] = (buffer[byte] & mask) | (data << offset); if (remaining > bits) { // Weirdly we need a second masking here, to keep untouched bits as `1`. - buffer[byte] |= 255 << (bits + bit); + buffer[byte] |= 255 << (bits + offset); break; } else if (remaining == bits) @@ -321,7 +410,7 @@ namespace isobus else { bits -= remaining; - bit = 0; + offset = 0; remaining = 8; mask = 0; ++byte; diff --git a/isobus/src/can_parameter_group_builder.hpp b/isobus/src/can_parameter_group_builder.hpp deleted file mode 100644 index 174c511ad..000000000 --- a/isobus/src/can_parameter_group_builder.hpp +++ /dev/null @@ -1,20 +0,0 @@ -namespace isobus -{ - // Note that this class currently only works for packets eight bytes or fewer, because that is - // all the use-cases I needed. The spec has a whole section on how data can span byte - // boundaries, and how to align the bits when a data-type that is not a multiple of eight bits - // crosses said boundaries. Then every single parameter group uses padding to avoid those - // cases! The five-bit tractor command types have three bits of padding to make them exactly - // one byte. Why even specify the scheme if it isn't used? - // - // I wrote a huge amount of this file once, then somehow it vanished. I don't know how! - class GroupBuilder - { - private: - int bitOffset = 0; - int bytesUsed = 0; - char data[8]; - - }; -} - From a397b0217eab0b7109863106dd3a98d970d5875a Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 13:43:14 +0000 Subject: [PATCH 10/21] Reserve and get bits. --- .../isobus/can_parameter_group_builder.hpp | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 0eb51f97c..1006dacb2 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -95,7 +95,7 @@ namespace isobus writeOffset = revert; return false; } - buffer[byte] = data++; + buffer[byte] = *data++; byte += 1; bits -= 8; } @@ -271,6 +271,18 @@ namespace isobus } public: + GroupBuilder() + { + buffer[0] = 255; + buffer[1] = 255; + buffer[2] = 255; + buffer[3] = 255; + buffer[4] = 255; + buffer[5] = 255; + buffer[6] = 255; + buffer[7] = 255; + } + unsigned int get_written_bits() const { return writeOffset; @@ -447,7 +459,7 @@ namespace isobus bool read(char * & data) { // Read until NULL. - return read((unsigned char *)data); + return read((unsigned char * &)data); } template <> @@ -478,7 +490,7 @@ namespace isobus bool read(char * & data, unsigned int bits) { // It is a bit awkward to specify how much of a string to read. - return read((unsigned char *)data, bits); + return read((unsigned char * &)data, bits); } template <> @@ -511,6 +523,19 @@ namespace isobus // Easy! readOffset += bits; } + + unsigned int get_data(unsigned char output[8]) + { + output[0] = buffer[0]; + output[1] = buffer[1]; + output[2] = buffer[2]; + output[3] = buffer[3]; + output[4] = buffer[4]; + output[5] = buffer[5]; + output[6] = buffer[6]; + output[7] = buffer[7]; + return get_written_bytes(); + } }; } From 009322d2dd9f3d9331895005feedf8c3db975448 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 14:55:37 +0000 Subject: [PATCH 11/21] Get some sane values in buffer read error cases. --- isobus/include/isobus/isobus/can_parameter_group_builder.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 1006dacb2..7f3fcd7ca 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -440,6 +440,8 @@ namespace isobus template bool read(T & data, unsigned int bits) { + // Clear the memory, since we may not be reading the full width. + memset(&data, 0, sizeof(T)); return read_bits((unsigned char *)&data, bits); } @@ -452,6 +454,7 @@ namespace isobus data = !!bits; return true; } + data = false; return false; } @@ -474,6 +477,7 @@ namespace isobus if (!read_bits(ptr, 8)) { readOffset = revert; + *data = '\0'; return false; } if (*ptr == 0) @@ -510,6 +514,7 @@ namespace isobus if (!read_bits(ptr, 8)) { readOffset = revert; + *data = '\0'; return false; } bits -= 8; From eb512d3dbdb2fe68f8a7f42d4b6003c01cf86abc Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 15:00:20 +0000 Subject: [PATCH 12/21] Check if skipping input bits is possible. --- .../include/isobus/isobus/can_parameter_group_builder.hpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 7f3fcd7ca..24977a94d 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -525,8 +525,13 @@ namespace isobus bool skip(unsigned int bits) { - // Easy! readOffset += bits; + if (readOffset > 8 * 8) + { + readOffset -= bits; + return false; + } + return true; } unsigned int get_data(unsigned char output[8]) From b7bda0c5220c1838ccc66b84badbb13ca98c5516 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 15:09:02 +0000 Subject: [PATCH 13/21] Fix clearing the upper bits when loading small bit width values. --- .../isobus/can_parameter_group_builder.hpp | 47 +++++-------------- 1 file changed, 12 insertions(+), 35 deletions(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 24977a94d..3074ce4e7 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -202,17 +202,14 @@ namespace isobus return false; } // Move in the new data. - *data = (*data & ~(255 << output)) | ((buffer[byte] >> input) << output); + *data = (*data & (255 >> space)) | ((buffer[byte] >> input) << output); + if (space >= bits) + { + space -= bits; + break; + } if (space > remaining) { - if (remaining >= bits) - { - bits = 0; - } - else - { - bits -= remaining; - } space -= remaining; output = 8 - space; remaining = 0; @@ -221,51 +218,31 @@ namespace isobus } else if (remaining > space) { - if (space >= bits) - { - bits = 0; - } - else - { - bits -= space; - } + bits -= space; remaining -= space; input = 8 - remaining; space = 8; output = 0; ++data; - if (bits) - { - *data = 0; - } + *data = 0; } else { - if (remaining >= bits) - { - bits = 0; - } - else - { - bits -= space; - } + bits -= space; // Perfectly matched. remaining = 8; input = 0; space = 8; output = 0; ++data; - if (bits) - { - *data = 0; - } + *data = 0; ++byte; } } - if (output) + if (space) { // Bits left to mask. - *data = *data & ~(255 << output); + *data = *data & (255 >> space); } return true; } From ca0b328957a5ce65ba5b66e88a3de55e186cc9d3 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Tue, 7 Mar 2023 15:23:12 +0000 Subject: [PATCH 14/21] Some stream read/writing. --- isobus/include/isobus/isobus/can_parameter_group_builder.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 3074ce4e7..0acf90f17 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -8,7 +8,7 @@ namespace isobus // one byte. Why even specify the scheme if it isn't used? // // I wrote a huge amount of this file once, then somehow it vanished. I don't know how! - class GroupBuilder + class ParameterGroupBuilder { private: int writeOffset = 0; From d62605e91d356a8705d909625d31fe6657f274a8 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Wed, 8 Mar 2023 07:30:02 +0000 Subject: [PATCH 15/21] `#pragma once` --- isobus/include/isobus/isobus/can_parameter_group_builder.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 0acf90f17..c9f8f6011 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -1,3 +1,5 @@ +#pragma once + namespace isobus { // Note that this class currently only works for packets eight bytes or fewer, because that is @@ -248,7 +250,7 @@ namespace isobus } public: - GroupBuilder() + ParameterGroupBuilder() { buffer[0] = 255; buffer[1] = 255; From 90c3b32fe9facd63a1062d38b5c254b6eb032295 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Wed, 8 Mar 2023 12:24:27 +0000 Subject: [PATCH 16/21] Pre-check that there is read/write space left. --- .../isobus/can_parameter_group_builder.hpp | 55 ++++++------------- 1 file changed, 17 insertions(+), 38 deletions(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index c9f8f6011..958e27c12 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -52,11 +52,15 @@ namespace isobus // writing some data we can roll back the entire change. This is replicated in the // string writing code because it has to roll back the entire string, not just the last // character. - unsigned int revert = writeOffset; unsigned int byte = get_write_byte_offset(); unsigned int offset = get_write_bit_offset(); // Adjust first, and revert later. writeOffset += bits; + if (writeOffset > 64) + { + writeOffset -= bits; + return false; + } // ------------------------------- // Stage naught - trivial cases. @@ -66,12 +70,6 @@ namespace isobus unsigned int remaining = 8 - offset; if (remaining >= bits) { - if (byte == 8) - { - // Out of space for an eight byte packet. - writeOffset = revert; - return false; - } // Everything will fit in the current byte, which must mean there's at most one byte // of data to write. Hence we put this version first because it covers more single // byte cases. @@ -91,12 +89,6 @@ namespace isobus // Whole bytes, which are aligned. do { - if (byte == 8) - { - // Out of space for an eight byte packet. - writeOffset = revert; - return false; - } buffer[byte] = *data++; byte += 1; bits -= 8; @@ -111,12 +103,6 @@ namespace isobus // ---------------------------------------- // All this data can come from the first byte of the input. - if (byte == 8) - { - // Out of space for an eight byte packet. - writeOffset = revert; - return false; - } // Everything will fit in the current byte, which must mean there's at most one byte // of data to write. Hence we put this version first because it covers more single // byte cases. @@ -131,12 +117,6 @@ namespace isobus while (bits > 8) { - if (byte == 8) - { - // Out of space for an eight byte packet. - writeOffset = revert; - return false; - } buffer[byte] = (*data >> remaining); ++data; buffer[byte] = (buffer[byte] & mask) | (*data << offset); @@ -161,12 +141,6 @@ namespace isobus } // I'm sure this code can be compressed with the code above. - if (byte == 8) - { - // Out of space for an eight byte packet. - writeOffset = revert; - return false; - } buffer[byte] = (*data >> remaining); if (offset < bits) { @@ -185,7 +159,6 @@ namespace isobus bool read_bits(unsigned char * data, unsigned int bits) { - unsigned int revert = readOffset; unsigned int byte = get_read_byte_offset(); unsigned int input = get_read_bit_offset(); unsigned int remaining = 8 - input; @@ -193,16 +166,22 @@ namespace isobus unsigned int space = 8 - output; // Mark as read, even though we actually haven't yet. readOffset += bits; + if (readOffset > writeOffset) + { + // Trying to read too much data. + readOffset -= bits; + return false; + } + if (readOffset > 64) + { + // Trying to read too much data. + readOffset -= bits; + return false; + } // Initialise the current destination byte. *data = 0; while (bits) { - if (byte == 8) - { - // Ran out of data. - readOffset = revert; - return false; - } // Move in the new data. *data = (*data & (255 >> space)) | ((buffer[byte] >> input) << output); if (space >= bits) From 2fdb40fa5fddb6f712c2108bae0d60267ca8d317 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Wed, 8 Mar 2023 12:27:19 +0000 Subject: [PATCH 17/21] Data input constructor. --- .../isobus/can_parameter_group_builder.hpp | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 958e27c12..785b6c016 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -231,14 +231,28 @@ namespace isobus public: ParameterGroupBuilder() { - buffer[0] = 255; - buffer[1] = 255; - buffer[2] = 255; - buffer[3] = 255; - buffer[4] = 255; - buffer[5] = 255; - buffer[6] = 255; - buffer[7] = 255; + unsigned int i = 0; + while (i != 8) + { + buffer[i] = 255; + ++i; + } + } + + ParameterGroupBuilder(unsigned char *data, unsigned int len) + { + unsigned int i = 0; + while (i != len) + { + buffer[i] = data[i]; + ++i; + } + while (i != 8) + { + buffer[i] = 255; + ++i; + } + writeOffset = len * 8; } unsigned int get_written_bits() const From 8d8fda3e05f77f81450b7ecf2f0ff74da387ab8b Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Wed, 8 Mar 2023 12:29:58 +0000 Subject: [PATCH 18/21] More limit pre-checks. --- .../isobus/isobus/can_parameter_group_builder.hpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 785b6c016..77d381f86 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -365,20 +365,19 @@ namespace isobus bool pad(unsigned int bits, bool value = true) { - unsigned int revert = writeOffset; unsigned char data = value ? 255 : 0; unsigned int byte = get_write_byte_offset(); unsigned int offset = get_write_bit_offset(); unsigned int remaining = 8 - offset; unsigned char mask = 255 >> remaining; writeOffset += bits; + if (writeOffset > 64) + { + writeOffset -= bits; + return false; + } for ( ; ; ) { - if (byte == 8) - { - writeOffset = revert; - return false; - } buffer[byte] = (buffer[byte] & mask) | (data << offset); if (remaining > bits) { @@ -498,7 +497,7 @@ namespace isobus bool skip(unsigned int bits) { readOffset += bits; - if (readOffset > 8 * 8) + if (readOffset > 64) { readOffset -= bits; return false; From 39fd56161c2d79da2655f63fe3c4d5b0546f829c Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Wed, 8 Mar 2023 13:05:14 +0000 Subject: [PATCH 19/21] Remove the eight byte limit on packets. --- .../isobus/can_parameter_group_builder.hpp | 53 ++++--------------- 1 file changed, 11 insertions(+), 42 deletions(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index 77d381f86..b3adf6eeb 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -15,7 +15,7 @@ namespace isobus private: int writeOffset = 0; int readOffset = 0; - unsigned char buffer[8]; + std::vector buffer; unsigned int get_write_byte_offset() const { @@ -54,13 +54,9 @@ namespace isobus // character. unsigned int byte = get_write_byte_offset(); unsigned int offset = get_write_bit_offset(); - // Adjust first, and revert later. + // Adjust the size, ensure the buffer is large enough, and initialise new values. writeOffset += bits; - if (writeOffset > 64) - { - writeOffset -= bits; - return false; - } + buffer.resize((writeOffset + 7) / 8, 255); // ------------------------------- // Stage naught - trivial cases. @@ -172,12 +168,6 @@ namespace isobus readOffset -= bits; return false; } - if (readOffset > 64) - { - // Trying to read too much data. - readOffset -= bits; - return false; - } // Initialise the current destination byte. *data = 0; while (bits) @@ -239,20 +229,10 @@ namespace isobus } } - ParameterGroupBuilder(unsigned char *data, unsigned int len) + ParameterGroupBuilder(std::vector const &data) { - unsigned int i = 0; - while (i != len) - { - buffer[i] = data[i]; - ++i; - } - while (i != 8) - { - buffer[i] = 255; - ++i; - } - writeOffset = len * 8; + buffer = data; + writeOffset = data.size() * 8; } unsigned int get_written_bits() const @@ -371,12 +351,8 @@ namespace isobus unsigned int remaining = 8 - offset; unsigned char mask = 255 >> remaining; writeOffset += bits; - if (writeOffset > 64) - { - writeOffset -= bits; - return false; - } - for ( ; ; ) + buffer.resize((writeOffset + 7) / 8, 255); + for (;;) { buffer[byte] = (buffer[byte] & mask) | (data << offset); if (remaining > bits) @@ -497,7 +473,7 @@ namespace isobus bool skip(unsigned int bits) { readOffset += bits; - if (readOffset > 64) + if (readOffset > writeOffset) { readOffset -= bits; return false; @@ -505,16 +481,9 @@ namespace isobus return true; } - unsigned int get_data(unsigned char output[8]) + unsigned int get_data(std::vector &output) { - output[0] = buffer[0]; - output[1] = buffer[1]; - output[2] = buffer[2]; - output[3] = buffer[3]; - output[4] = buffer[4]; - output[5] = buffer[5]; - output[6] = buffer[6]; - output[7] = buffer[7]; + output = buffer; return get_written_bytes(); } }; From 4950e8789f4112b995ddca9b4cb1842e75643448 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Wed, 8 Mar 2023 13:07:11 +0000 Subject: [PATCH 20/21] Use standard size types. --- .../isobus/can_parameter_group_builder.hpp | 140 +++++++++--------- 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index b3adf6eeb..b49f544bc 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -1,5 +1,7 @@ #pragma once +#include + namespace isobus { // Note that this class currently only works for packets eight bytes or fewer, because that is @@ -13,35 +15,35 @@ namespace isobus class ParameterGroupBuilder { private: - int writeOffset = 0; - int readOffset = 0; - std::vector buffer; + std::size_t writeOffset = 0; + std::size_t readOffset = 0; + std::vector buffer; - unsigned int get_write_byte_offset() const + std::size_t get_write_byte_offset() const { // Which byte to write to. return writeOffset / 8; } - unsigned int get_write_bit_offset() const + std::size_t get_write_bit_offset() const { // Which bit to write to in the current byte. return writeOffset % 8; } - unsigned int get_read_byte_offset() const + std::size_t get_read_byte_offset() const { // Which byte to read from. return readOffset / 8; } - unsigned int get_read_bit_offset() const + std::size_t get_read_bit_offset() const { // Which bit to read from in the current byte. return readOffset % 8; } - bool write_bits(unsigned char const * data, unsigned int bits) + bool write_bits(std::uint8_t const * data, std::size_t bits) { if (bits == 0) { @@ -52,8 +54,8 @@ namespace isobus // writing some data we can roll back the entire change. This is replicated in the // string writing code because it has to roll back the entire string, not just the last // character. - unsigned int byte = get_write_byte_offset(); - unsigned int offset = get_write_bit_offset(); + std::size_t byte = get_write_byte_offset(); + std::size_t offset = get_write_bit_offset(); // Adjust the size, ensure the buffer is large enough, and initialise new values. writeOffset += bits; buffer.resize((writeOffset + 7) / 8, 255); @@ -63,13 +65,13 @@ namespace isobus // ------------------------------- // How much space is there left in this byte? - unsigned int remaining = 8 - offset; + std::size_t remaining = 8 - offset; if (remaining >= bits) { // Everything will fit in the current byte, which must mean there's at most one byte // of data to write. Hence we put this version first because it covers more single // byte cases. - unsigned char mask = (1 << offset) - 1; + std::uint8_t mask = (1 << offset) - 1; buffer[byte] = (buffer[byte] & mask) | (*data << offset); if (writeOffset % 8 != 0) { @@ -102,7 +104,7 @@ namespace isobus // Everything will fit in the current byte, which must mean there's at most one byte // of data to write. Hence we put this version first because it covers more single // byte cases. - unsigned char mask = (1 << offset) - 1; + std::uint8_t mask = (1 << offset) - 1; buffer[byte] = (buffer[byte] & mask) | (*data << offset); bits -= remaining; ++byte; @@ -153,13 +155,13 @@ namespace isobus return true; } - bool read_bits(unsigned char * data, unsigned int bits) + bool read_bits(std::uint8_t * data, std::size_t bits) { - unsigned int byte = get_read_byte_offset(); - unsigned int input = get_read_bit_offset(); - unsigned int remaining = 8 - input; - unsigned int output = 0; - unsigned int space = 8 - output; + std::size_t byte = get_read_byte_offset(); + std::size_t input = get_read_bit_offset(); + std::size_t remaining = 8 - input; + std::size_t output = 0; + std::size_t space = 8 - output; // Mark as read, even though we actually haven't yet. readOffset += bits; if (readOffset > writeOffset) @@ -219,38 +221,34 @@ namespace isobus } public: - ParameterGroupBuilder() + ParameterGroupBuilder() : + buffer() { - unsigned int i = 0; - while (i != 8) - { - buffer[i] = 255; - ++i; - } } - ParameterGroupBuilder(std::vector const &data) + ParameterGroupBuilder(std::vector const &data) : + buffer() { buffer = data; writeOffset = data.size() * 8; } - unsigned int get_written_bits() const + std::size_t get_written_bits() const { return writeOffset; } - unsigned int get_written_bytes() const + std::size_t get_written_bytes() const { return (writeOffset + 7) / 8; } - unsigned int get_read_bits() const + std::size_t get_read_bits() const { return readOffset; } - unsigned int get_read_bytes() const + std::size_t get_read_bytes() const { return (readOffset + 7) / 8; } @@ -258,19 +256,19 @@ namespace isobus template bool write(T const & data) { - return write_bits((unsigned char const *)&data, sizeof (T) * 8); + return write_bits((std::uint8_t const *)&data, sizeof (T) * 8); } template - bool write(T const & data, unsigned int bits) + bool write(T const & data, std::size_t bits) { - return write_bits((unsigned char const *)&data, bits); + return write_bits((std::uint8_t const *)&data, bits); } template <> bool write(bool const & data) { - unsigned char bits = data ? 255 : 0; + std::uint8_t bits = data ? 255 : 0; return write_bits(&bits, 1); } @@ -278,45 +276,45 @@ namespace isobus bool write(char const * const & data) { // What should the default for including NULL be? - return write((unsigned char const *)data, false); + return write((std::uint8_t const *)data, false); } template <> bool write(char * const & data) { // What should the default for including NULL be? - return write((unsigned char const *)data, false); + return write((std::uint8_t const *)data, false); } template <> - bool write(unsigned char const * const & data) + bool write(std::uint8_t const * const & data) { // What should the default for including NULL be? - return write((unsigned char const *)data, false); + return write((std::uint8_t const *)data, false); } template <> - bool write(unsigned char * const & data) + bool write(std::uint8_t * const & data) { // What should the default for including NULL be? - return write((unsigned char const *)data, false); + return write((std::uint8_t const *)data, false); } bool write(char const * data, bool includeNull) { - return write((unsigned char const *)data, includeNull); + return write((std::uint8_t const *)data, includeNull); } bool write(char * data, bool includeNull) { - return write((unsigned char const *)data, includeNull); + return write((std::uint8_t const *)data, includeNull); } - bool write(unsigned char const * data, bool includeNull) + bool write(std::uint8_t const * data, bool includeNull) { // Base case. Write each byte separately so they don't get put in little-endian, which // makes no sense for strings. - unsigned int revert = writeOffset; + std::size_t revert = writeOffset; while (*data) { if (!write_bits(data, 8)) @@ -328,7 +326,7 @@ namespace isobus } if (includeNull) { - unsigned char naught = 0; + std::uint8_t naught = 0; if (!write_bits(&naught, 8)) { writeOffset = revert; @@ -338,18 +336,18 @@ namespace isobus return true; } - bool write(unsigned char * data, bool includeNull) + bool write(std::uint8_t * data, bool includeNull) { - return write((unsigned char const *)data, includeNull); + return write((std::uint8_t const *)data, includeNull); } - bool pad(unsigned int bits, bool value = true) + bool pad(std::size_t bits, bool value = true) { - unsigned char data = value ? 255 : 0; - unsigned int byte = get_write_byte_offset(); - unsigned int offset = get_write_bit_offset(); - unsigned int remaining = 8 - offset; - unsigned char mask = 255 >> remaining; + std::uint8_t data = value ? 255 : 0; + std::size_t byte = get_write_byte_offset(); + std::size_t offset = get_write_bit_offset(); + std::size_t remaining = 8 - offset; + std::uint8_t mask = 255 >> remaining; writeOffset += bits; buffer.resize((writeOffset + 7) / 8, 255); for (;;) @@ -381,21 +379,21 @@ namespace isobus template bool read(T & data) { - return read_bits((unsigned char *)&data, sizeof (T) * 8); + return read_bits((std::uint8_t *)&data, sizeof (T) * 8); } template - bool read(T & data, unsigned int bits) + bool read(T & data, std::size_t bits) { // Clear the memory, since we may not be reading the full width. memset(&data, 0, sizeof(T)); - return read_bits((unsigned char *)&data, bits); + return read_bits((std::uint8_t *)&data, bits); } template <> bool read(bool & data) { - unsigned char bits = 0; + std::uint8_t bits = 0; if (read_bits(&bits, 1)) { data = !!bits; @@ -409,16 +407,16 @@ namespace isobus bool read(char * & data) { // Read until NULL. - return read((unsigned char * &)data); + return read((std::uint8_t * &)data); } template <> - bool read(unsigned char * & data) + bool read(std::uint8_t * & data) { // Read until NULL. - unsigned int revert = readOffset; + std::size_t revert = readOffset; // Don't modify `data`! - unsigned char * ptr = data; + std::uint8_t * ptr = data; for ( ; ; ) { if (!read_bits(ptr, 8)) @@ -438,14 +436,14 @@ namespace isobus } template <> - bool read(char * & data, unsigned int bits) + bool read(char * & data, std::size_t bits) { // It is a bit awkward to specify how much of a string to read. - return read((unsigned char * &)data, bits); + return read((std::uint8_t * &)data, bits); } template <> - bool read(unsigned char * & data, unsigned int bits) + bool read(std::uint8_t * & data, std::size_t bits) { if (bits % 8 != 0) { @@ -453,9 +451,9 @@ namespace isobus return false; } // Don't modify `data`! - unsigned char * ptr = data; + std::uint8_t * ptr = data; // Don't write NULL, just assume the caller handles that. - unsigned int revert = readOffset; + std::size_t revert = readOffset; while (bits) { if (!read_bits(ptr, 8)) @@ -470,7 +468,7 @@ namespace isobus return true; } - bool skip(unsigned int bits) + bool skip(std::size_t bits) { readOffset += bits; if (readOffset > writeOffset) @@ -481,10 +479,12 @@ namespace isobus return true; } - unsigned int get_data(std::vector &output) + std::size_t get_data(std::vector &output) { + size_t size = get_written_bytes(); output = buffer; - return get_written_bytes(); + output.resize(size); + return size; } }; } From a5add6ca5d54a3c81308794549cb74e5542b14f0 Mon Sep 17 00:00:00 2001 From: Alex Cole Date: Wed, 8 Mar 2023 13:39:27 +0000 Subject: [PATCH 21/21] Read/write resets. --- .../isobus/isobus/can_parameter_group_builder.hpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp index b49f544bc..6adf54824 100644 --- a/isobus/include/isobus/isobus/can_parameter_group_builder.hpp +++ b/isobus/include/isobus/isobus/can_parameter_group_builder.hpp @@ -486,6 +486,18 @@ namespace isobus output.resize(size); return size; } + + void reset_read() + { + readOffset = 0; + } + + void reset_write() + { + readOffset = 0; + writeOffset = 0; + buffer.clear(); + } }; }