Switch device type to use a uint32 (#15918)

* Switch device type to use a uint32 * fix print size. * Update src/lib/dnssd/tests/TestTxtFields.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
project-chip · Mar 15, 2022 · c2b6479 · c2b6479
1 parent eac433a
commit c2b6479
Show file tree

Hide file tree

Showing 15 changed files with 35 additions and 37 deletions.
diff --git a/examples/minimal-mdns/advertiser.cpp b/examples/minimal-mdns/advertiser.cpp
@@ -45,7 +45,7 @@ struct Options
     // commissionable node / commissioner params
     Optional<uint16_t> vendorId;
     Optional<uint16_t> productId;
-    Optional<uint16_t> deviceType;
+    Optional<uint32_t> deviceType;
     Optional<const char *> deviceName;
 
     // commissionable node params
@@ -134,7 +134,7 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
         }
         return true;
     case kOptionCommissioningDeviceType:
-        gOptions.deviceType = Optional<uint16_t>::Value(static_cast<uint16_t>(atoi(aValue)));
+        gOptions.deviceType = Optional<uint32_t>::Value(static_cast<uint32_t>(atoi(aValue)));
         return true;
     case kOptionCommissioningDeviceName:
         gOptions.deviceName = Optional<const char *>::Value(static_cast<const char *>(aValue));

diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp
@@ -321,6 +321,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
     advertiseParameters.SetMac(mac);
 
     uint16_t value;
+    uint32_t val32;
     if (DeviceLayer::ConfigurationMgr().GetVendorId(value) != CHIP_NO_ERROR)
     {
         ChipLogProgress(Discovery, "Vendor ID not known");
@@ -356,9 +357,9 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
         .SetLongDiscriminator(discriminator);
 
     if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceTypeEnabled() &&
-        DeviceLayer::ConfigurationMgr().GetDeviceTypeId(value) == CHIP_NO_ERROR)
+        DeviceLayer::ConfigurationMgr().GetDeviceTypeId(val32) == CHIP_NO_ERROR)
     {
-        advertiseParameters.SetDeviceType(chip::Optional<uint16_t>::Value(value));
+        advertiseParameters.SetDeviceType(chip::Optional<uint32_t>::Value(val32));
     }
 
     char deviceName[chip::Dnssd::kKeyDeviceNameMaxLength + 1];

diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h
@@ -152,7 +152,7 @@ class ConfigurationManager
     virtual void LogDeviceConfig() = 0;
 
     virtual bool IsCommissionableDeviceTypeEnabled()                              = 0;
-    virtual CHIP_ERROR GetDeviceTypeId(uint16_t & deviceType)                     = 0;
+    virtual CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType)                     = 0;
     virtual bool IsCommissionableDeviceNameEnabled()                              = 0;
     virtual CHIP_ERROR GetCommissionableDeviceName(char * buf, size_t bufSize)    = 0;
     virtual CHIP_ERROR GetInitialPairingHint(uint16_t & pairingHint)              = 0;

diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp
@@ -281,9 +281,9 @@ inline CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSoftwareVer
 }
 
 template <class ConfigClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetDeviceTypeId(uint16_t & deviceType)
+CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetDeviceTypeId(uint32_t & deviceType)
 {
-    deviceType = static_cast<uint16_t>(CHIP_DEVICE_CONFIG_DEVICE_TYPE);
+    deviceType = static_cast<uint32_t>(CHIP_DEVICE_CONFIG_DEVICE_TYPE);
     return CHIP_NO_ERROR;
 }
 
@@ -864,12 +864,12 @@ void GenericConfigurationManagerImpl<ConfigClass>::LogDeviceConfig()
     }
 
     {
-        uint16_t deviceType;
+        uint32_t deviceType;
         if (GetDeviceTypeId(deviceType) != CHIP_NO_ERROR)
         {
             deviceType = 0;
         }
-        ChipLogProgress(DeviceLayer, "  Device Type: %" PRIu16 " (0x%" PRIX16 ")", deviceType, deviceType);
+        ChipLogProgress(DeviceLayer, "  Device Type: %" PRIu32 " (0x%" PRIX32 ")", deviceType, deviceType);
     }
 }
 

diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.h b/src/include/platform/internal/GenericConfigurationManagerImpl.h
@@ -87,7 +87,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR SetFailSafeArmed(bool val) override;
     CHIP_ERROR GetBLEDeviceIdentificationInfo(Ble::ChipBLEDeviceIdentificationInfo & deviceIdInfo) override;
     bool IsCommissionableDeviceTypeEnabled() override;
-    CHIP_ERROR GetDeviceTypeId(uint16_t & deviceType) override;
+    CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType) override;
     bool IsCommissionableDeviceNameEnabled() override;
     CHIP_ERROR GetCommissionableDeviceName(char * buf, size_t bufSize) override;
     CHIP_ERROR GetInitialPairingHint(uint16_t & pairingHint) override;

