Fix some post-landing review comments for rotating device id.

src/setup_payload/AdditionalDataPayloadGenerator.cpp

@@ -60,24 +60,24 @@ AdditionalDataPayloadGenerator::generateAdditionalDataPayload(uint16_t lifetimeC
     if (additionalDataFields.Has(AdditionalDataFields::RotatingDeviceId))
     {
         uint8_t rotatingDeviceIdInternalBuffer[RotatingDeviceId::kMaxLength];
-        MutableByteSpan rotatingDeviceIdBuffer = MutableByteSpan(rotatingDeviceIdInternalBuffer);
+        MutableByteSpan rotatingDeviceIdBuffer(rotatingDeviceIdInternalBuffer);
 
         // Generating Device Rotating Id
-        ReturnErrorOnFailure(generateRotatingDeviceIdAsBinary(lifetimeCounter, serialNumberBuffer, serialNumberBufferSize,
-                                                              rotatingDeviceIdBuffer));
+        ReturnErrorOnFailure(
+            generateRotatingDeviceIdAsBinary(lifetimeCounter, serialNumberBuffer, serialNumberBufferSize, rotatingDeviceIdBuffer));
         // Adding the rotating device id to the TLV data
-        ReturnErrorOnFailure(innerWriter.PutBytes(ContextTag(kRotatingDeviceIdTag),
-                                                  rotatingDeviceIdBuffer.data(),
-                                                  static_cast<uint32_t>(rotatingDeviceIdBuffer.size())));
+        ReturnErrorOnFailure(innerWriter.Put(ContextTag(kRotatingDeviceIdTag), rotatingDeviceIdBuffer));
     }
 
     ReturnErrorOnFailure(writer.CloseContainer(innerWriter));
 
     return writer.Finalize(&bufferHandle);
 }
 
