Skip to content

Commit

Permalink
Move GetVendorId and GetProductId to DeviceInstanceInfoProvider (#19514)
Browse files Browse the repository at this point in the history
* Move GetVendorId and GetProductId to DeviceInstanceInfoProvider

GetVendorId and GetProductId are part of ConfigurationManager,
while it can be useful to put those data into the factory data.

* Moved GetVendorId and GetProductId to DeviceInstanceInfoProvider
* Added DeviceInstanceInfoProviderImpl for the platforms that has
different methods implementation than
LegacyDeviceInstanceInfoProvider.

* Fixed missing include for CHIP_ENABLE_ROTATING_DEVICE_ID definition

* Addressed review comments

* Moved LegacyDeviceInstanceInfoProvider to be
GenericDeviceInstanceInfoProvider that is always compiled and provides
default implementations of methods.
* For Darwin, Linux, Tizen, Webos and Android removed duplicated
DeviceInstanceInfoProvider code and added inheriting from Generic
class to allow overriding only desired methods.

* Added unit tests vendor/product id/name getters

* Removed redundant ifdefs for Darwin
  • Loading branch information
kkasperczyk-no authored Jun 15, 2022
1 parent 663ecf4 commit 09f3e4a
Show file tree
Hide file tree
Showing 48 changed files with 848 additions and 345 deletions.
4 changes: 2 additions & 2 deletions examples/common/pigweed/rpc_services/Device.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ class Device : public pw_rpc::nanopb::Device::Service<Device>
{

uint16_t vendor_id;
if (DeviceLayer::ConfigurationMgr().GetVendorId(vendor_id) == CHIP_NO_ERROR)
if (DeviceLayer::GetDeviceInstanceInfoProvider()->GetVendorId(vendor_id) == CHIP_NO_ERROR)
{
response.vendor_id = static_cast<uint32_t>(vendor_id);
}
Expand All @@ -292,7 +292,7 @@ class Device : public pw_rpc::nanopb::Device::Service<Device>
}

uint16_t product_id;
if (DeviceLayer::ConfigurationMgr().GetProductId(product_id) == CHIP_NO_ERROR)
if (DeviceLayer::GetDeviceInstanceInfoProvider()->GetProductId(product_id) == CHIP_NO_ERROR)
{
response.product_id = static_cast<uint32_t>(product_id);
}
Expand Down
8 changes: 4 additions & 4 deletions src/app/clusters/basic/basic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,14 @@ CHIP_ERROR BasicAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attrib
case VendorName::Id: {
constexpr size_t kMaxLen = DeviceLayer::ConfigurationManager::kMaxVendorNameLength;
char vendorName[kMaxLen + 1] = { 0 };
status = ConfigurationMgr().GetVendorName(vendorName, sizeof(vendorName));
status = GetDeviceInstanceInfoProvider()->GetVendorName(vendorName, sizeof(vendorName));
status = EncodeStringOnSuccess(status, aEncoder, vendorName, kMaxLen);
break;
}

case VendorID::Id: {
uint16_t vendorId = 0;
status = ConfigurationMgr().GetVendorId(vendorId);
status = GetDeviceInstanceInfoProvider()->GetVendorId(vendorId);
if (status == CHIP_NO_ERROR)
{
status = aEncoder.Encode(vendorId);
Expand All @@ -109,14 +109,14 @@ CHIP_ERROR BasicAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attrib
case ProductName::Id: {
constexpr size_t kMaxLen = DeviceLayer::ConfigurationManager::kMaxProductNameLength;
char productName[kMaxLen + 1] = { 0 };
status = ConfigurationMgr().GetProductName(productName, sizeof(productName));
status = GetDeviceInstanceInfoProvider()->GetProductName(productName, sizeof(productName));
status = EncodeStringOnSuccess(status, aEncoder, productName, kMaxLen);
break;
}

case ProductID::Id: {
uint16_t productId = 0;
status = ConfigurationMgr().GetProductId(productId);
status = GetDeviceInstanceInfoProvider()->GetProductId(productId);
if (status == CHIP_NO_ERROR)
{
status = aEncoder.Encode(productId);
Expand Down
6 changes: 3 additions & 3 deletions src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ void DefaultOTARequestor::NotifyUpdateApplied()
{
// Log the VersionApplied event
uint16_t productId;
if (DeviceLayer::ConfigurationMgr().GetProductId(productId) != CHIP_NO_ERROR)
if (DeviceLayer::GetDeviceInstanceInfoProvider()->GetProductId(productId) != CHIP_NO_ERROR)
{
ChipLogError(SoftwareUpdate, "Cannot get Product ID");
RecordErrorUpdateState(CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -722,10 +722,10 @@ CHIP_ERROR DefaultOTARequestor::SendQueryImageRequest(OperationalDeviceProxy & d
QueryImage::Type args;

uint16_t vendorId;
ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetVendorId(vendorId));
ReturnErrorOnFailure(DeviceLayer::GetDeviceInstanceInfoProvider()->GetVendorId(vendorId));
args.vendorId = static_cast<VendorId>(vendorId);

ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetProductId(args.productId));
ReturnErrorOnFailure(DeviceLayer::GetDeviceInstanceInfoProvider()->GetProductId(args.productId));

ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetSoftwareVersion(args.softwareVersion));

Expand Down
5 changes: 3 additions & 2 deletions src/app/clusters/ota-requestor/ExtendedOTARequestorDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "ExtendedOTARequestorDriver.h"
#include "OTARequestorInterface.h"
#include <app/server/Server.h>
#include <platform/DeviceInstanceInfoProvider.h>

namespace chip {
namespace DeviceLayer {
Expand Down Expand Up @@ -84,8 +85,8 @@ CHIP_ERROR ExtendedOTARequestorDriver::GetUserConsentSubject(chip::ota::UserCons
}
subject.requestorNodeId = fabricInfo->GetPeerId().GetNodeId();

ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetVendorId(subject.requestorVendorId));
ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetProductId(subject.requestorProductId));
ReturnErrorOnFailure(DeviceLayer::GetDeviceInstanceInfoProvider()->GetVendorId(subject.requestorVendorId));
ReturnErrorOnFailure(DeviceLayer::GetDeviceInstanceInfoProvider()->GetProductId(subject.requestorProductId));
ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetSoftwareVersion(subject.requestorCurrentVersion));
subject.requestorTargetVersion = update.softwareVersion;
subject.metadata = update.metadataForRequestor;
Expand Down
6 changes: 3 additions & 3 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
#include <platform/CHIPDeviceLayer.h>
#include <platform/CommissionableDataProvider.h>
#include <platform/ConfigurationManager.h>
#include <platform/DeviceInstanceInfoProvider.h>
#include <protocols/secure_channel/PASESession.h>
#if CHIP_ENABLE_ROTATING_DEVICE_ID && defined(CHIP_DEVICE_CONFIG_ROTATING_DEVICE_ID_UNIQUE_ID)
#include <platform/DeviceInstanceInfoProvider.h>
#include <setup_payload/AdditionalDataPayloadGenerator.h>
#endif
#include <credentials/FabricTable.h>
Expand Down Expand Up @@ -298,7 +298,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi

uint16_t value;
uint32_t val32;
if (DeviceLayer::ConfigurationMgr().GetVendorId(value) != CHIP_NO_ERROR)
if (DeviceLayer::GetDeviceInstanceInfoProvider()->GetVendorId(value) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "Vendor ID not known");
}
Expand All @@ -307,7 +307,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
advertiseParameters.SetVendorId(chip::Optional<uint16_t>::Value(value));
}

if (DeviceLayer::ConfigurationMgr().GetProductId(value) != CHIP_NO_ERROR)
if (DeviceLayer::GetDeviceInstanceInfoProvider()->GetProductId(value) != CHIP_NO_ERROR)
{
ChipLogDetail(Discovery, "Product ID not known");
}
Expand Down
9 changes: 5 additions & 4 deletions src/app/server/OnboardingCodesUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <lib/support/logging/CHIPLogging.h>
#include <platform/CHIPDeviceLayer.h>
#include <platform/CommissionableDataProvider.h>
#include <platform/DeviceInstanceInfoProvider.h>
#include <setup_payload/ManualSetupPayloadGenerator.h>
#include <setup_payload/QRCodeSetupPayloadGenerator.h>

Expand Down Expand Up @@ -121,17 +122,17 @@ CHIP_ERROR GetPayloadContents(chip::PayloadContents & aPayload, chip::Rendezvous
return err;
}

err = ConfigurationMgr().GetVendorId(aPayload.vendorID);
err = chip::DeviceLayer::GetDeviceInstanceInfoProvider()->GetVendorId(aPayload.vendorID);
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "ConfigurationMgr().GetVendorId() failed: %s", chip::ErrorStr(err));
ChipLogError(AppServer, "GetDeviceInstanceInfoProvider()->GetVendorId() failed: %s", chip::ErrorStr(err));
return err;
}

err = ConfigurationMgr().GetProductId(aPayload.productID);
err = chip::DeviceLayer::GetDeviceInstanceInfoProvider()->GetProductId(aPayload.productID);
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "ConfigurationMgr().GetProductId() failed: %s", chip::ErrorStr(err));
ChipLogError(AppServer, "GetDeviceInstanceInfoProvider()->GetProductId() failed: %s", chip::ErrorStr(err));
return err;
}