diff --git a/src/lib/dnssd/Advertiser.h b/src/lib/dnssd/Advertiser.h
@@ -180,12 +180,12 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams<CommissionA
     }
     CommissioningMode GetCommissioningMode() const { return mCommissioningMode; }
 
-    CommissionAdvertisingParameters & SetDeviceType(Optional<uint16_t> deviceType)
+    CommissionAdvertisingParameters & SetDeviceType(Optional<uint32_t> deviceType)
     {
         mDeviceType = deviceType;
         return *this;
     }
-    Optional<uint16_t> GetDeviceType() const { return mDeviceType; }
+    Optional<uint32_t> GetDeviceType() const { return mDeviceType; }
 
     CommissionAdvertisingParameters & SetDeviceName(Optional<const char *> deviceName)
     {
@@ -262,7 +262,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams<CommissionA
     CommissioningMode mCommissioningMode = CommissioningMode::kEnabledBasic;
     chip::Optional<uint16_t> mVendorId;
     chip::Optional<uint16_t> mProductId;
-    chip::Optional<uint16_t> mDeviceType;
+    chip::Optional<uint32_t> mDeviceType;
     chip::Optional<uint16_t> mPairingHint;
 
     char mDeviceName[kKeyDeviceNameMaxLength + 1];

diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
@@ -657,7 +657,7 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis
     char txtDeviceType[chip::Dnssd::kKeyDeviceTypeMaxLength + 4];
     if (params.GetDeviceType().HasValue())
     {
-        snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%d", params.GetDeviceType().Value());
+        snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, params.GetDeviceType().Value());
         txtFields[numTxtFields++] = txtDeviceType;
     }
 

diff --git a/src/lib/dnssd/Constants.h b/src/lib/dnssd/Constants.h
@@ -38,7 +38,7 @@ constexpr size_t kHostNameMaxLength = 16; // MAC or 802.15.4 Extended Address in
 constexpr size_t kSubTypeShortDiscriminatorMaxLength      = 4;  // _S<dd>
 constexpr size_t kSubTypeLongDiscriminatorMaxLength       = 6;  // _L<dddd>
 constexpr size_t kSubTypeVendorIdMaxLength                = 7;  // _V<ddddd>
-constexpr size_t kSubTypeDeviceTypeMaxLength              = 5;  // _T<ddd>
+constexpr size_t kSubTypeDeviceTypeMaxLength              = 12; // _T<dddddddddd>
 constexpr size_t kSubTypeCommissioningModeMaxLength       = 3;  // _C<d>
 constexpr size_t kSubTypeAdditionalCommissioningMaxLength = 3;  // _A<d>
 constexpr size_t kSubTypeCompressedFabricIdMaxLength      = 18; // _I<16-hex-digits>

diff --git a/src/lib/dnssd/ServiceNaming.cpp b/src/lib/dnssd/ServiceNaming.cpp
@@ -121,8 +121,7 @@ CHIP_ERROR MakeServiceSubtype(char * buffer, size_t bufferLen, DiscoveryFilter s
         requiredSize = snprintf(buffer, bufferLen, "_V%" PRIu16, static_cast<uint16_t>(subtype.code));
         break;
     case DiscoveryFilterType::kDeviceType:
-        // TODO: Not totally clear the size required here: see spec issue #3226
-        requiredSize = snprintf(buffer, bufferLen, "_T%" PRIu16, static_cast<uint16_t>(subtype.code));
+        requiredSize = snprintf(buffer, bufferLen, "_T%" PRIu32, static_cast<uint32_t>(subtype.code));
         break;
     case DiscoveryFilterType::kCommissioningMode:
         requiredSize = snprintf(buffer, bufferLen, "_CM");

diff --git a/src/lib/dnssd/TxtFields.cpp b/src/lib/dnssd/TxtFields.cpp
@@ -145,10 +145,9 @@ uint8_t GetCommissioningMode(const ByteSpan & value)
     return MakeU8FromAsciiDecimal(value);
 }
 
-// TODO: possibly 32-bit? see spec issue #3226
-uint16_t GetDeviceType(const ByteSpan & value)
+uint32_t GetDeviceType(const ByteSpan & value)
 {
-    return MakeU16FromAsciiDecimal(value);
+    return MakeU32FromAsciiDecimal(value);
 }
 
 void GetDeviceName(const ByteSpan & value, char * name)

