Add LLRP Probe Request PDU, fix the length flags bug

(cherry picked from commit e9dd374)

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 <stdint.h>
+
+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_

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

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);

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;
 

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<unsigned int>(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<uint8_t>(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<uint8_t>(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<uint8_t>(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<uint8_t>(pdu_size);
   data[2] = 0x01;
   data[3] = 0x21;
-  data[4] = PDU::VFLAG_MASK;
+  data[4] = VFLAG_MASK;
   data[5] = static_cast<uint8_t>(data_size);
   data[6] = 0x01;
   data[7] = 0x21;

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];

libs/acn/LLRPPDU.cpp

@@ -112,8 +112,7 @@ void LLRPPDU::PrependPDU(ola::io::IOStack *stack,
 
   vector = HostToNetwork(vector);
   stack->Write(reinterpret_cast<uint8_t*>(&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

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;

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,

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<uint16_t>(size);
     flags_and_length |= (VFLAG_MASK | HFLAG_MASK | DFLAG_MASK) << 8u;
     *stream << HostToNetwork(flags_and_length);
   } else {
     uint8_t vhl_flags = static_cast<uint8_t>((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<uint16_t>(size);
     flags_and_length |= static_cast<uint16_t>(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<uint8_t>((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<uint8_t>((size & 0xff00) >> 8);
     flags_and_length[2] = static_cast<uint8_t>(size & 0xff);
     output->Write(flags_and_length, sizeof(flags_and_length));