Add orphan header files from src/controller to gn (project-chip#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 <[email protected]>

examples/platform/linux/AppMain.cpp

@@ -19,7 +19,8 @@
 #include <platform/CHIPDeviceLayer.h>
 #include <platform/PlatformManager.h>
 
-#include "app/clusters/network-commissioning/network-commissioning.h"
+#include <app/InteractionModelEngine.h>
+#include <app/clusters/network-commissioning/network-commissioning.h>
 #include <app/server/Dnssd.h>
 #include <app/server/OnboardingCodesUtil.h>
 #include <app/server/Server.h>

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)

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",

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" ]
 }

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,

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<OperationalDeviceProxy *>(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<GroupDeviceProxy *>(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;
     }
 

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",

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",