diff --git a/src/lib/dnssd/TxtFields.h b/src/lib/dnssd/TxtFields.h
@@ -41,7 +41,7 @@ static constexpr size_t kKeyLongDiscriminatorMaxLength       = 5;
 static constexpr size_t kKeyVendorProductMaxLength           = 11;
 static constexpr size_t kKeyAdditionalCommissioningMaxLength = 1;
 static constexpr size_t kKeyCommissioningModeMaxLength       = 1;
-static constexpr size_t kKeyDeviceTypeMaxLength              = 5;
+static constexpr size_t kKeyDeviceTypeMaxLength              = 10;
 static constexpr size_t kKeyDeviceNameMaxLength              = 32;
 static constexpr size_t kKeyRotatingDeviceIdMaxLength        = 100;
 static constexpr size_t kKeyPairingInstructionMaxLength      = 128;
@@ -104,8 +104,7 @@ uint16_t GetProduct(const ByteSpan & value);
 uint16_t GetVendor(const ByteSpan & value);
 uint16_t GetLongDiscriminator(const ByteSpan & value);
 uint8_t GetCommissioningMode(const ByteSpan & value);
-// TODO: possibly 32-bit? see spec issue #3226
-uint16_t GetDeviceType(const ByteSpan & value);
+uint32_t GetDeviceType(const ByteSpan & value);
 void GetDeviceName(const ByteSpan & value, char * name);
 void GetRotatingDeviceId(const ByteSpan & value, uint8_t * rotatingId, size_t * len);
 uint16_t GetPairingHint(const ByteSpan & value);

