From ee0a5ab90f3dd6b3128a578dafd35072e4a73725 Mon Sep 17 00:00:00 2001 From: Peter Newman Date: Tue, 4 Feb 2020 15:35:15 +0000 Subject: [PATCH] Add LLRP Probe Request PDU, fix the length flags bug (cherry picked from commit e9dd3742f9310c0a72bccacfc45cfb9bacf115aa) --- include/ola/acn/ACNFlags.h | 70 +++++++++++++++++++++++++++++++++++ include/ola/acn/Makefile.mk | 1 + libs/acn/BaseInflator.cpp | 4 +- libs/acn/BaseInflator.h | 7 +++- libs/acn/BaseInflatorTest.cpp | 18 ++++----- libs/acn/LLRPInflatorTest.cpp | 2 +- libs/acn/LLRPPDU.cpp | 3 +- libs/acn/LLRPPDU.h | 8 ++-- libs/acn/LLRPPDUTest.cpp | 25 +++++++------ libs/acn/PDU.cpp | 32 ++++++++++++---- libs/acn/PDU.h | 37 +++++++++++++----- 11 files changed, 159 insertions(+), 48 deletions(-) create mode 100644 include/ola/acn/ACNFlags.h diff --git a/include/ola/acn/ACNFlags.h b/include/ola/acn/ACNFlags.h new file mode 100644 index 0000000000..a055a429b0 --- /dev/null +++ b/include/ola/acn/ACNFlags.h @@ -0,0 +1,70 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Library General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * ACNFlags.h + * Flags used in ACN PDUs + * Copyright (C) 2020 Peter Newman + */ + +#ifndef INCLUDE_OLA_ACN_ACNFLAGS_H_ +#define INCLUDE_OLA_ACN_ACNFLAGS_H_ + +/** + * @addtogroup acn + * @{ + * @file ACNFlags.h + * @brief ACN flag values. + * @} + */ + +#include + +namespace ola { +namespace acn { + +/** + * @addtogroup acn + * @{ + */ + +// masks for the flag fields +/** + * @brief This indicates a 20 bit length field (default is 12 bits) + */ +static const uint8_t LFLAG_MASK = 0x80; + +/** + * @brief This indicates a vector is present + */ +static const uint8_t VFLAG_MASK = 0x40; + +/** + * @brief This indicates a header field is present + */ +static const uint8_t HFLAG_MASK = 0x20; + +/** + * @brief This indicates a data field is present + */ +static const uint8_t DFLAG_MASK = 0x10; + +/** + * @} + */ + +} // namespace acn +} // namespace ola + +#endif // INCLUDE_OLA_ACN_ACNFLAGS_H_ diff --git a/include/ola/acn/Makefile.mk b/include/ola/acn/Makefile.mk index 56bc647eb5..afd628d812 100644 --- a/include/ola/acn/Makefile.mk +++ b/include/ola/acn/Makefile.mk @@ -3,6 +3,7 @@ olaacninclude_HEADERS = if INSTALL_ACN olaacninclude_HEADERS += \ + include/ola/acn/ACNFlags.h \ include/ola/acn/ACNPort.h \ include/ola/acn/ACNVectors.h \ include/ola/acn/CID.h diff --git a/libs/acn/BaseInflator.cpp b/libs/acn/BaseInflator.cpp index e3917967ad..a41dca370c 100644 --- a/libs/acn/BaseInflator.cpp +++ b/libs/acn/BaseInflator.cpp @@ -163,7 +163,7 @@ bool BaseInflator::DecodeLength(const uint8_t *data, bool BaseInflator::DecodeVector(uint8_t flags, const uint8_t *data, unsigned int length, uint32_t *vector, unsigned int *bytes_used) { - if (flags & PDU::VFLAG_MASK) { + if (flags & ola::acn::VFLAG_MASK) { if ((unsigned int) m_vector_size > length) { *vector = 0; *bytes_used = 0; @@ -223,7 +223,7 @@ bool BaseInflator::InflatePDU(HeaderSet *headers, return false; } - if (flags & PDU::HFLAG_MASK) { + if (flags & ola::acn::HFLAG_MASK) { result = DecodeHeader(headers, data + data_offset, pdu_len - data_offset, &header_bytes_used); diff --git a/libs/acn/BaseInflator.h b/libs/acn/BaseInflator.h index 384351bc28..07b7c6b88b 100644 --- a/libs/acn/BaseInflator.h +++ b/libs/acn/BaseInflator.h @@ -87,8 +87,11 @@ class BaseInflator : public InflatorInterface { unsigned int len); // masks for the flag fields - // This indicates a 20 bit length field (default is 12 bits) - static const uint8_t LFLAG_MASK = 0x80; + /** + * @brief This indicates a 20 bit length field (default is 12 bits) + * @deprecated Use ola::acn::LFLAG_MASK instead (4 Feb 2020). + */ + static const uint8_t LFLAG_MASK = ola::acn::LFLAG_MASK; // This masks the first 4 bits of the length field static const uint8_t LENGTH_MASK = 0x0F; diff --git a/libs/acn/BaseInflatorTest.cpp b/libs/acn/BaseInflatorTest.cpp index 17e86121b8..a4e0a0dd81 100644 --- a/libs/acn/BaseInflatorTest.cpp +++ b/libs/acn/BaseInflatorTest.cpp @@ -204,7 +204,7 @@ void BaseInflatorTest::testDecodeVector() { uint8_t data[] = {1, 2, 3, 4, 5, 6}; // the test data unsigned int vector = 1; unsigned int bytes_used = 0; - uint8_t flags = PDU::VFLAG_MASK; + uint8_t flags = VFLAG_MASK; OLA_ASSERT_FALSE(inflator.DecodeVector(flags, data, 0, &vector, &bytes_used)); OLA_ASSERT_EQ((unsigned int) 0, vector); @@ -235,7 +235,7 @@ void BaseInflatorTest::testDecodeVector() { } // now try with a vector size of 2 - flags = PDU::VFLAG_MASK; + flags = VFLAG_MASK; TestInflator inflator2(0, PDU::TWO_BYTES); for (unsigned int i = 0; i < 2; i++) { OLA_ASSERT_FALSE( @@ -270,7 +270,7 @@ void BaseInflatorTest::testDecodeVector() { } // now try with a vector size of 4 - flags = PDU::VFLAG_MASK; + flags = VFLAG_MASK; TestInflator inflator4(0, PDU::FOUR_BYTES); for (unsigned int i = 0; i < 4; i++) { OLA_ASSERT_FALSE( @@ -297,7 +297,7 @@ void BaseInflatorTest::testDecodeVector() { void BaseInflatorTest::testInflatePDU() { TestInflator inflator; // test with a vector size of 2 HeaderSet header_set; - uint8_t flags = PDU::VFLAG_MASK; + uint8_t flags = VFLAG_MASK; unsigned int data_size = static_cast(PDU::TWO_BYTES + sizeof(PDU_DATA)); uint8_t *data = new uint8_t[data_size]; @@ -324,7 +324,7 @@ void BaseInflatorTest::testInflatePDUBlock() { length_size + PDU::TWO_BYTES + sizeof(PDU_DATA)); uint8_t *data = new uint8_t[data_size]; // setup the vector - data[0] = PDU::VFLAG_MASK; + data[0] = VFLAG_MASK; data[1] = static_cast(data_size); data[2] = 0x01; data[3] = 0x21; @@ -337,14 +337,14 @@ void BaseInflatorTest::testInflatePDUBlock() { // inflate a multi-pdu block data = new uint8_t[2 * data_size]; - data[0] = PDU::VFLAG_MASK; + data[0] = VFLAG_MASK; data[1] = static_cast(data_size); data[2] = 0x01; data[3] = 0x21; memcpy(data + length_size + PDU::TWO_BYTES, PDU_DATA, sizeof(PDU_DATA)); - data[data_size] = PDU::VFLAG_MASK; + data[data_size] = VFLAG_MASK; data[data_size + 1] = static_cast(data_size); data[data_size + 2] = 0x01; data[data_size + 3] = 0x21; @@ -362,11 +362,11 @@ void BaseInflatorTest::testInflatePDUBlock() { unsigned int pdu_size = data_size + length_size + PDU::TWO_BYTES; data = new uint8_t[pdu_size]; - data[0] = PDU::VFLAG_MASK; + data[0] = VFLAG_MASK; data[1] = static_cast(pdu_size); data[2] = 0x01; data[3] = 0x21; - data[4] = PDU::VFLAG_MASK; + data[4] = VFLAG_MASK; data[5] = static_cast(data_size); data[6] = 0x01; data[7] = 0x21; diff --git a/libs/acn/LLRPInflatorTest.cpp b/libs/acn/LLRPInflatorTest.cpp index b36c5cdc05..16bcd96431 100644 --- a/libs/acn/LLRPInflatorTest.cpp +++ b/libs/acn/LLRPInflatorTest.cpp @@ -108,7 +108,7 @@ void LLRPInflatorTest::testInflatePDU() { LLRPHeader header(destination_cid, 2370); // TODO(Peter): pass a different type of msg here as well LLRPPDU pdu(3, header, NULL); - OLA_ASSERT_EQ((unsigned int) 26, pdu.Size()); + OLA_ASSERT_EQ((unsigned int) 27, pdu.Size()); unsigned int size = pdu.Size(); uint8_t *data = new uint8_t[size]; diff --git a/libs/acn/LLRPPDU.cpp b/libs/acn/LLRPPDU.cpp index e13fcce225..4fb3b3644f 100644 --- a/libs/acn/LLRPPDU.cpp +++ b/libs/acn/LLRPPDU.cpp @@ -112,8 +112,7 @@ void LLRPPDU::PrependPDU(ola::io::IOStack *stack, vector = HostToNetwork(vector); stack->Write(reinterpret_cast(&vector), sizeof(vector)); - // Flags for LLRP should always be 0xF0 - PrependFlagsAndLength(stack, 0xf0); + PrependFlagsAndLength(stack, VFLAG_MASK | HFLAG_MASK | DFLAG_MASK, true); } } // namespace acn } // namespace ola diff --git a/libs/acn/LLRPPDU.h b/libs/acn/LLRPPDU.h index 76ccdc7d65..a2636c1a9c 100644 --- a/libs/acn/LLRPPDU.h +++ b/libs/acn/LLRPPDU.h @@ -37,7 +37,7 @@ class LLRPPDU: public PDU { LLRPPDU(unsigned int vector, const LLRPHeader &header, const PDU *pdu): - PDU(vector), + PDU(vector, FOUR_BYTES, true), m_header(header), m_pdu(pdu) {} ~LLRPPDU() {} @@ -50,8 +50,10 @@ class LLRPPDU: public PDU { void PackHeader(ola::io::OutputStream *stream) const; void PackData(ola::io::OutputStream *stream) const; - static void PrependPDU(ola::io::IOStack *stack, uint32_t vector, - const ola::acn::CID &destination_cid, uint32_t transaction_number); + static void PrependPDU(ola::io::IOStack *stack, + uint32_t vector, + const ola::acn::CID &destination_cid, + uint32_t transaction_number); private: LLRPHeader m_header; diff --git a/libs/acn/LLRPPDUTest.cpp b/libs/acn/LLRPPDUTest.cpp index af85b283e5..01c30db22a 100644 --- a/libs/acn/LLRPPDUTest.cpp +++ b/libs/acn/LLRPPDUTest.cpp @@ -73,7 +73,7 @@ void LLRPPDUTest::testSimpleLLRPPDU() { OLA_ASSERT_EQ(20u, pdu.HeaderSize()); OLA_ASSERT_EQ(0u, pdu.DataSize()); - OLA_ASSERT_EQ(26u, pdu.Size()); + OLA_ASSERT_EQ(27u, pdu.Size()); unsigned int size = pdu.Size(); uint8_t *data = new uint8_t[size]; @@ -82,20 +82,21 @@ void LLRPPDUTest::testSimpleLLRPPDU() { OLA_ASSERT_EQ(size, bytes_used); // spot check the data - OLA_ASSERT_EQ((uint8_t) 0x70, data[0]); - OLA_ASSERT_EQ((uint8_t) bytes_used, data[1]); + OLA_ASSERT_EQ((uint8_t) 0xf0, data[0]); + // bytes_used is technically data[1] and data[2] if > 255 + OLA_ASSERT_EQ((uint8_t) bytes_used, data[2]); unsigned int actual_value; - memcpy(&actual_value, data + 2, sizeof(actual_value)); + memcpy(&actual_value, data + 3, sizeof(actual_value)); OLA_ASSERT_EQ(HostToNetwork(TEST_VECTOR), actual_value); uint8_t buffer[CID::CID_LENGTH]; destination_cid.Pack(buffer); - OLA_ASSERT_FALSE(memcmp(&data[6], buffer, CID::CID_LENGTH)); + OLA_ASSERT_DATA_EQUALS(&data[7], CID::CID_LENGTH, buffer, sizeof(buffer)); // transaction number - OLA_ASSERT_EQ((uint8_t) 0, data[6 + CID::CID_LENGTH]); - OLA_ASSERT_EQ((uint8_t) 0, data[6 + CID::CID_LENGTH + 1]); - OLA_ASSERT_EQ((uint8_t) 0, data[6 + CID::CID_LENGTH + 2]); - OLA_ASSERT_EQ((uint8_t) 101, data[6 + CID::CID_LENGTH + 3]); + OLA_ASSERT_EQ((uint8_t) 0, data[7 + CID::CID_LENGTH]); + OLA_ASSERT_EQ((uint8_t) 0, data[7 + CID::CID_LENGTH + 1]); + OLA_ASSERT_EQ((uint8_t) 0, data[7 + CID::CID_LENGTH + 2]); + OLA_ASSERT_EQ((uint8_t) 101, data[7 + CID::CID_LENGTH + 3]); // test undersized buffer bytes_used = size - 1; @@ -120,19 +121,19 @@ void LLRPPDUTest::testSimpleLLRPPDUToOutputStream() { OLA_ASSERT_EQ(20u, pdu.HeaderSize()); OLA_ASSERT_EQ(0u, pdu.DataSize()); - OLA_ASSERT_EQ(26u, pdu.Size()); + OLA_ASSERT_EQ(27u, pdu.Size()); IOQueue output; OutputStream stream(&output); pdu.Write(&stream); - OLA_ASSERT_EQ(26u, output.Size()); + OLA_ASSERT_EQ(27u, output.Size()); uint8_t *pdu_data = new uint8_t[output.Size()]; unsigned int pdu_size = output.Peek(pdu_data, output.Size()); OLA_ASSERT_EQ(output.Size(), pdu_size); uint8_t EXPECTED[] = { - 0xf0, 0x1a, + 0xf0, 0x00, 0x1b, 0, 0, 0, 39, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, diff --git a/libs/acn/PDU.cpp b/libs/acn/PDU.cpp index c9166db6cb..0f256eca13 100644 --- a/libs/acn/PDU.cpp +++ b/libs/acn/PDU.cpp @@ -35,8 +35,9 @@ using ola::network::HostToNetwork; unsigned int PDU::Size() const { unsigned int length = m_vector_size + HeaderSize() + DataSize(); - if (length > TWOB_LENGTH_LIMIT - 2) + if ((length > TWOB_LENGTH_LIMIT - 2) || m_force_length_flag) { length += 1; + } length += 2; return length; } @@ -59,7 +60,7 @@ bool PDU::Pack(uint8_t *buffer, unsigned int *length) const { return false; } - if (size <= TWOB_LENGTH_LIMIT) { + if (size <= TWOB_LENGTH_LIMIT && !m_force_length_flag) { buffer[0] = (uint8_t) ((size & 0x0f00) >> 8); buffer[1] = (uint8_t) (size & 0xff); } else { @@ -69,6 +70,11 @@ bool PDU::Pack(uint8_t *buffer, unsigned int *length) const { offset += 1; } + if (m_force_length_flag) { + // TODO(Peter): Should this happen regardless of the force when we're + // writing 20 bits of length? + buffer[0] |= LFLAG_MASK; + } buffer[0] |= VFLAG_MASK; buffer[0] |= HFLAG_MASK; buffer[0] |= DFLAG_MASK; @@ -117,13 +123,18 @@ bool PDU::Pack(uint8_t *buffer, unsigned int *length) const { void PDU::Write(OutputStream *stream) const { unsigned int size = Size(); - if (size <= TWOB_LENGTH_LIMIT) { + if (size <= TWOB_LENGTH_LIMIT && !m_force_length_flag) { uint16_t flags_and_length = static_cast(size); flags_and_length |= (VFLAG_MASK | HFLAG_MASK | DFLAG_MASK) << 8u; *stream << HostToNetwork(flags_and_length); } else { uint8_t vhl_flags = static_cast((size & 0x0f0000) >> 16); vhl_flags |= VFLAG_MASK | HFLAG_MASK | DFLAG_MASK; + if (m_force_length_flag) { + // TODO(Peter): Should this happen regardless of the force as we're + // writing 20 bits of length? + vhl_flags |= LFLAG_MASK; + } *stream << vhl_flags; *stream << (uint8_t) ((size & 0xff00) >> 8); *stream << (uint8_t) (size & 0xff); @@ -150,8 +161,9 @@ void PDU::Write(OutputStream *stream) const { * Prepend the flags and length to an OutputBufferInterface. */ void PDU::PrependFlagsAndLength(ola::io::OutputBufferInterface *output, - uint8_t flags) { - PrependFlagsAndLength(output, output->Size(), flags); + uint8_t flags, + bool force_length_flag) { + PrependFlagsAndLength(output, output->Size(), flags, force_length_flag); } @@ -160,8 +172,9 @@ void PDU::PrependFlagsAndLength(ola::io::OutputBufferInterface *output, */ void PDU::PrependFlagsAndLength(ola::io::OutputBufferInterface *output, unsigned int size, - uint8_t flags) { - if (size + 2 <= TWOB_LENGTH_LIMIT) { + uint8_t flags, + bool force_length_flag) { + if (size + 2 <= TWOB_LENGTH_LIMIT && !force_length_flag) { size += 2; uint16_t flags_and_length = static_cast(size); flags_and_length |= static_cast(flags << 8u); @@ -173,6 +186,11 @@ void PDU::PrependFlagsAndLength(ola::io::OutputBufferInterface *output, uint8_t flags_and_length[3]; flags_and_length[0] = static_cast((size & 0x0f0000) >> 16); flags_and_length[0] |= flags; + if (force_length_flag) { + // TODO(Peter): Should this happen regardless of the force as we're + // writing 20 bits of length? + flags_and_length[0] |= LFLAG_MASK; + } flags_and_length[1] = static_cast((size & 0xff00) >> 8); flags_and_length[2] = static_cast(size & 0xff); output->Write(flags_and_length, sizeof(flags_and_length)); diff --git a/libs/acn/PDU.h b/libs/acn/PDU.h index 64c1e82adb..2be285dead 100644 --- a/libs/acn/PDU.h +++ b/libs/acn/PDU.h @@ -22,6 +22,7 @@ #define LIBS_ACN_PDU_H_ #include +#include #include #include #include @@ -41,9 +42,12 @@ class PDU { FOUR_BYTES = 4, } vector_size; - explicit PDU(unsigned int vector, vector_size size = FOUR_BYTES): + explicit PDU(unsigned int vector, + vector_size size = FOUR_BYTES, + bool force_length_flag = false): m_vector(vector), - m_vector_size(size) {} + m_vector_size(size), + m_force_length_flag(force_length_flag) {} virtual ~PDU() {} // Returns the size of this PDU @@ -72,23 +76,36 @@ class PDU { static void PrependFlagsAndLength( ola::io::OutputBufferInterface *output, - uint8_t flags = VFLAG_MASK | HFLAG_MASK | DFLAG_MASK); + uint8_t flags = VFLAG_MASK | HFLAG_MASK | DFLAG_MASK, + bool force_length_flag = false); static void PrependFlagsAndLength( ola::io::OutputBufferInterface *output, unsigned int length, - uint8_t flags); + uint8_t flags, + bool force_length_flag = false); + + /** + * @brief This indicates a vector is present. + * @deprecated Use ola::acn::VFLAG_MASK instead (4 Feb 2020). + */ + static const uint8_t VFLAG_MASK = ola::acn::VFLAG_MASK; + /** + * @brief This indicates a header field is present. + * @deprecated Use ola::acn::HFLAG_MASK instead (4 Feb 2020). + */ + static const uint8_t HFLAG_MASK = ola::acn::HFLAG_MASK; + /** + * @brief This indicates a data field is present. + * @deprecated Use ola::acn::DFLAG_MASK instead (4 Feb 2020). + */ + static const uint8_t DFLAG_MASK = ola::acn::DFLAG_MASK; - // This indicates a vector is present - static const uint8_t VFLAG_MASK = 0x40; - // This indicates a header field is present - static const uint8_t HFLAG_MASK = 0x20; - // This indicates a data field is present - static const uint8_t DFLAG_MASK = 0x10; private: unsigned int m_vector; unsigned int m_vector_size; + bool m_force_length_flag; // The max PDU length that can be represented with the 2 byte format for // the length field.