Initial support for adding codegen for interaction model #4298
Conversation
pullapprove
bot
requested review from
andy31415,
BroderickCarlin,
bzbarsky-apple,
chrisdecenzo,
hawk248,
jelderton,
mspang,
rwalker-apple and
saurabhst
|
Size diff reports 4K extra code and 1K extra BSS. Are these size diffs expected? |
src/app/im-encoder.cpp
Outdated
| TLV::TLVType dummyType = TLV::kTLVType_NotSpecified; | ||
|
|
||
| uint8_t argSeqNumber = 0; | ||
| (void) argSeqNumber; |
There was a problem hiding this comment.
this (void) does not seem needed as argSeqNumber is used. Same below.
There was a problem hiding this comment.
This is a generated code, some commands does not have arguments and compiler will complain.
There was a problem hiding this comment.
Can code generation detect this?
Ideally I would like even generated code to be readable and look close to human-generated. an (if no arguments, add void statement) seems to be easy enough to place in a template.
There was a problem hiding this comment.
Maybe zcl_command_arguments_count ?
src/app/zap-templates/templates/chip/python-ChipDeviceController-ClusterCommands.zapt
Outdated
Show resolved
Hide resolved
src/app/zap-templates/templates/chip/python-chip-ChipCluster.zapt
Outdated
Show resolved
Hide resolved
src/app/zap-templates/templates/chip/python-chip-ChipCluster.zapt
Outdated
Show resolved
Hide resolved
src/app/zap-templates/templates/chip/python-chip-ChipCluster.zapt
Outdated
Show resolved
Hide resolved
src/app/zap-templates/templates/chip/python-chip-ChipCluster.zapt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
please request my re-review once @mspang's comments are addressed
erjiaqing
requested review from
jepenven-silabs,
vivien-apple and
rwalker-apple
| @@ -202,6 +205,9 @@ CHIP_ERROR nl_Chip_Stack_Shutdown(); | |||
| const char * nl_Chip_Stack_ErrorToString(CHIP_ERROR err); | |||
There was a problem hiding this comment.
nit for future PRs: will we remove/replace the "nl_" prefix at some point?
|
@mspang, are your comments all resolved? |
| return CHIP_ERROR_NO_MEMORY; | ||
| } | ||
|
|
||
| strncpy(value, val->second.c_str(), val->second.size() + 1); |
| { | ||
| mStorage[key] = value; | ||
| ChipLogDetail(Controller, "SetKeyValue: K=%s", key); | ||
| ChipLogDetail(Controller, "SetKeyValue: V=%s", value); |
There was a problem hiding this comment.
Why separate logs for key and value?
…p#4298) Co-authored-by: Restyled.io <[email protected]>
…p#4298) Co-authored-by: Restyled.io <[email protected]>
…p#4298) Co-authored-by: Restyled.io <[email protected]>
Problem
Interaction model is merged but not enabled.
Summary of Changes
This PR implemented ZAP templates and some necessary API in device and controller to support interaction model.
Testing
Useful snippet for device controller:
./gn_build.sh chip_devctrl_use_interaction_model=true chip_app_use_interation_model=true && (yes | sudo pip3 uninstall chip) && sudo pip3 install ./out/debug/linux_arm64_clang/controller/python/chip-0.0-cp38-cp38-linux_aarch64.whlBuild linux lighting app:
gn gen out --args="chip_app_use_interation_model=true" && ninja -C out1.5 Config IPv6 address on device running controller and device running lighting app
Run chip-device-controller and linux lighting app
In chip-device-controller, run:
Fixes #4253