From 26fa72f7f29b20618e42bb177bb57a532132ffc4 Mon Sep 17 00:00:00 2001
From: Andrei Litvin <andreilitvin@google.com>
Date: Fri, 24 May 2024 10:47:48 -0400
Subject: [PATCH 1/5] Initial version of splitting out command handler
dependencies
---
.github/workflows/lint.yml | 1 -
src/app/BUILD.gn | 47 +++++++++++++++++++++++++++++++++-----
src/app/CommandHandler.cpp | 14 +++---------
src/app/CommandHandler.h | 3 +--
4 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 4685948fd7bd12..bd5385580b9c22 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -98,7 +98,6 @@ jobs:
--known-failure controller/ExamplePersistentStorage.cpp \
--known-failure controller/ExamplePersistentStorage.h \
--known-failure app/AttributeAccessToken.h \
- --known-failure app/CommandHandler.h \
--known-failure app/CommandHandlerInterface.h \
--known-failure app/CommandResponseSender.h \
--known-failure app/CommandSenderLegacyCallback.h \
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
index 4bb4a05a555d39..6c4137de12c708 100644
--- a/src/app/BUILD.gn
+++ b/src/app/BUILD.gn
@@ -179,9 +179,6 @@ static_library("interaction-model") {
"ReadClient.h", # TODO: cpp is only included conditionally. Needs logic
# fixing
"ReadPrepareParams.h",
- "RequiredPrivilege.h",
- "StatusResponse.cpp",
- "StatusResponse.h",
"SubscriptionResumptionStorage.h",
"TimedHandler.cpp",
"TimedHandler.h",
@@ -210,6 +207,7 @@ static_library("interaction-model") {
public_deps = [
":app_config",
+ ":command-handler",
":constants",
":paths",
":subscription-info-provider",
@@ -301,6 +299,46 @@ static_library("attribute-access") {
]
}
+source_set("required-privileges") {
+ sources = [ "RequiredPrivilege.h" ]
+ public_deps = [
+ ":paths",
+ "${chip_root}/src/access:types",
+ ]
+}
+
+source_set("status-response") {
+ sources = [
+ "StatusResponse.cpp",
+ "StatusResponse.h",
+ ]
+ public_deps = [
+ ":constants",
+ "${chip_root}/src/app/MessageDef",
+ "${chip_root}/src/messaging",
+ ]
+}
+
+source_set("command-handler") {
+ sources = [
+ "CommandHandler.cpp",
+ "CommandHandler.h",
+ "CommandHandlerExchangeInterface.h",
+ ]
+
+ public_deps = [
+ ":paths",
+ ":required-privileges",
+ ":status-response",
+ "${chip_root}/src/access:types",
+ "${chip_root}/src/app/MessageDef",
+ "${chip_root}/src/app/data-model",
+ "${chip_root}/src/app/util:callbacks",
+ "${chip_root}/src/lib/support",
+ "${chip_root}/src/messaging",
+ ]
+}
+
# 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") {
@@ -312,8 +350,6 @@ static_library("app") {
"AttributePersistenceProvider.h",
"ChunkedWriteCallback.cpp",
"ChunkedWriteCallback.h",
- "CommandHandler.cpp",
- "CommandHandlerExchangeInterface.h",
"CommandResponseHelper.h",
"CommandResponseSender.cpp",
"DefaultAttributePersistenceProvider.cpp",
@@ -338,7 +374,6 @@ static_library("app") {
# (app depending on im and im including these headers):
# Name with _ so that linter does not recognize it
# "CommandResponseSender._h"
- # "CommandHandler._h"
# "ReadHandler._h",
# "WriteHandler._h"
]
diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
index 86ef22ab886212..d9a031a369a254 100644
--- a/src/app/CommandHandler.cpp
+++ b/src/app/CommandHandler.cpp
@@ -15,28 +15,20 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-/**
- * @file
- * This file defines object for a CHIP IM Invoke Command Handler
- *
- */
-
-#include "CommandHandler.h"
-#include "InteractionModelEngine.h"
-#include "RequiredPrivilege.h"
-#include "messaging/ExchangeContext.h"
+#include <app/CommandHandler.h>
#include <access/AccessControl.h>
#include <app-common/zap-generated/cluster-objects.h>
#include <app/RequiredPrivilege.h>
#include <app/util/MatterCallbacks.h>
+#include <app/StatusResponse.h>
#include <credentials/GroupDataProvider.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/TLVData.h>
#include <lib/core/TLVUtilities.h>
#include <lib/support/IntrusiveList.h>
#include <lib/support/TypeTraits.h>
+#include <messaging/ExchangeContext.h>
#include <platform/LockTracker.h>
#include <protocols/secure_channel/Constants.h>
diff --git a/src/app/CommandHandler.h b/src/app/CommandHandler.h
index cce19879a6c8a9..92432bc1d9c283 100644
--- a/src/app/CommandHandler.h
+++ b/src/app/CommandHandler.h
@@ -30,9 +30,8 @@
#pragma once
-#include "CommandPathRegistry.h"
-
#include <app/CommandHandlerExchangeInterface.h>
+#include <app/CommandPathRegistry.h>
#include <app/ConcreteCommandPath.h>
#include <app/data-model/Encode.h>
#include <lib/core/CHIPCore.h>
From 9f26d0111ebaf06a6b4542430d8a8f0b9228d643 Mon Sep 17 00:00:00 2001
From: Andrei Litvin <andreilitvin@google.com>
Date: Fri, 24 May 2024 10:48:17 -0400
Subject: [PATCH 2/5] Restyle
---
src/app/CommandHandler.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
index d9a031a369a254..6077e0934721f7 100644
--- a/src/app/CommandHandler.cpp
+++ b/src/app/CommandHandler.cpp
@@ -20,8 +20,8 @@
#include <access/AccessControl.h>
#include <app-common/zap-generated/cluster-objects.h>
#include <app/RequiredPrivilege.h>
-#include <app/util/MatterCallbacks.h>
#include <app/StatusResponse.h>
+#include <app/util/MatterCallbacks.h>
#include <credentials/GroupDataProvider.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/TLVData.h>
From 1cae8738687b33ef8d2daa089120a3733f7f6c0d Mon Sep 17 00:00:00 2001
From: Andrei Litvin <andreilitvin@google.com>
Date: Fri, 24 May 2024 11:22:06 -0400
Subject: [PATCH 3/5] move privilege-storage based on dynamic server build
rules...this is somewhat messy
---
src/app/BUILD.gn | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
index 6c4137de12c708..6e0aea0cdf0c79 100644
--- a/src/app/BUILD.gn
+++ b/src/app/BUILD.gn
@@ -246,8 +246,6 @@ static_library("interaction-model") {
"dynamic_server/AccessControl.cpp",
"dynamic_server/AccessControl.h",
"dynamic_server/DynamicDispatcher.cpp",
- "util/privilege-storage.cpp",
- "util/privilege-storage.h",
]
public_deps += [
@@ -305,6 +303,12 @@ source_set("required-privileges") {
":paths",
"${chip_root}/src/access:types",
]
+ if (chip_build_controller_dynamic_server) {
+ sources += [
+ "util/privilege-storage.cpp",
+ "util/privilege-storage.h",
+ ]
+ }
}
source_set("status-response") {
From efc1918a3f7ad58a7380a57f1158555f69abf77c Mon Sep 17 00:00:00 2001
From: Andrei Litvin <andreilitvin@google.com>
Date: Fri, 24 May 2024 11:33:22 -0400
Subject: [PATCH 4/5] More dynamic server fixes
---
src/app/BUILD.gn | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
index 6e0aea0cdf0c79..1fdea4808308cf 100644
--- a/src/app/BUILD.gn
+++ b/src/app/BUILD.gn
@@ -308,6 +308,14 @@ source_set("required-privileges") {
"util/privilege-storage.cpp",
"util/privilege-storage.h",
]
+
+ public_deps += [
+ ":global-attributes",
+ "${chip_root}/src/access",
+ "${chip_root}/src/app/dynamic_server:mock-codegen-includes",
+ ]
+
+ public_configs += [ ":config-controller-dynamic-server" ]
}
}
From 895b9b3f2f884aa4e96e5e4804fcad77701d84e0 Mon Sep 17 00:00:00 2001
From: Andrei Litvin <andreilitvin@google.com>
Date: Fri, 24 May 2024 11:35:49 -0400
Subject: [PATCH 5/5] Another gn fix for android/dynamic-server builds
---
src/app/BUILD.gn | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
index 1fdea4808308cf..f49a40f398842a 100644
--- a/src/app/BUILD.gn
+++ b/src/app/BUILD.gn
@@ -299,10 +299,12 @@ static_library("attribute-access") {
source_set("required-privileges") {
sources = [ "RequiredPrivilege.h" ]
+
public_deps = [
":paths",
"${chip_root}/src/access:types",
]
+
if (chip_build_controller_dynamic_server) {
sources += [
"util/privilege-storage.cpp",
@@ -315,7 +317,7 @@ source_set("required-privileges") {
"${chip_root}/src/app/dynamic_server:mock-codegen-includes",
]
- public_configs += [ ":config-controller-dynamic-server" ]
+ public_configs = [ ":config-controller-dynamic-server" ]
}
}