diff --git a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
@@ -106,7 +106,7 @@ const QNamePart kCmSubParts[]                          = { "_CM", "_sub", "_matt
 const QNamePart kLongSubParts[]                        = { "_L22", "_sub", "_matterc", "_udp", "local" };
 const QNamePart kShortSubParts[]                       = { "_S2", "_sub", "_matterc", "_udp", "local" };
 const QNamePart kVendorSubParts[]                      = { "_V555", "_sub", "_matterc", "_udp", "local" };
-const QNamePart kDeviceTypeSubParts[]                  = { "_T25", "_sub", "_matterc", "_udp", "local" };
+const QNamePart kDeviceTypeSubParts[]                  = { "_T70000", "_sub", "_matterc", "_udp", "local" };
 const FullQName kMatterCommissionableNodeQueryName     = FullQName(kMatterCommissionableNodeQueryParts);
 FullQName kLongSubFullLenName                          = FullQName(kLongSubPartsFullLen);
 FullQName kShortSubFullLenName                         = FullQName(kShortSubPartsFullLen);
@@ -150,14 +150,14 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic =
         .SetLongDiscriminator(22)
         .SetShortDiscriminator(2)
         .SetVendorId(chip::Optional<uint16_t>(555))
-        .SetDeviceType(chip::Optional<uint16_t>(25))
+        .SetDeviceType(chip::Optional<uint32_t>(70000))
         .SetCommissioningMode(CommissioningMode::kEnabledBasic)
         .SetDeviceName(chip::Optional<const char *>("testy-test"))
         .SetPairingHint(chip::Optional<uint16_t>(3))
         .SetPairingInstruction(chip::Optional<const char *>("Pair me"))
         .SetProductId(chip::Optional<uint16_t>(897))
         .SetRotatingDeviceId(chip::Optional<const char *>("id_that_spins"));
-QNamePart txtCommissionableNodeParamsLargeBasicParts[] = { "D=22",          "VP=555+897",       "CM=1",       "DT=25",
+QNamePart txtCommissionableNodeParamsLargeBasicParts[] = { "D=22",          "VP=555+897",       "CM=1",       "DT=70000",
                                                            "DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3" };
 FullQName txtCommissionableNodeParamsLargeBasicName    = FullQName(txtCommissionableNodeParamsLargeBasicParts);
 TxtResourceRecord txtCommissionableNodeParamsLargeBasic =
@@ -170,7 +170,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced =
         .SetLongDiscriminator(22)
         .SetShortDiscriminator(2)
         .SetVendorId(chip::Optional<uint16_t>(555))
-        .SetDeviceType(chip::Optional<uint16_t>(25))
+        .SetDeviceType(chip::Optional<uint32_t>(70000))
         .SetCommissioningMode(CommissioningMode::kEnabledEnhanced)
         .SetDeviceName(chip::Optional<const char *>("testy-test"))
         .SetPairingHint(chip::Optional<uint16_t>(3))
@@ -180,7 +180,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced =
         .SetTcpSupported(chip::Optional<bool>(true))
         // 3600005 is more than the max so should be adjusted down
         .SetMRPConfig(ReliableMessageProtocolConfig(3600000_ms32, 3600005_ms32));
-QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22",          "VP=555+897",       "CM=2",       "DT=25",
+QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22",          "VP=555+897",       "CM=2",       "DT=70000",
                                                               "DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3",
                                                               "CRA=3600000",   "CRI=3600000",      "T=1" };
 FullQName txtCommissionableNodeParamsLargeEnhancedName    = FullQName(txtCommissionableNodeParamsLargeEnhancedParts);

diff --git a/src/lib/dnssd/platform/tests/TestPlatform.cpp b/src/lib/dnssd/platform/tests/TestPlatform.cpp
@@ -86,7 +86,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic =
         .SetLongDiscriminator(22)
         .SetShortDiscriminator(2)
         .SetVendorId(chip::Optional<uint16_t>(555))
-        .SetDeviceType(chip::Optional<uint16_t>(25))
+        .SetDeviceType(chip::Optional<uint32_t>(70000))
         .SetCommissioningMode(CommissioningMode::kEnabledBasic)
         .SetDeviceName(chip::Optional<const char *>("testy-test"))
         .SetPairingHint(chip::Optional<uint16_t>(3))
@@ -104,7 +104,7 @@ test::ExpectedCall commissionableLargeBasic = test::ExpectedCall()
                                                   .AddTxt("D", "22")
                                                   .AddTxt("VP", "555+897")
                                                   .AddTxt("CM", "1")
-                                                  .AddTxt("DT", "25")
+                                                  .AddTxt("DT", "70000")
                                                   .AddTxt("DN", "testy-test")
                                                   .AddTxt("RI", "id_that_spins")
                                                   .AddTxt("PI", "Pair me")
@@ -115,7 +115,7 @@ test::ExpectedCall commissionableLargeBasic = test::ExpectedCall()
                                                   .AddSubtype("_S2")
                                                   .AddSubtype("_L22")
                                                   .AddSubtype("_V555")
-                                                  .AddSubtype("_T25")
+                                                  .AddSubtype("_T70000")
                                                   .AddSubtype("_CM");
 CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced =
     CommissionAdvertisingParameters()
@@ -124,7 +124,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced =
         .SetLongDiscriminator(22)
         .SetShortDiscriminator(2)
         .SetVendorId(chip::Optional<uint16_t>(555))
-        .SetDeviceType(chip::Optional<uint16_t>(25))
+        .SetDeviceType(chip::Optional<uint32_t>(70000))
         .SetCommissioningMode(CommissioningMode::kEnabledEnhanced)
         .SetDeviceName(chip::Optional<const char *>("testy-test"))
         .SetPairingHint(chip::Optional<uint16_t>(3))
@@ -139,15 +139,15 @@ test::ExpectedCall commissionableLargeEnhanced = test::ExpectedCall()
                                                      .AddTxt("D", "22")
                                                      .AddTxt("VP", "555+897")
                                                      .AddTxt("CM", "2")
-                                                     .AddTxt("DT", "25")
+                                                     .AddTxt("DT", "70000")
                                                      .AddTxt("DN", "testy-test")
                                                      .AddTxt("RI", "id_that_spins")
                                                      .AddTxt("PI", "Pair me")
                                                      .AddTxt("PH", "3")
                                                      .AddSubtype("_S2")
                                                      .AddSubtype("_L22")
                                                      .AddSubtype("_V555")
-                                                     .AddSubtype("_T25")
+                                                     .AddSubtype("_T70000")
                                                      .AddSubtype("_CM");
 void TestStub(nlTestSuite * inSuite, void * inContext)
 {

diff --git a/src/lib/dnssd/tests/TestTxtFields.cpp b/src/lib/dnssd/tests/TestTxtFields.cpp
@@ -174,8 +174,8 @@ void TestGetDeviceType(nlTestSuite * inSuite, void * inContext)
     strcpy(dt, "1234");
     NL_TEST_ASSERT(inSuite, GetDeviceType(GetSpan(dt)) == 1234);
 
-    // overflow a uint16
-    sprintf(dt, "%" PRIu32, static_cast<uint32_t>(std::numeric_limits<uint16_t>::max()) + 1);
+    // overflow a uint32
+    sprintf(dt, "%" PRIu64, static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()) + 1);
     NL_TEST_ASSERT(inSuite, GetDeviceType(GetSpan(dt)) == 0);
 }
 

diff --git a/src/platform/fake/ConfigurationManagerImpl.h b/src/platform/fake/ConfigurationManagerImpl.h
@@ -69,7 +69,7 @@ class ConfigurationManagerImpl : public ConfigurationManager
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }
     bool IsCommissionableDeviceTypeEnabled() override { return false; }
-    CHIP_ERROR GetDeviceTypeId(uint16_t & deviceType) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
+    CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     bool IsCommissionableDeviceNameEnabled() override { return false; }
     CHIP_ERROR GetCommissionableDeviceName(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetInitialPairingHint(uint16_t & pairingHint) override { return CHIP_ERROR_NOT_IMPLEMENTED; }