From 71051966dc4a47f4e8ac62204fa6c61e64731e51 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Thu, 15 Aug 2024 16:51:33 +0000 Subject: [PATCH] Fix adding fabricindex to LocationDirectory --- .../ecosystem-information-server.cpp | 30 ++++++++----------- .../ecosystem-information-server.h | 26 +++++++++------- 2 files changed, 27 insertions(+), 29 deletions(-) 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 9e5ced0e48aad2..ed87fe37e4a49c 100644 --- a/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp +++ b/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp @@ -215,40 +215,28 @@ EcosystemLocationStruct::Builder::SetLocationDescriptorLastEdit(uint64_t aLocati return *this; } -EcosystemLocationStruct::Builder & EcosystemLocationStruct::Builder::SetFabricIndex(FabricIndex aFabricIndex) -{ - VerifyOrDie(!mIsAlreadyBuilt); - mFabricIndex = aFabricIndex; - return *this; -} - - std::unique_ptr EcosystemLocationStruct::Builder::Build() { VerifyOrReturnValue(!mIsAlreadyBuilt, nullptr, ChipLogError(Zcl, "Build() already called")); VerifyOrReturnValue(!mLocationDescriptor.mLocationName.empty(), nullptr, ChipLogError(Zcl, "Must Provided Location Name")); VerifyOrReturnValue(mLocationDescriptor.mLocationName.size() <= kLocationDescriptorNameMaxSize, nullptr, ChipLogError(Zcl, "Must Location Name must be less than 64 bytes")); - VerifyOrReturnValue(mFabricIndex >= kMinValidFabricIndex, nullptr, ChipLogError(Zcl, "Fabric index is invalid")); - VerifyOrReturnValue(mFabricIndex <= kMaxValidFabricIndex, nullptr, ChipLogError(Zcl, "Fabric index is invalid")); // std::make_unique does not have access to private constructor we workaround with using new std::unique_ptr ret{ new EcosystemLocationStruct(std::move(mLocationDescriptor), - mLocationDescriptorLastEditEpochUs, - mFabricIndex) }; + mLocationDescriptorLastEditEpochUs) }; mIsAlreadyBuilt = true; return ret; } CHIP_ERROR EcosystemLocationStruct::Encode(const AttributeValueEncoder::ListEncodeHelper & aEncoder, - const std::string & aUniqueLocationId) + const EcosystemLocationIdentifier & aUniqueLocationId) { Structs::EcosystemLocationStruct::Type locationStruct; - VerifyOrDie(!aUniqueLocationId.empty()); - locationStruct.uniqueLocationID = CharSpan(aUniqueLocationId.c_str(), aUniqueLocationId.size()); + locationStruct.uniqueLocationID = CharSpan(aUniqueLocationId.mUniqueLocationId.c_str(), aUniqueLocationId.mUniqueLocationId.size()); locationStruct.locationDescriptor = GetEncodableLocationDescriptorStruct(mLocationDescriptor); locationStruct.locationDescriptorLastEdit = mLocationDescriptorLastEditEpochUs; - locationStruct.SetFabricIndex(mFabricIndex); + locationStruct.SetFabricIndex(aUniqueLocationId.mFabricIndex); return aEncoder.Encode(locationStruct); } @@ -283,16 +271,22 @@ CHIP_ERROR EcosystemInformationServer::AddDeviceInfo(EndpointId aEndpoint, std:: } CHIP_ERROR EcosystemInformationServer::AddLocationInfo(EndpointId aEndpoint, const std::string & aLocationId, + 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()), + // TODO rename foobar + EcosystemLocationIdentifier foobar = {.mUniqueLocationId=aLocationId, .mFabricIndex=aFabricIndex}; + VerifyOrReturnError((deviceInfo.mLocationDirectory.find(foobar) == deviceInfo.mLocationDirectory.end()), CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError((deviceInfo.mLocationDirectory.size() < kLocationDirectoryMaxSize), CHIP_ERROR_NO_MEMORY); - deviceInfo.mLocationDirectory[aLocationId] = std::move(aLocation); + deviceInfo.mLocationDirectory[foobar] = std::move(aLocation); 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 17c20de89132dc..b5f9253de385ae 100644 --- a/src/app/clusters/ecosystem-information-server/ecosystem-information-server.h +++ b/src/app/clusters/ecosystem-information-server/ecosystem-information-server.h @@ -101,6 +101,14 @@ struct LocationDescriptorStruct std::optional mAreaType; }; +struct EcosystemLocationIdentifier { + bool operator<(const EcosystemLocationIdentifier& other) const { + return mUniqueLocationId < other.mUniqueLocationId || (mUniqueLocationId == other.mUniqueLocationId && mFabricIndex < other.mFabricIndex); + } + std::string mUniqueLocationId; + FabricIndex mFabricIndex; +}; + // This intentionally mirrors Structs::EcosystemLocationStruct::Type but has ownership // of underlying types. class EcosystemLocationStruct @@ -115,7 +123,6 @@ class EcosystemLocationStruct Builder & SetFloorNumber(std::optional aFloorNumber); Builder & SetAreaTypeTag(std::optional aAreaTypeTag); Builder & SetLocationDescriptorLastEdit(uint64_t aLocationDescriptorLastEditEpochUs); - Builder & SetFabricIndex(FabricIndex aFabricIndex); // Upon success this object will have moved all ownership of underlying // types to EcosystemDeviceStruct and should not be used afterwards. @@ -124,25 +131,23 @@ class EcosystemLocationStruct private: LocationDescriptorStruct mLocationDescriptor; uint64_t mLocationDescriptorLastEditEpochUs = 0; - FabricIndex mFabricIndex = kUndefinedFabricIndex; bool mIsAlreadyBuilt = false; }; - CHIP_ERROR Encode(const AttributeValueEncoder::ListEncodeHelper & aEncoder, const std::string & aUniqueLocationId); + CHIP_ERROR Encode(const AttributeValueEncoder::ListEncodeHelper & aEncoder, const EcosystemLocationIdentifier & aUniqueLocationId); private: // Constructor is intentionally private. This is to ensure that it is only constructed with // values that conform to the spec. - explicit EcosystemLocationStruct(LocationDescriptorStruct && aLocationDescriptor, uint64_t aLocationDescriptorLastEditEpochUs, FabricIndex aFabricIndex) : - mLocationDescriptor(aLocationDescriptor), mLocationDescriptorLastEditEpochUs(aLocationDescriptorLastEditEpochUs), mFabricIndex(aFabricIndex) + explicit EcosystemLocationStruct(LocationDescriptorStruct && aLocationDescriptor, uint64_t aLocationDescriptorLastEditEpochUs) : + 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 manditory 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; - FabricIndex mFabricIndex; }; class EcosystemInformationServer @@ -186,7 +191,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); /** @@ -207,8 +212,7 @@ class EcosystemInformationServer { 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);