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

ZCL Cleanup Plugin Macro #3627

Closed
jepenven-silabs opened this issue Nov 3, 2020 · 3 comments · Fixed by #3872
Closed

ZCL Cleanup Plugin Macro #3627

jepenven-silabs opened this issue Nov 3, 2020 · 3 comments · Fixed by #3872

Comments

@jepenven-silabs
Copy link
Contributor

Problem

Since plugins macros are not generated anymore by ZAP they were moved to /src/app/util/plugin-config.h However no cleanup was done to see which one we should keep.

Proposed Solution

Removed unused macro from /src/app/util/plugin-config.h

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.61. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@vivien-apple
Copy link
Contributor

I have gone thought all the macros. Here is the list of macros that are currently used from the set of macros from plugin-config.h from #3464 :

#define EMBER_BINDING_TABLE_SIZE 10
#define EMBER_AF_PLUGIN_COLOR_CONTROL_SERVER_XY
#define EMBER_AF_PLUGIN_COLOR_CONTROL_SERVER_TEMP
#define EMBER_AF_PLUGIN_COLOR_CONTROL_SERVER_HSV
#define EMBER_AF_PLUGIN_GROUPS_SERVER 
#define EMBER_AF_PLUGIN_IAS_ZONE_CLIENT
#define EMBER_AF_PLUGIN_IAS_ZONE_CLIENT_MAX_DEVICES 10
#define EMBER_AF_PLUGIN_IAS_ZONE_SERVER_ZONE_TYPE 541
#define EMBER_AF_PLUGIN_LEVEL_CONTROL
#define EMBER_AF_PLUGIN_LEVEL_CONTROL_MAXIMUM_LEVEL 255
#define EMBER_AF_PLUGIN_LEVEL_CONTROL_MINIMUM_LEVEL 0
#define EMBER_AF_PLUGIN_LEVEL_CONTROL_RATE 0
#define EMBER_AF_PLUGIN_REPORTING
#define EMBER_AF_PLUGIN_REPORTING_TABLE_SIZE 5
#define EMBER_AF_PLUGIN_REPORTING_ENABLE_GROUP_BOUND_REPORTS
#define EMBER_AF_PLUGIN_SCENES
#define EMBER_AF_PLUGIN_SCENES_TABLE_SIZE 3

@vivien-apple
Copy link
Contributor

That said I don't think adding src/app/util/plugin-config.h is the way to go.

I would have expect ZAP to have generated something in gen_config.h.

For example, if the groups server is enabled, it could have generated:

#define EMBER_AF_PLUGIN_GROUPS_SERVER

Of if any of the attributes is tagged as reportable, it could have generated:

#define EMBER_AF_PLUGIN_REPORTING

Or if the Level Control server has been selected, it could have generated:

#define EMBER_AF_PLUGIN_LEVEL_CONTROL
#define EMBER_AF_PLUGIN_LEVEL_CONTROL_MAXIMUM_LEVEL 255
#define EMBER_AF_PLUGIN_LEVEL_CONTROL_MINIMUM_LEVEL 0
#define EMBER_AF_PLUGIN_LEVEL_CONTROL_RATE 0

Or depending on which commands have been selected for the Color Control cluster it may have generated:

#define EMBER_AF_PLUGIN_COLOR_CONTROL_SERVER_XY

but not

#define EMBER_AF_PLUGIN_COLOR_CONTROL_SERVER_TEMP

I understand that the generated templates may differs for a different framework than CHIP, but since in this case we are speaking of templates targeting CHIP itself I would have expected those to match the current code architecture.

So I think that's something we need to fix in the current templates that will land in #3638 since having those #define be part of src/app/ have some side effects on the other clusters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants