Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate out attribute access interface registry/cache, remove free functions from InteractionModelEngine.h #32954

Merged
merged 22 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
8768f64
Move GetAttributeAccessOverride to the right place
andy31415 Apr 11, 2024
311348d
A first pass at decoupling the AttributeAccessOverride
andy31415 Apr 11, 2024
a31a1b9
Start adding a bit of registry, update attribute-storage.h
andy31415 Apr 11, 2024
c1563e5
Add the registry include everywhere it was used to register things
andy31415 Apr 11, 2024
56079e8
Restyle
andy31415 Apr 11, 2024
f289314
More definition and usage cleanup
andy31415 Apr 11, 2024
d07686f
Add include in ember compatibility layer
andy31415 Apr 11, 2024
d193d9f
Cleanup a TODO
andy31415 Apr 11, 2024
139c930
Fix linter too
andy31415 Apr 11, 2024
aa7668f
Fix dynamic dispatcher compile
andy31415 Apr 11, 2024
e1657c2
Darwin connectivity manager does not use TLV
andy31415 Apr 11, 2024
e88d964
Fix include dependency for ESP32
andy31415 Apr 11, 2024
ad15fca
Remove usage of chip::tlv from connectivity manager: no TLV is used a…
andy31415 Apr 12, 2024
9b73ef0
Fix tizen includes: if you need Wifi enums, cluster-enums.h need incl…
andy31415 Apr 12, 2024
c2c9b9e
Fix esp32 build
andy31415 Apr 12, 2024
00d7326
Fix mbed compile: its impl header had no includes for some reason
andy31415 Apr 12, 2024
df76dee
Add needed include to InteractiveServer.cpp to fix darwin build
andy31415 Apr 12, 2024
fb05c95
Also include vectore because we use it
andy31415 Apr 12, 2024
698dd39
Also fix boufallolab includes
andy31415 Apr 12, 2024
2ebbfdb
Update src/darwin/Framework/CHIP/ServerEndpoint/MTRServerCluster.mm
andy31415 Apr 12, 2024
84dec17
Apply rename suggestion for unregisterAll...ForEndpoint
andy31415 Apr 12, 2024
12ef336
Apply code review comment: log category switch
andy31415 Apr 12, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ jobs:
--known-failure app/app-platform/ContentAppPlatform.h \
--known-failure controller/ExamplePersistentStorage.cpp \
--known-failure controller/ExamplePersistentStorage.h \
--known-failure app/AttributeAccessInterface.h \
--known-failure app/AttributeAccessToken.h \
--known-failure app/CommandHandler.h \
--known-failure app/CommandHandlerInterface.h \
Expand All @@ -107,11 +106,11 @@ jobs:
--known-failure app/reporting/tests/MockReportScheduler.cpp \
--known-failure app/reporting/tests/MockReportScheduler.h \
--known-failure app/util/attribute-storage.cpp \
--known-failure app/util/attribute-storage.h \
--known-failure app/util/attribute-storage-detail.h \
--known-failure app/util/attribute-storage.h \
--known-failure app/util/attribute-table.cpp \
--known-failure app/util/attribute-table.h \
--known-failure app/util/attribute-table-detail.h \
--known-failure app/util/attribute-table.h \
--known-failure app/util/binding-table.cpp \
--known-failure app/util/binding-table.h \
--known-failure app/util/config.h \
Expand All @@ -123,6 +122,7 @@ jobs:
--known-failure app/util/generic-callbacks.h \
--known-failure app/util/generic-callback-stubs.cpp \
--known-failure app/util/im-client-callbacks.h \
--known-failure app/util/IMClusterCommandHandler.h \
--known-failure app/util/util.cpp \
--known-failure app/util/util.h \
--known-failure app/WriteHandler.h \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/clusters/fan-control-server/fan-control-server.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
Expand Down
1 change: 1 addition & 0 deletions examples/bridge-app/asr/src/bridged-actions-stub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down
1 change: 1 addition & 0 deletions examples/bridge-app/esp32/main/DeviceCallbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down
1 change: 1 addition & 0 deletions examples/bridge-app/linux/bridged-actions-stub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down
1 change: 1 addition & 0 deletions examples/bridge-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/ConcreteAttributePath.h>
#include <app/EventLogging.h>
#include <app/reporting/reporting.h>
Expand Down
1 change: 1 addition & 0 deletions examples/bridge-app/telink/src/DeviceCallbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down
1 change: 1 addition & 0 deletions examples/chef/common/chef-fan-control-manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/clusters/fan-control-server/fan-control-server.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
Expand Down
2 changes: 2 additions & 0 deletions examples/placeholder/linux/InteractiveServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@
#include "InteractiveServer.h"

