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

[BUG] Clusters::ScenesManagement::ScenesServer error / SceneTableImpl.h:33:79: error: expected primary-expression before ‘;’ token #35378

Closed
lboue opened this issue Sep 3, 2024 · 3 comments · Fixed by #35406
Labels
bug Something isn't working linux needs triage

Comments

@lboue
Copy link
Contributor

lboue commented Sep 3, 2024

Reproduction steps

Reproduction steps

Build Chef waterleakdetector for Linux with ScenesManagement enabled by defaut:

cd ~/connectedhomeip
scripts/activate.sh
./examples/chef/chef.py -t linux -d rootnode_waterleakdetector_0b067acfa3 -zb

Error

In file included from ../third_party/connectedhomeip/src/app/clusters/scenes-server/scenes-server.h:24,
                 from ../third_party/connectedhomeip/src/app/clusters/groups-server/groups-server.cpp:33:
../third_party/connectedhomeip/src/app/clusters/scenes-server/SceneTableImpl.h:33:79: error: expected primary-expression before ‘;’ token
   33 | static constexpr uint16_t kMaxScenesPerEndpoint = SCENES_MANAGEMENT_TABLE_SIZE;
      |                                                                               ^
../third_party/connectedhomeip/src/app/clusters/scenes-server/SceneTableImpl.h:38:37: error: non-constant condition for static assertion
   38 | static_assert(kMaxScenesPerEndpoint <= CHIP_CONFIG_MAX_SCENES_TABLE_SIZE,
../third_party/connectedhomeip/src/app/clusters/scenes-server/SceneTableImpl.h:41:37: error: non-constant condition for static assertion
   41 | static_assert(kMaxScenesPerEndpoint >= 16, "Per spec, kMaxScenesPerEndpoint must be at least 16");
      |               ~~~~~~~~~~~~~~~~~~~~~~^~~~~
../third_party/connectedhomeip/src/app/clusters/scenes-server/SceneTableImpl.h:42:77: error: conversion from ‘int’ to ‘uint16_t’ {aka ‘short unsigned int’} may change value [-Werror=conversion]
   42 | static constexpr uint16_t kMaxScenesPerFabric = (kMaxScenesPerEndpoint - 1) / 2;

It comes from these lines:

#ifdef SCENES_MANAGEMENT_TABLE_SIZE
static constexpr uint16_t kMaxScenesPerEndpoint = SCENES_MANAGEMENT_TABLE_SIZE;
#else
static constexpr uint16_t kMaxScenesPerEndpoint = CHIP_CONFIG_MAX_SCENES_TABLE_SIZE;
#endif

Bug prevalence

Always

GitHub hash of the SDK that was being used

5dd517c

Platform

core

Platform Version(s)

N/A

Anything else?

Build log
chef_ScenesManagement_enabled_error.log

Workaround

Disabling ScenesManagement Clusters works as a workaround

./examples/chef/chef.py -t linux -d rootnode_waterleakdetector_0b067acfa3 -g
./examples/chef/linux/out/rootnode_waterleakdetector_0b067acfa3

image

@lboue lboue added bug Something isn't working needs triage labels Sep 3, 2024
@github-actions github-actions bot added the linux label Sep 3, 2024
@lpbeliveau-silabs
Copy link
Contributor

There are two problems here causing this, one is that, in its .zap file, the app defines "name": "SceneTableSize", value as
"defaultValue": "", (which should not be accepted);

And SceneTableImp.h uses: #ifdef SCENES_MANAGEMENT_TABLE_SIZE instead of #if defined(SCENES_MANAGEMENT_TABLE_SIZE) && SCENES_MANAGEMENT_TABLE_SIZE.

Note that the later would still fail on an empty define but the error message would at least be more clear.

Will open a PR shortly to fix both problems.

@lboue
Copy link
Contributor Author

lboue commented Sep 4, 2024

There are two problems here causing this, one is that, in its .zap file, the app defines "name": "SceneTableSize", value as "defaultValue": "", (which should not be accepted);

This comes directly from the template file for chef devices. The problem should be corrected at source:

"name": "SceneTableSize",
"code": 1,
"mfgCode": null,
"side": "server",
"type": "int16u",
"included": 1,
"storageOption": "RAM",
"singleton": 0,
"bounded": 0,
"defaultValue": "",

And SceneTableImp.h uses: #ifdef SCENES_MANAGEMENT_TABLE_SIZE instead of #if defined(SCENES_MANAGEMENT_TABLE_SIZE) && SCENES_MANAGEMENT_TABLE_SIZE.

Note that the later would still fail on an empty define but the error message would at least be more clear.

Will open a PR shortly to fix both problems.

Thanks

lboue added a commit to lboue/connectedhomeip that referenced this issue Sep 4, 2024
@lpbeliveau-silabs
Copy link
Contributor

As mentioned here in the PR comment, turns out that waterleakdetector does not need scene.

Thus I will remove the cluster from the template as well to avoid this scenario in the future for other apps that might add Scenes by mistake.

@mergify mergify bot closed this as completed in #35406 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working linux needs triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants