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

[Build] lighting-app/python example build is broken #29508

Closed
jamesharrow opened this issue Sep 29, 2023 · 9 comments
Closed

[Build] lighting-app/python example build is broken #29508

jamesharrow opened this issue Sep 29, 2023 · 9 comments

Comments

@jamesharrow
Copy link
Contributor

jamesharrow commented Sep 29, 2023

Build issue(s)

I tried to build the lighting.py example following the https://github.com/project-chip/connectedhomeip/tree/master/examples/lighting-app/python readme
C++ compilation bombs out with an error trying to include:
#include <zap-generated/CHIPClusters.h>
in
src/app/clusters/ota-requestor/DefaultOTARequestor.cpp

  1. cd ~/connectedhomeip/
  2. git submodule update --init
  3. source scripts/activate.sh
  4. ./scripts/build_python_device.sh --chip_detail_logging true

Platform

python

Anything else?

I managed to get it to compile, by modifying DefaultOTARequestor.cpp by making the follow changes (although I'm not confident that you would implement it this way in master):

---
 .../ota-requestor/DefaultOTARequestor.cpp     | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp b/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
index 1bf1588ee9..c272f4f37f 100644
--- a/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
@@ -27,12 +27,29 @@
 #include <platform/DeviceInstanceInfoProvider.h>
 #include <platform/OTAImageProcessor.h>
 #include <protocols/bdx/BdxUri.h>
-#include <zap-generated/CHIPClusters.h>
+// #include <zap-generated/CHIPClusters.h>
+#include <app-common/zap-generated/ids/Clusters.h>
+#include <app-common/zap-generated/ids/Commands.h>
+#include <app/CommandHandler.h>
+#include <controller/CHIPDeviceController.h>
 
 #include "BDXDownloader.h"
 #include "DefaultOTARequestor.h"
 
 namespace chip {
+namespace Controller {
+
+class DLL_EXPORT OtaSoftwareUpdateProviderCluster : public ClusterBase
+{
+public:
+    OtaSoftwareUpdateProviderCluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session,
+                                     EndpointId endpoint) :
+        ClusterBase(exchangeManager, session, endpoint)
+    {}
+    ~OtaSoftwareUpdateProviderCluster() {}
+};
+
+} // namespace Controller
 
 using namespace app;
 using namespace app::Clusters;
-- 
2.34.1

@bzbarsky-apple
Copy link
Contributor

Sounds like something in that build script is not doing the necessary codegen properly or not setting up the right include paths....

@markus-becker-tridonic-com can you take a look, please?

@bzbarsky-apple bzbarsky-apple changed the title [Build] [Build] lighting-app/python example build is broken Sep 29, 2023
@markus-becker-tridonic-com
Copy link
Contributor

A quick work-around I worked on some time back (e6f610b):

diff --git a/src/app/chip_data_model.gni b/src/app/chip_data_model.gni
index b6aa8f6942..2bfd4f681d 100644
--- a/src/app/chip_data_model.gni
+++ b/src/app/chip_data_model.gni
@@ -75,13 +75,13 @@ template("chip_data_model") {
       prune_outputs = []
     }
 
-    if (chip_controller) {
+#    if (chip_controller) {
       outputs += [ "zap-generated/CHIPClusters.h" ]
-    } else {
-      if (defined(prune_outputs)) {
-        prune_outputs += [ "zap-generated/CHIPClusters.h" ]
-      }
-    }
+#    } else {
+#      if (defined(prune_outputs)) {
+#        prune_outputs += [ "zap-generated/CHIPClusters.h" ]
+#      }
+#    }
 
     # TODO: It is unclear here why `zap_pregenerated_dir` has any relevance
     #       in including IMClusterCommandHandler or not.

@jamesharrow
Copy link
Contributor Author

I have tried this change and it seems to compile for me now. Thanks

@markus-becker-tridonic-com
Copy link
Contributor

we could give an additional argument python_device=true at https://github.com/project-chip/connectedhomeip/blob/master/scripts/build_python_device.sh#L100C170-L100C185 and check for that in src/app/chip_data_model.gni with if (chip_controller || python_device) {.

Additionally, it would be good to add .github/workflows/examples-python-device.yaml.

@bzbarsky-apple @jamesharrow would that work for you?

@bzbarsky-apple
Copy link
Contributor

I'm not sure what python_device is or why it needs special handling here. Please check with @andy31415 what this code is supposed to be doing and why (and in particular, which CHIPClusters.h it should be picking up).

@markus-becker-tridonic-com
Copy link
Contributor

It should pick up the ChipClusters.h from lighting-app.

@andy31415 would you have a hint here?

@bzbarsky-apple
Copy link
Contributor

Honestly, we should just remove CHIPClusters.h; it's pretty useless at this point. I'll do a PR to stop using it in core src/app code.

@bzbarsky-apple
Copy link
Contributor

#29772

markus-becker-tridonic-com added a commit to markus-becker-tridonic-com/connectedhomeip that referenced this issue Oct 19, 2023
Python lighting-app was broken (see project-chip#29508) and got fixed in project-chip#29772.

Let's make sure that it does not break again by compiling it in CI.

Signed-off-by: Markus Becker <markus.becker@tridonic.com>
mergify bot pushed a commit that referenced this issue Oct 19, 2023
Python lighting-app was broken (see #29508) and got fixed in #29772.

Let's make sure that it does not break again by compiling it in CI.

Signed-off-by: Markus Becker <markus.becker@tridonic.com>
@markus-becker-tridonic-com
Copy link
Contributor

@jamesharrow i think this issue can be closed. would you verify?

HunsupJung pushed a commit to HunsupJung/connectedhomeip that referenced this issue Oct 23, 2023
Python lighting-app was broken (see project-chip#29508) and got fixed in project-chip#29772.

Let's make sure that it does not break again by compiling it in CI.

Signed-off-by: Markus Becker <markus.becker@tridonic.com>
shripad621git pushed a commit to shripad621git/connectedhomeip that referenced this issue Oct 31, 2023
Python lighting-app was broken (see project-chip#29508) and got fixed in project-chip#29772.

Let's make sure that it does not break again by compiling it in CI.

Signed-off-by: Markus Becker <markus.becker@tridonic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants