-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix handling of optional struct-typed command arguments in chip-tool. #31658
Fix handling of optional struct-typed command arguments in chip-tool. #31658
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering why e.g. the OpenDuration in Valve Configuration and Control cluster is not getting marked here 🤔
AddArgument("OpenDuration", 0, UINT32_MAX, &mRequest.openDuration);
Maybe there is an issue in the XML for that cluster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will likely break the chip-tool command lines in some test cases and so we should give a heads up to folks like @Rajashreekalmane
PR #31658: Size comparison from 16409ca to 8a10fba Decreases (1 build for efr32)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
AddArgument documents that for "complex" (i.e. list and struct) arguments there is no automatic type-based handling of optionality and the "kOptional" flag needs to be passed in explicitly. But the generated code was not doing that.
8a10fba
to
fbf4f43
Compare
PR #31658: Size comparison from 4aa833d to fbf4f43 Decreases (1 build for efr32)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
It doesn't need to, because in that case it's using It's only struct-typed command fields that use a different type for the AddArgument and hence need the manual bit. |
Okay, I see. Thanks for clarifying. |
AddArgument documents that for "complex" (i.e. list and struct) arguments there is no automatic type-based handling of optionality and the "kOptional" flag needs to be passed in explicitly. But the generated code was not doing that.