Expand Down
4 changes: 0 additions & 4 deletions src/include/platform/ConfigurationManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ class ConfigurationManager
kMaxLanguageTagLength = 5 // ISO 639-1 standard language codes
};

virtual CHIP_ERROR GetVendorName(char * buf, size_t bufSize) = 0;
virtual CHIP_ERROR GetVendorId(uint16_t & vendorId) = 0;
virtual CHIP_ERROR GetProductName(char * buf, size_t bufSize) = 0;
virtual CHIP_ERROR GetProductId(uint16_t & productId) = 0;
virtual CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) = 0;
virtual CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) = 0;
virtual CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf) = 0;
Expand Down
44 changes: 44 additions & 0 deletions src/include/platform/DeviceInstanceInfoProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,50 @@ class DeviceInstanceInfoProvider
DeviceInstanceInfoProvider() = default;
virtual ~DeviceInstanceInfoProvider() = default;

/**
* @brief Obtain the Vendor Name from the device's factory data.
*
* @param[in, out] buf Buffer to copy string.
* On CHIP_NO_ERROR return from this function this buffer will be null-terminated.
* On error CHIP_ERROR_BUFFER_TOO_SMALL there is no guarantee that buffer will be null-terminated.
* @param[in] bufSize Size of data, including the null terminator, that can be written to buf.
* This size should be +1 higher than maximum possible string.
* @returns CHIP_NO_ERROR on success, or another CHIP_ERROR from the underlying implementation
* if access fails.
*/
virtual CHIP_ERROR GetVendorName(char * buf, size_t bufSize) = 0;

/**
* @brief Obtain the Vendor Id from the device's factory data.
*
* @param[out] vendorId Reference to location where the vendor id integer will be copied
* @returns CHIP_NO_ERROR on success, or another CHIP_ERROR from the underlying implementation
* if access fails.
*/
virtual CHIP_ERROR GetVendorId(uint16_t & vendorId) = 0;

/**
* @brief Obtain the Product Name from the device's factory data.
*
* @param[in, out] buf Buffer to copy string.
* On CHIP_NO_ERROR return from this function this buffer will be null-terminated.
* On error CHIP_ERROR_BUFFER_TOO_SMALL there is no guarantee that buffer will be null-terminated.
* @param[in] bufSize Size of data, including the null terminator, that can be written to buf.
* This size should be +1 higher than maximum possible string.
* @returns CHIP_NO_ERROR on success, or another CHIP_ERROR from the underlying implementation
* if access fails.
*/
virtual CHIP_ERROR GetProductName(char * buf, size_t bufSize) = 0;

/**
* @brief Obtain the Product Id from the device's factory data.
*
* @param[out] productId Reference to location where the product id integer will be copied
* @returns CHIP_NO_ERROR on success, or another CHIP_ERROR from the underlying implementation
* if access fails.
*/
virtual CHIP_ERROR GetProductId(uint16_t & productId) = 0;

/**
* @brief Obtain the Serial Number from the device's factory data.
*
Expand Down
12 changes: 2 additions & 10 deletions src/include/platform/internal/GenericConfigurationManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,8 @@ class ProvisioningDataSet;

namespace Internal {

#if CHIP_USE_TRANSITIONAL_DEVICE_INSTANCE_INFO_PROVIDER
template <class ConfigClass>
class LegacyDeviceInstanceInfoProvider;
#endif // CHIP_USE_TRANSITIONAL_DEVICE_INSTANCE_INFO_PROVIDER
class GenericDeviceInstanceInfoProvider;

#if CHIP_USE_TRANSITIONAL_COMMISSIONABLE_DATA_PROVIDER
template <class ConfigClass>
Expand All @@ -64,10 +62,6 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
// ===== Methods that implement the ConfigurationManager abstract interface.

CHIP_ERROR Init() override;
CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override;
CHIP_ERROR GetVendorId(uint16_t & vendorId) override;
CHIP_ERROR GetProductName(char * buf, size_t bufSize) override;
CHIP_ERROR GetProductId(uint16_t & productId) override;
CHIP_ERROR StoreHardwareVersion(uint16_t hardwareVer) override;
CHIP_ERROR GetSoftwareVersionString(char * buf, size_t bufSize) override;
CHIP_ERROR GetSoftwareVersion(uint32_t & softwareVer) override;
Expand Down Expand Up @@ -129,9 +123,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
uint8_t mRotatingDeviceIdUniqueId[kRotatingDeviceIDUniqueIDLength] = CHIP_DEVICE_CONFIG_ROTATING_DEVICE_ID_UNIQUE_ID;
#endif

#if CHIP_USE_TRANSITIONAL_DEVICE_INSTANCE_INFO_PROVIDER
friend LegacyDeviceInstanceInfoProvider<ConfigClass>;
#endif // CHIP_USE_TRANSITIONAL_DEVICE_INSTANCE_INFO_PROVIDER
friend GenericDeviceInstanceInfoProvider<ConfigClass>;

#if CHIP_USE_TRANSITIONAL_COMMISSIONABLE_DATA_PROVIDER
friend LegacyTemporaryCommissionableDataProvider<ConfigClass>;
Expand Down
Loading

0 comments on commit 09f3e4a

Please sign in to comment.