From 295eea34c65f854d22d3915e862d6db66eda941d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 6 Feb 2024 12:19:34 -0500 Subject: [PATCH] Add orphan header files from `src/controller` to gn (#31882) * Add files from src/controller to gn * Add more orphaned files from controller itself * Restyle * Move more files into sources since CHIPCluster actually includes them. Seems VERY odd to have this header used but not actually have any implementation. This is a bug! * Attempt to fix some of the include oddities. Controller is VERY broken and feels like a god object definition * Move more headers ... CHIPDeviceController strongly pulls the entire world. * Fix typo * One more header * Fix chef * Add more includes that seem needed * Add another dependency * Add includes config to controller as it needs relative includes * Restyle * Fix clang-tidy logic * Apply some clang-tidy fixes since I looked at controller. Minor nullptr usage and remove else after return * Ensure no errors about unused assignments * Yet another fix for logic to make sure all defined lists are used * Include only headers as the special targets to force include errors * Fix typo * Fix typo --------- Co-authored-by: Andrei Litvin --- examples/platform/linux/AppMain.cpp | 3 +- .../linux/ControllerShellCommands.cpp | 1 - src/app/icd/client/BUILD.gn | 1 + src/controller/BUILD.gn | 71 +++++++++++++++---- .../AndroidOperationalCredentialsIssuer.cpp | 6 +- .../java/CHIPDeviceController-JNI.cpp | 24 +++---- src/controller/tests/data_model/BUILD.gn | 1 + src/credentials/BUILD.gn | 1 + 8 files changed, 78 insertions(+), 30 deletions(-) diff --git a/examples/platform/linux/AppMain.cpp b/examples/platform/linux/AppMain.cpp index 061d7b66c5e4c6..71701f74aaedd6 100644 --- a/examples/platform/linux/AppMain.cpp +++ b/examples/platform/linux/AppMain.cpp @@ -19,7 +19,8 @@ #include #include -#include "app/clusters/network-commissioning/network-commissioning.h" +#include +#include #include #include #include diff --git a/examples/platform/linux/ControllerShellCommands.cpp b/examples/platform/linux/ControllerShellCommands.cpp index 27116b28c15256..49b1e20dc97eb8 100644 --- a/examples/platform/linux/ControllerShellCommands.cpp +++ b/examples/platform/linux/ControllerShellCommands.cpp @@ -37,7 +37,6 @@ namespace chip { namespace Shell { using namespace chip; -using namespace ::chip::Controller; #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY static CHIP_ERROR ResetUDC(bool printHeader) diff --git a/src/app/icd/client/BUILD.gn b/src/app/icd/client/BUILD.gn index 1a1a5acdd34ae7..4d59a627a4bcbb 100644 --- a/src/app/icd/client/BUILD.gn +++ b/src/app/icd/client/BUILD.gn @@ -47,6 +47,7 @@ source_set("handler") { public_deps = [ ":manager", "${chip_root}/src/app", + "${chip_root}/src/controller", "${chip_root}/src/lib/core", "${chip_root}/src/messaging", "${chip_root}/src/protocols", diff --git a/src/controller/BUILD.gn b/src/controller/BUILD.gn index 9742bf05684b3c..b06b2defecb8f2 100644 --- a/src/controller/BUILD.gn +++ b/src/controller/BUILD.gn @@ -24,40 +24,87 @@ source_set("nodatamodel") { public_deps = [ "${chip_root}/src/app" ] } +CHIP_CONTROLLER_HEADERS = [ "ExampleOperationalCredentialsIssuer.h" ] +CHIP_READ_CLIENT_HEADERS = [ + "CommissioningWindowOpener.h", + "CurrentFabricRemover.h", +] + +# This source set exists specifically to deny including them without dependencies +# if "controller" sources do not contain them +# +# They are NOT intended to be used directly. +# +# The intent for this is to force gn to be aware that these files are known and +# error out as `include not allowed due to missing dependency` even if +# controller is built without these sources +source_set("gen_check_chip_controller_headers") { + sources = CHIP_CONTROLLER_HEADERS +} + +source_set("gen_check_chip_read_client_headers") { + sources = CHIP_READ_CLIENT_HEADERS +} + +source_set("delegates") { + sources = [ "OperationalCredentialsDelegate.h" ] +} + static_library("controller") { output_name = "libChipController" - sources = [ "CHIPCluster.h" ] + sources = [ + # TODO: these dependencies are broken. Specifically: + # a) ChipDeviceControllerFactory.h includes CHIPDeviceController.h + # b) CHIPDeviceController.cpp is only compiled for read_client + # These conditionals are NOT ok and should have been solved with separate + # source sets - either include some functionality or do not + # + # Then CHIPDeviceController.h pulls in AbstractDnssdDiscoveryController and everything + # cascades. In the end it looks like basically every header file is pulled in. + # + # Unfortunately this is hard to fix in one go, hence these odd list of header-only + # dependencies + "AbstractDnssdDiscoveryController.h", + "AutoCommissioner.h", + "CHIPCluster.h", + "CHIPCommissionableNodeController.h", + "CHIPDeviceController.h", + "CHIPDeviceControllerSystemState.h", + "CommandSenderAllocator.h", + "CommissioneeDeviceProxy.h", + "CommissioningDelegate.h", + "DeviceDiscoveryDelegate.h", + "DevicePairingDelegate.h", + "InvokeInteraction.h", + "ReadInteraction.h", + "SetUpCodePairer.h", + "TypedCommandCallback.h", + "TypedReadCallback.h", + "WriteInteraction.h", + ] if (chip_controller && chip_build_controller) { + sources += CHIP_CONTROLLER_HEADERS sources += [ "AbstractDnssdDiscoveryController.cpp", "AutoCommissioner.cpp", - "AutoCommissioner.h", "CHIPCommissionableNodeController.cpp", - "CHIPCommissionableNodeController.h", "CHIPDeviceControllerFactory.cpp", "CHIPDeviceControllerFactory.h", "CommissioneeDeviceProxy.cpp", - "CommissioneeDeviceProxy.h", "CommissionerDiscoveryController.cpp", "CommissionerDiscoveryController.h", "CommissioningDelegate.cpp", - "DeviceDiscoveryDelegate.h", - "DevicePairingDelegate.h", "ExampleOperationalCredentialsIssuer.cpp", - "ExampleOperationalCredentialsIssuer.h", "SetUpCodePairer.cpp", - "SetUpCodePairer.h", ] if (chip_enable_read_client) { + sources += CHIP_READ_CLIENT_HEADERS sources += [ "CHIPDeviceController.cpp", - "CHIPDeviceController.h", "CommissioningWindowOpener.cpp", - "CommissioningWindowOpener.h", "CurrentFabricRemover.cpp", - "CurrentFabricRemover.h", ] } } @@ -86,5 +133,5 @@ static_library("controller") { deps += [ "${chip_root}/src/lib/support:testing" ] } - defines = [] + public_configs = [ "${chip_root}/src:includes" ] } diff --git a/src/controller/java/AndroidOperationalCredentialsIssuer.cpp b/src/controller/java/AndroidOperationalCredentialsIssuer.cpp index f63f1a46ecbbe3..af9e8adc8621a0 100644 --- a/src/controller/java/AndroidOperationalCredentialsIssuer.cpp +++ b/src/controller/java/AndroidOperationalCredentialsIssuer.cpp @@ -135,10 +135,8 @@ CHIP_ERROR AndroidOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan { return CallbackGenerateNOCChain(csrElements, csrNonce, attestationSignature, attestationChallenge, DAC, PAI, onCompletion); } - else - { - return LocalGenerateNOCChain(csrElements, csrNonce, attestationSignature, attestationChallenge, DAC, PAI, onCompletion); - } + + return LocalGenerateNOCChain(csrElements, csrNonce, attestationSignature, attestationChallenge, DAC, PAI, onCompletion); } CHIP_ERROR AndroidOperationalCredentialsIssuer::CallbackGenerateNOCChain(const ByteSpan & csrElements, const ByteSpan & csrNonce, diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp index 1d3cd938c16ca0..7f311a823850f1 100644 --- a/src/controller/java/CHIPDeviceController-JNI.cpp +++ b/src/controller/java/CHIPDeviceController-JNI.cpp @@ -106,7 +106,7 @@ JavaVM * sJVM; pthread_t sIOThread = PTHREAD_NULL; -jclass sChipDeviceControllerExceptionCls = NULL; +jclass sChipDeviceControllerExceptionCls = nullptr; } // namespace @@ -131,7 +131,7 @@ jint JNI_OnLoad(JavaVM * jvm, void * reserved) // Get a JNI environment object. env = JniReferences::GetInstance().GetEnvForCurrentThread(); - VerifyOrExit(env != NULL, err = CHIP_JNI_ERROR_NO_ENV); + VerifyOrExit(env != nullptr, err = CHIP_JNI_ERROR_NO_ENV); ChipLogProgress(Controller, "Loading Java class references."); @@ -171,7 +171,7 @@ void JNI_OnUnload(JavaVM * jvm, void * reserved) // should be stopped before the library is unloaded. StopIOThread(); - sJVM = NULL; + sJVM = nullptr; chip::Platform::MemoryShutdown(); } @@ -280,7 +280,7 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr { chip::DeviceLayer::StackLock lock; CHIP_ERROR err = CHIP_NO_ERROR; - AndroidDeviceControllerWrapper * wrapper = NULL; + AndroidDeviceControllerWrapper * wrapper = nullptr; jlong result = 0; ChipLogProgress(Controller, "newDeviceController() called"); @@ -478,7 +478,7 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr // Create and start the IO thread. Must be called after Controller()->Init if (sIOThread == PTHREAD_NULL) { - int pthreadErr = pthread_create(&sIOThread, NULL, IOThreadMain, NULL); + int pthreadErr = pthread_create(&sIOThread, nullptr, IOThreadMain, nullptr); VerifyOrExit(pthreadErr == 0, err = CHIP_ERROR_POSIX(pthreadErr)); } @@ -487,7 +487,7 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr exit: if (err != CHIP_NO_ERROR) { - if (wrapper != NULL) + if (wrapper != nullptr) { delete wrapper; } @@ -1391,7 +1391,7 @@ JNI_METHOD(void, releaseOperationalDevicePointer)(JNIEnv * env, jobject self, jl { chip::DeviceLayer::StackLock lock; OperationalDeviceProxy * device = reinterpret_cast(devicePtr); - if (device != NULL) + if (device != nullptr) { delete device; } @@ -1422,7 +1422,7 @@ JNI_METHOD(void, releaseGroupDevicePointer)(JNIEnv * env, jobject self, jlong de { chip::DeviceLayer::StackLock lock; GroupDeviceProxy * device = reinterpret_cast(devicePtr); - if (device != NULL) + if (device != nullptr) { delete device; } @@ -2117,7 +2117,7 @@ JNI_METHOD(void, deleteDeviceController)(JNIEnv * env, jobject self, jlong handl ChipLogProgress(Controller, "deleteDeviceController() called"); - if (wrapper != NULL) + if (wrapper != nullptr) { delete wrapper; } @@ -3067,7 +3067,7 @@ void * IOThreadMain(void * arg) // This allows the JVM to shutdown without waiting for this thread to exit. attachArgs.version = JNI_VERSION_1_6; attachArgs.name = (char *) "CHIP Device Controller IO Thread"; - attachArgs.group = NULL; + attachArgs.group = nullptr; #ifdef __ANDROID__ sJVM->AttachCurrentThreadAsDaemon(&env, (void *) &attachArgs); #else @@ -3081,7 +3081,7 @@ void * IOThreadMain(void * arg) // Detach the thread from the JVM. sJVM->DetachCurrentThread(); - return NULL; + return nullptr; } // NOTE: This function SHALL be called with the stack lock held. @@ -3094,7 +3094,7 @@ CHIP_ERROR StopIOThread() chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); - pthread_join(sIOThread, NULL); + pthread_join(sIOThread, nullptr); sIOThread = PTHREAD_NULL; } diff --git a/src/controller/tests/data_model/BUILD.gn b/src/controller/tests/data_model/BUILD.gn index 3c04f0012f59c6..e7621af5b0359a 100644 --- a/src/controller/tests/data_model/BUILD.gn +++ b/src/controller/tests/data_model/BUILD.gn @@ -34,6 +34,7 @@ chip_test_suite_using_nltest("data_model") { "${chip_root}/src/app/common:cluster-objects", "${chip_root}/src/app/tests:helpers", "${chip_root}/src/app/util/mock:mock_ember", + "${chip_root}/src/controller", "${chip_root}/src/lib/support:testing_nlunit", "${chip_root}/src/messaging/tests:helpers", "${chip_root}/src/transport/raw/tests:helpers", diff --git a/src/credentials/BUILD.gn b/src/credentials/BUILD.gn index dc5b7163be92e3..df7afc0c1025ba 100644 --- a/src/credentials/BUILD.gn +++ b/src/credentials/BUILD.gn @@ -125,6 +125,7 @@ static_library("credentials") { public_deps = [ ":build_time_header", + "${chip_root}/src/controller:delegates", "${chip_root}/src/crypto", "${chip_root}/src/lib/asn1", "${chip_root}/src/lib/core",