diff --git a/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp b/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp index c51a5165f3ca9d..2ce82394a57464 100644 --- a/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp +++ b/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp @@ -128,6 +128,13 @@ EcosystemDeviceStruct::Builder & EcosystemDeviceStruct::Builder::AddUniqueLocati return *this; } +EcosystemDeviceStruct::Builder & EcosystemDeviceStruct::Builder::SetFabricIndex(FabricIndex aFabricIndex) +{ + VerifyOrDie(!mIsAlreadyBuilt); + mFabricIndex = aFabricIndex; + return *this; +} + std::unique_ptr EcosystemDeviceStruct::Builder::Build() { VerifyOrReturnValue(!mIsAlreadyBuilt, nullptr, ChipLogError(Zcl, "Build() already called")); @@ -136,6 +143,8 @@ std::unique_ptr EcosystemDeviceStruct::Builder::Build() VerifyOrReturnValue(!mDeviceTypes.empty(), nullptr, ChipLogError(Zcl, "No device types added")); VerifyOrReturnValue(mUniqueLocationIds.size() <= kUniqueLocationIdsListMaxSize, nullptr, ChipLogError(Zcl, "Too many location ids")); + VerifyOrReturnValue(mFabricIndex >= kMinValidFabricIndex, nullptr, ChipLogError(Zcl, "Fabric index is invalid")); + VerifyOrReturnValue(mFabricIndex <= kMaxValidFabricIndex, nullptr, ChipLogError(Zcl, "Fabric index is invalid")); for (auto & locationId : mUniqueLocationIds) { @@ -145,12 +154,12 @@ std::unique_ptr EcosystemDeviceStruct::Builder::Build() // std::make_unique does not have access to private constructor we workaround with using new std::unique_ptr ret{ new EcosystemDeviceStruct( std::move(mDeviceName), mDeviceNameLastEditEpochUs, mBridgedEndpoint, mOriginalEndpoint, std::move(mDeviceTypes), - std::move(mUniqueLocationIds), mUniqueLocationIdsLastEditEpochUs) }; + std::move(mUniqueLocationIds), mUniqueLocationIdsLastEditEpochUs, mFabricIndex) }; mIsAlreadyBuilt = true; return ret; } -CHIP_ERROR EcosystemDeviceStruct::Encode(const AttributeValueEncoder::ListEncodeHelper & aEncoder, const FabricIndex & aFabricIndex) +CHIP_ERROR EcosystemDeviceStruct::Encode(const AttributeValueEncoder::ListEncodeHelper & aEncoder) { Structs::EcosystemDeviceStruct::Type deviceStruct; if (!mDeviceName.empty()) @@ -172,9 +181,7 @@ CHIP_ERROR EcosystemDeviceStruct::Encode(const AttributeValueEncoder::ListEncode deviceStruct.uniqueLocationIDs = DataModel::List(locationIds.data(), locationIds.size()); deviceStruct.uniqueLocationIDsLastEdit = mUniqueLocationIdsLastEditEpochUs; - - // TODO(#33223) this is a hack, use mFabricIndex when it exists. - deviceStruct.SetFabricIndex(aFabricIndex); + deviceStruct.SetFabricIndex(mFabricIndex); return aEncoder.Encode(deviceStruct); } @@ -226,12 +233,9 @@ CHIP_ERROR EcosystemLocationStruct::Encode(const AttributeValueEncoder::ListEnco const std::string & aUniqueLocationId, const FabricIndex & aFabricIndex) { Structs::EcosystemLocationStruct::Type locationStruct; - VerifyOrDie(!aUniqueLocationId.empty()); locationStruct.uniqueLocationID = CharSpan(aUniqueLocationId.c_str(), aUniqueLocationId.size()); locationStruct.locationDescriptor = GetEncodableLocationDescriptorStruct(mLocationDescriptor); locationStruct.locationDescriptorLastEdit = mLocationDescriptorLastEditEpochUs; - - // TODO(#33223) this is a hack, use mFabricIndex when it exists. locationStruct.SetFabricIndex(aFabricIndex); return aEncoder.Encode(locationStruct); } @@ -266,16 +270,20 @@ CHIP_ERROR EcosystemInformationServer::AddDeviceInfo(EndpointId aEndpoint, std:: } CHIP_ERROR EcosystemInformationServer::AddLocationInfo(EndpointId aEndpoint, const std::string & aLocationId, - std::unique_ptr aLocation) + FabricIndex aFabricIndex, std::unique_ptr aLocation) { VerifyOrReturnError(aLocation, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError((aEndpoint != kRootEndpointId && aEndpoint != kInvalidEndpointId), CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(!aLocationId.empty(), CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(aFabricIndex >= kMinValidFabricIndex, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(aFabricIndex <= kMaxValidFabricIndex, CHIP_ERROR_INVALID_ARGUMENT); - auto & deviceInfo = mDevicesMap[aEndpoint]; - VerifyOrReturnError((deviceInfo.mLocationDirectory.find(aLocationId) == deviceInfo.mLocationDirectory.end()), + auto & deviceInfo = mDevicesMap[aEndpoint]; + EcosystemLocationKey key = { .mUniqueLocationId = aLocationId, .mFabricIndex = aFabricIndex }; + VerifyOrReturnError((deviceInfo.mLocationDirectory.find(key) == deviceInfo.mLocationDirectory.end()), CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError((deviceInfo.mLocationDirectory.size() < kLocationDirectoryMaxSize), CHIP_ERROR_NO_MEMORY); - deviceInfo.mLocationDirectory[aLocationId] = std::move(aLocation); + deviceInfo.mLocationDirectory[key] = std::move(aLocation); return CHIP_NO_ERROR; } @@ -352,11 +360,10 @@ CHIP_ERROR EcosystemInformationServer::EncodeDeviceDirectoryAttribute(EndpointId return aEncoder.EncodeEmptyList(); } - FabricIndex fabricIndex = aEncoder.AccessingFabricIndex(); return aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR { for (auto & device : deviceInfo.mDeviceDirectory) { - ReturnErrorOnFailure(device->Encode(encoder, fabricIndex)); + ReturnErrorOnFailure(device->Encode(encoder)); } return CHIP_NO_ERROR; }); @@ -379,11 +386,10 @@ CHIP_ERROR EcosystemInformationServer::EncodeLocationStructAttribute(EndpointId return aEncoder.EncodeEmptyList(); } - FabricIndex fabricIndex = aEncoder.AccessingFabricIndex(); return aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR { - for (auto & [id, device] : deviceInfo.mLocationDirectory) + for (auto & [key, device] : deviceInfo.mLocationDirectory) { - ReturnErrorOnFailure(device->Encode(encoder, id, fabricIndex)); + ReturnErrorOnFailure(device->Encode(encoder, key.mUniqueLocationId, key.mFabricIndex)); } return CHIP_NO_ERROR; }); diff --git a/src/app/clusters/ecosystem-information-server/ecosystem-information-server.h b/src/app/clusters/ecosystem-information-server/ecosystem-information-server.h index dce12e745bf14e..d4f4d7dc3a6fc1 100644 --- a/src/app/clusters/ecosystem-information-server/ecosystem-information-server.h +++ b/src/app/clusters/ecosystem-information-server/ecosystem-information-server.h @@ -49,6 +49,7 @@ class EcosystemDeviceStruct Builder & SetOriginalEndpoint(EndpointId aOriginalEndpoint); Builder & AddDeviceType(Structs::DeviceTypeStruct::Type aDeviceType); Builder & AddUniqueLocationId(std::string aUniqueLocationId, uint64_t aUniqueLocationIdsLastEditEpochUs); + Builder & SetFabricIndex(FabricIndex aFabricIndex); // Upon success this object will have moved all ownership of underlying // types to EcosystemDeviceStruct and should not be used afterwards. @@ -62,21 +63,25 @@ class EcosystemDeviceStruct std::vector mDeviceTypes; std::vector mUniqueLocationIds; uint64_t mUniqueLocationIdsLastEditEpochUs = 0; + FabricIndex mFabricIndex = kUndefinedFabricIndex; bool mIsAlreadyBuilt = false; }; - CHIP_ERROR Encode(const AttributeValueEncoder::ListEncodeHelper & aEncoder, const FabricIndex & aFabricIndex); + CHIP_ERROR Encode(const AttributeValueEncoder::ListEncodeHelper & aEncoder); private: // Constructor is intentionally private. This is to ensure that it is only constructed with // values that conform to the spec. explicit EcosystemDeviceStruct(std::string && aDeviceName, uint64_t aDeviceNameLastEditEpochUs, EndpointId aBridgedEndpoint, EndpointId aOriginalEndpoint, std::vector && aDeviceTypes, - std::vector && aUniqueLocationIds, uint64_t aUniqueLocationIdsLastEditEpochUs) : + std::vector && aUniqueLocationIds, uint64_t aUniqueLocationIdsLastEditEpochUs, + FabricIndex aFabricIndex) : mDeviceName(std::move(aDeviceName)), mDeviceNameLastEditEpochUs(aDeviceNameLastEditEpochUs), mBridgedEndpoint(aBridgedEndpoint), mOriginalEndpoint(aOriginalEndpoint), mDeviceTypes(std::move(aDeviceTypes)), - mUniqueLocationIds(std::move(aUniqueLocationIds)), mUniqueLocationIdsLastEditEpochUs(aUniqueLocationIdsLastEditEpochUs) + mUniqueLocationIds(std::move(aUniqueLocationIds)), mUniqueLocationIdsLastEditEpochUs(aUniqueLocationIdsLastEditEpochUs), + mFabricIndex(aFabricIndex) + {} const std::string mDeviceName; @@ -86,10 +91,7 @@ class EcosystemDeviceStruct std::vector mDeviceTypes; std::vector mUniqueLocationIds; uint64_t mUniqueLocationIdsLastEditEpochUs; - // TODO(#33223) This structure needs to contain fabric index to be spec compliant. - // To keep initial PR smaller, we are going to assume that all entries - // here are for any fabric. This will allow follow up PR introducing - // fabric scoped to be more throughly reviewed with focus on fabric scoping. + FabricIndex mFabricIndex; }; struct LocationDescriptorStruct @@ -134,15 +136,11 @@ class EcosystemLocationStruct mLocationDescriptor(aLocationDescriptor), mLocationDescriptorLastEditEpochUs(aLocationDescriptorLastEditEpochUs) {} // EcosystemLocationStruct is used as a value in a key-value map. - // Because UniqueLocationId is manditory when an entry exist, and - // it is unique, we use it as a key to the key-value pair and is why it is + // Because UniqueLocationId and FabricIndex are mandatory when an entry exist, + // and needs to be unique, we use it as a key to the key-value pair and is why it is // not explicitly in this struct. LocationDescriptorStruct mLocationDescriptor; uint64_t mLocationDescriptorLastEditEpochUs; - // TODO(#33223) This structure needs to contain fabric index to be spec compliant. - // To keep initial PR smaller, we are going to assume that all entries - // here are for any fabric. This will allow follow up PR introducing - // fabric scoped to be more throughly reviewed with focus on fabric scoping. }; class EcosystemInformationServer @@ -186,7 +184,7 @@ class EcosystemInformationServer * @return #CHIP_NO_ERROR on success. * @return Other CHIP_ERROR associated with issue. */ - CHIP_ERROR AddLocationInfo(EndpointId aEndpoint, const std::string & aLocationId, + CHIP_ERROR AddLocationInfo(EndpointId aEndpoint, const std::string & aLocationId, FabricIndex aFabricIndex, std::unique_ptr aLocation); /** @@ -203,12 +201,22 @@ class EcosystemInformationServer CHIP_ERROR ReadAttribute(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder); private: + struct EcosystemLocationKey + { + bool operator<(const EcosystemLocationKey & other) const + { + return mUniqueLocationId < other.mUniqueLocationId || + (mUniqueLocationId == other.mUniqueLocationId && mFabricIndex < other.mFabricIndex); + } + std::string mUniqueLocationId; + FabricIndex mFabricIndex; + }; + struct DeviceInfo { Optional mRemovedOn = NullOptional; std::vector> mDeviceDirectory; - // Map key is using the UniqueLocationId - std::map> mLocationDirectory; + std::map> mLocationDirectory; }; CHIP_ERROR EncodeRemovedOnAttribute(EndpointId aEndpoint, AttributeValueEncoder & aEncoder);