#include <json/json.h>
#include <lib/core/CHIPSafeCasts.h>
#include <lib/support/Base64.h>
#include <platform/CHIPDeviceLayer.h>
#include <platform/logging/LogV.h>

#include <string>
#include <vector>

using namespace chip::DeviceLayer;

Expand Down
1 change: 1 addition & 0 deletions examples/placeholder/linux/src/bridged-actions-stub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/util/attribute-storage.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down
126 changes: 126 additions & 0 deletions src/app/AttributeAccessInterfaceRegistry.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <app/AttributeAccessInterfaceRegistry.h>

#include <app/AttributeAccessInterfaceCache.h>

using namespace chip::app;

namespace {

AttributeAccessInterface * gAttributeAccessOverrides = nullptr;
AttributeAccessInterfaceCache gAttributeAccessInterfaceCache;

// shouldUnregister returns true if the given AttributeAccessInterface should be
// unregistered.
template <typename F>
void UnregisterMatchingAttributeAccessInterfaces(F shouldUnregister)
{
AttributeAccessInterface * prev = nullptr;
AttributeAccessInterface * cur = gAttributeAccessOverrides;
while (cur)
{
AttributeAccessInterface * next = cur->GetNext();
if (shouldUnregister(cur))
{
// Remove it from the list
if (prev)
{
prev->SetNext(next);
}
else
{
gAttributeAccessOverrides = next;
}

cur->SetNext(nullptr);

// Do not change prev in this case.
}
else
{
prev = cur;
}
cur = next;
}
}

} // namespace

void unregisterAttributeAccessOverride(AttributeAccessInterface * attrOverride)
{
gAttributeAccessInterfaceCache.Invalidate();
UnregisterMatchingAttributeAccessInterfaces([attrOverride](AttributeAccessInterface * entry) { return entry == attrOverride; });
}

void unregisterAttributeAccessOverrideForEndpoint(EmberAfDefinedEndpoint * definedEndpoint)
{
UnregisterMatchingAttributeAccessInterfaces(
[endpoint = definedEndpoint->endpoint](AttributeAccessInterface * entry) { return entry->MatchesEndpoint(endpoint); });
}

bool registerAttributeAccessOverride(AttributeAccessInterface * attrOverride)
{
gAttributeAccessInterfaceCache.Invalidate();
for (auto * cur = gAttributeAccessOverrides; cur; cur = cur->GetNext())
{
if (cur->Matches(*attrOverride))
{
ChipLogError(Zcl, "Duplicate attribute override registration failed");
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}
attrOverride->SetNext(gAttributeAccessOverrides);
gAttributeAccessOverrides = attrOverride;
return true;
}

namespace chip {
namespace app {

app::AttributeAccessInterface * GetAttributeAccessOverride(EndpointId endpointId, ClusterId clusterId)
{
using CacheResult = AttributeAccessInterfaceCache::CacheResult;

AttributeAccessInterface * cached = nullptr;
CacheResult result = gAttributeAccessInterfaceCache.Get(endpointId, clusterId, &cached);
switch (result)
{
case CacheResult::kDefinitelyUnused:
return nullptr;
case CacheResult::kDefinitelyUsed:
return cached;
case CacheResult::kCacheMiss:
default:
// Did not cache yet, search set of AAI registered, and cache if found.
for (app::AttributeAccessInterface * cur = gAttributeAccessOverrides; cur; cur = cur->GetNext())
{
if (cur->Matches(endpointId, clusterId))
{
gAttributeAccessInterfaceCache.MarkUsed(endpointId, clusterId, cur);
return cur;
}
}

// Did not find AAI registered: mark as definitely not using.
gAttributeAccessInterfaceCache.MarkUnused(endpointId, clusterId);
}

return nullptr;
}

} // namespace app
} // namespace chip
54 changes: 54 additions & 0 deletions src/app/AttributeAccessInterfaceRegistry.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once

#include <app/AttributeAccessInterface.h>
#include <app/util/af-types.h>

/**
* Register an attribute access override. It will remain registered until the
* endpoint it's registered for is disabled (or until shutdown if it's
* registered for all endpoints) or until it is explicitly unregistered.
* Registration will fail if there is an already-registered override for the
* same set of attributes.
*
* @return false if there is an existing override that the new one would
* conflict with. In this case the override is not registered.
* @return true if registration was successful.
*/
bool registerAttributeAccessOverride(chip::app::AttributeAccessInterface * attrOverride);

/**
* Unregister an attribute access override (for example if the object
* implementing AttributeAccessInterface is being destroyed).
*/
void unregisterAttributeAccessOverride(chip::app::AttributeAccessInterface * attrOverride);

/**
* Unregister all attribute access interfaces that match this given endpoint.
*/
void unregisterAttributeAccessOverrideForEndpoint(EmberAfDefinedEndpoint * definedEndpoint);
andy31415 marked this conversation as resolved.
Show resolved Hide resolved

namespace chip {
namespace app {

/**
* Get the registered attribute access override. nullptr when attribute access override is not found.
*/
AttributeAccessInterface * GetAttributeAccessOverride(EndpointId aEndpointId, ClusterId aClusterId);

} // namespace app
} // namespace chip
28 changes: 21 additions & 7 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -275,14 +275,32 @@ source_set("events") {
]
}

static_library("attribute-access") {
sources = [
"AttributeAccessInterface.cpp",
"AttributeAccessInterface.h",
"AttributeAccessInterfaceCache.h",
"AttributeAccessInterfaceRegistry.cpp",
"AttributeAccessInterfaceRegistry.h",
]

deps = [
":paths",
"${chip_root}/src/access:types",
"${chip_root}/src/app/MessageDef",
"${chip_root}/src/app/data-model",
"${chip_root}/src/app/util:af-types",
"${chip_root}/src/lib/core",
"${chip_root}/src/lib/support",
]
}

# Note to developpers, instead of continuously adding files in the app librabry, it is recommand to create smaller source_sets that app can depend on.
# This way, we can have a better understanding of dependencies and other componenets can depend on the different source_sets without needing to depend on the entire app library.
static_library("app") {
output_name = "libCHIPDataModel"

sources = [
"AttributeAccessInterface.cpp",
"AttributeAccessInterfaceCache.h",
"AttributePathExpandIterator.cpp",
"AttributePathExpandIterator.h",
"AttributePersistenceProvider.h",
Expand Down Expand Up @@ -315,15 +333,11 @@ static_library("app") {
# "CommandHandler._h"
# "ReadHandler._h",
# "WriteHandler._h"

# TODO: the following items cannot be included due to platform includes not being
# able to depend on src/app
# Name with _ so that linter does not recognize it
# "_AttributeAccessInterface._h",
]

public_deps = [
":app_config",
":attribute-access",
":constants",
":global-attributes",
":interaction-model",
Expand Down
1 change: 1 addition & 0 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "access/SubjectDescriptor.h"
#include <app/AppConfig.h>
#include <app/RequiredPrivilege.h>
#include <app/util/IMClusterCommandHandler.h>
#include <app/util/af-types.h>
#include <app/util/ember-compatibility-functions.h>
#include <app/util/endpoint-config-api.h>
Expand Down
10 changes: 0 additions & 10 deletions src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -712,15 +712,5 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
uint32_t mMagic = 0;
};

void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip::TLV::TLVReader & aReader,
CommandHandler * apCommandObj);

/**
* Get the registered attribute access override. nullptr when attribute access override is not found.
*
* TODO(#16806): This function and registerAttributeAccessOverride can be member functions of InteractionModelEngine.
*/
AttributeAccessInterface * GetAttributeAccessOverride(EndpointId aEndpointId, ClusterId aClusterId);

} // namespace app
} // namespace chip
1 change: 1 addition & 0 deletions src/app/WriteHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "messaging/ExchangeContext.h"
#include <app/AppConfig.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/InteractionModelEngine.h>
#include <app/MessageDef/EventPathIB.h>
#include <app/StatusResponse.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <app-common/zap-generated/cluster-objects.h>

#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/CommandHandler.h>
#include <app/ConcreteCommandPath.h>
#include <app/EventLogging.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include <app-common/zap-generated/cluster-objects.h>
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/CommandHandler.h>
#include <app/ConcreteCommandPath.h>
#include <app/EventLogging.h>
Expand Down
Loading
Loading