-CHIP_ERROR AdditionalDataPayloadGenerator::generateRotatingDeviceIdAsBinary(
-    uint16_t lifetimeCounter, const char * serialNumberBuffer, size_t serialNumberBufferSize, MutableByteSpan & rotatingDeviceIdBuffer)
+CHIP_ERROR AdditionalDataPayloadGenerator::generateRotatingDeviceIdAsBinary(uint16_t lifetimeCounter,
+                                                                            const char * serialNumberBuffer,
+                                                                            size_t serialNumberBufferSize,
+                                                                            MutableByteSpan & rotatingDeviceIdBuffer)
 {
     uint8_t hashOutputBuffer[kSHA256_Hash_Length];
     BufferWriter outputBufferWriter(rotatingDeviceIdBuffer);
@@ -113,9 +113,9 @@ CHIP_ERROR AdditionalDataPayloadGenerator::generateRotatingDeviceIdAsHexString(
     size_t rotatingDeviceIdBufferSize, size_t & rotatingDeviceIdValueOutputSize)
 {
     uint8_t rotatingDeviceIdInternalBuffer[RotatingDeviceId::kMaxLength];
-    MutableByteSpan rotatingDeviceIdBufferTemp = MutableByteSpan(rotatingDeviceIdInternalBuffer);
-    ReturnErrorOnFailure(generateRotatingDeviceIdAsBinary(lifetimeCounter, serialNumberBuffer, serialNumberBufferSize,
-                                                          rotatingDeviceIdBufferTemp));
+    MutableByteSpan rotatingDeviceIdBufferTemp(rotatingDeviceIdInternalBuffer);
+    ReturnErrorOnFailure(
+        generateRotatingDeviceIdAsBinary(lifetimeCounter, serialNumberBuffer, serialNumberBufferSize, rotatingDeviceIdBufferTemp));
 
     VerifyOrReturnError(rotatingDeviceIdBufferSize >= RotatingDeviceId::kHexMaxLength, CHIP_ERROR_BUFFER_TOO_SMALL);
     ReturnErrorOnFailure(BytesToUppercaseHexString(rotatingDeviceIdBufferTemp.data(), rotatingDeviceIdBufferTemp.size(),

src/setup_payload/AdditionalDataPayloadGenerator.h

@@ -78,10 +78,9 @@ class AdditionalDataPayloadGenerator
      * @param lifetimeCounter lifetime counter
      * @param serialNumberBuffer null-terminated serial number buffer
      * @param serialNumberBufferSize size of the serial number buffer supplied.
-     * @param rotatingDeviceIdBuffer rotating device id mutable byte span, it will be resized to the actual size used upon successful generation
-     *
-     * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise.
-     *
+     * @param [in,out] rotatingDeviceIdBuffer as input, the buffer to use for
+     *                 the binary data.  As output, will have its size set to
+     *                 the actual size used upon successful generation
      */
     CHIP_ERROR generateRotatingDeviceIdAsBinary(uint16_t lifetimeCounter, const char * serialNumberBuffer,
                                                 size_t serialNumberBufferSize, MutableByteSpan & rotatingDeviceIdBuffer);

src/setup_payload/AdditionalDataPayloadParser.cpp

@@ -53,12 +53,12 @@ CHIP_ERROR AdditionalDataPayloadParser::populatePayload(SetupPayloadData::Additi
         ByteSpan rotatingDeviceId;
         ReturnErrorOnFailure(innerReader.GetByteView(rotatingDeviceId));
 
-        VerifyOrReturnError(rotatingDeviceId.size() == RotatingDeviceId::kMaxLength, CHIP_ERROR_INVALID_STRING_LENGTH);
+        VerifyOrReturnError(rotatingDeviceId.size() <= RotatingDeviceId::kMaxLength, CHIP_ERROR_INVALID_STRING_LENGTH);
         char rotatingDeviceIdBufferTemp[RotatingDeviceId::kHexMaxLength];
 
         ReturnErrorOnFailure(Encoding::BytesToUppercaseHexString(rotatingDeviceId.data(), rotatingDeviceId.size(),
                                                                  rotatingDeviceIdBufferTemp, RotatingDeviceId::kHexMaxLength));
-        outPayload.rotatingDeviceId = std::string(rotatingDeviceIdBufferTemp, RotatingDeviceId::kHexMaxLength);
+        outPayload.rotatingDeviceId = std::string(rotatingDeviceIdBufferTemp, rotatingDeviceId.size() * 2);
     }
     else
     {

src/setup_payload/tests/TestAdditionalDataPayload.cpp

@@ -47,8 +47,10 @@ constexpr char kAdditionalDataPayloadWithoutRotatingDeviceId[]           = "1518
 constexpr char kAdditionalDataPayloadWithRotatingDeviceId[]              = "153000120A001998AB7130E38B7E9A401CFE9F7B79AF18";
 constexpr char kAdditionalDataPayloadWithInvalidRotatingDeviceIdLength[] = "153000FF0A001998AB7130E38B7E9A401CFE9F7B79AF18";
 constexpr char kAdditionalDataPayloadWithLongRotatingDeviceId[]          = "153000130A00191998AB7130E38B7E9A401CFE9F7B79AF18";
+constexpr char kAdditionalDataPayloadWithShortRotatingDeviceId[]         = "153000110A001998AB7130E38B7E9A401CFE9F7B7918";
 constexpr char kAdditionalDataPayloadWithRotatingDeviceIdAndMaxLifetimeCounter[] = "15300012FFFFFC1670A9F9666D1C4587FCBC4811549018";
 constexpr char kRotatingDeviceId[]                                               = "0A001998AB7130E38B7E9A401CFE9F7B79AF";
+constexpr char kShortRotatingDeviceId[]                                          = "0A001998AB7130E38B7E9A401CFE9F7B79";
 constexpr uint16_t kLifetimeCounter                                              = 10;
 constexpr uint16_t kAdditionalDataPayloadLength                                  = 51;
 constexpr uint16_t kShortRotatingIdLength                                        = 5;
@@ -221,6 +223,16 @@ void TestParsingAdditionalDataPayloadWithLongRotatingDeviceId(nlTestSuite * inSu
                                               resultPayload) == CHIP_ERROR_INVALID_STRING_LENGTH);
 }
 
+void TestParsingAdditionalDataPayloadWithShortRotatingDeviceId(nlTestSuite * inSuite, void * inContext)
+{
+    chip::SetupPayloadData::AdditionalDataPayload resultPayload;
+    NL_TEST_ASSERT(inSuite,
+                   ParseAdditionalDataPayload(kAdditionalDataPayloadWithShortRotatingDeviceId,
+                                              strlen(kAdditionalDataPayloadWithShortRotatingDeviceId),
+                                              resultPayload) == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, strcmp(resultPayload.rotatingDeviceId.c_str(), kShortRotatingDeviceId) == 0);
+}
+
 /**
  *  Test Suite that lists all the Test functions.
  */
@@ -238,6 +250,7 @@ const nlTest sTests[] =
     NL_TEST_DEF("Test Parsing Additional Data Payload without Rotating Device Id", TestParsingAdditionalDataPayloadWithoutRotatingDeviceId),
     NL_TEST_DEF("Test Parsing Additional Data Payload with Invalid Rotating Device Id Length", TestParsingAdditionalDataPayloadWithInvalidRotatingDeviceIdLength),
     NL_TEST_DEF("Test Parsing Additional Data Payload with Long Rotating Device Id", TestParsingAdditionalDataPayloadWithLongRotatingDeviceId),
+    NL_TEST_DEF("Test Parsing Additional Data Payload with Short Rotating Device Id", TestParsingAdditionalDataPayloadWithShortRotatingDeviceId),
     NL_TEST_SENTINEL()
 };
 // clang-format on