Switch yaml tests to use ClusterBase::WriteAttribute for attribute write. #11460
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pullapprove
bot
requested review from
anush-apple,
austinh0,
balducci-apple,
carol-apple,
cecille,
chrisdecenzo,
chulspro,
Damian-Nordic,
electrocucaracha,
erjiaqing,
franck-apple,
hawk248,
holbrookt,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kghost,
LuDuda,
mlepage-google,
mrjerryjohns,
msandstedt,
pan-apple,
robszewczyk,
sagar-apple,
saurabhst,
selissia,
tcarmelveilleux and
tecimovic
pullapprove
bot
requested review from
vivien-apple,
wbschiller,
woody-apple and
yunhanw-google
|
PR #11460: Size comparison from 39ff9bf to 645f948 Increases above 0.2%:
Increases (1 build for linux)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
bzbarsky-apple
force-pushed
the
use-template-for-writes
branch
from
645f948 to
f59184d
Compare
|
PR #11460: Size comparison from 7b71fb2 to f59184d Increases above 0.2%:
Increases (1 build for linux)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
bzbarsky-apple
force-pushed
the
use-template-for-writes
branch
from
f59184d to
a0d2c95
Compare
bzbarsky-apple
force-pushed
the
use-template-for-writes
branch
from
a0d2c95 to
2eb3e53
Compare
andy31415
reviewed
Nov 5, 2021
|
PR #11460: Size comparison from 11c32e8 to 2eb3e53 Increases above 0.2%:
Increases (2 builds for esp32, linux)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
mrjerryjohns
approved these changes
Nov 5, 2021
…ite. The changes to test_cluster.zapt are the actual change we care about. The change to WriteClient.h is to fix a pre-existing bug: it calls Encode() without including all the headers that declare various signatures of Encode(). The change to ClusterTestGeneration.js fixes a bug with the signature of the success callback for writes: it had an extra arg with the type of the attribute, whereas it should have no arg at all. The change to CHIPClusters-src.zapt is to instantiate WriteAttribute for all the non-writable attributes when compiling tests, because we actually issue writes for those non-writable attributes (and test that they fail). The change to controller/data_model/BUILD.gn is because the Android compiler in CI blows up trying to compile CHIPClustersTestWrite.cpp.
bzbarsky-apple
force-pushed
the
use-template-for-writes
branch
from
2eb3e53 to
555e6b6
Compare
|
PR #11460: Size comparison from 7addec8 to 555e6b6 Increases above 0.2%:
Increases (2 builds for esp32, linux)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
jmartinez-silabs
approved these changes
Nov 5, 2021
woody-apple
approved these changes
Nov 5, 2021
PSONALl
pushed a commit
to PSONALl/connectedhomeip
that referenced
this pull request
Dec 3, 2021
…ite. (project-chip#11460) The changes to test_cluster.zapt are the actual change we care about. The change to WriteClient.h is to fix a pre-existing bug: it calls Encode() without including all the headers that declare various signatures of Encode(). The change to ClusterTestGeneration.js fixes a bug with the signature of the success callback for writes: it had an extra arg with the type of the attribute, whereas it should have no arg at all. The change to CHIPClusters-src.zapt is to instantiate WriteAttribute for all the non-writable attributes when compiling tests, because we actually issue writes for those non-writable attributes (and test that they fail). The change to controller/data_model/BUILD.gn is because the Android compiler in CI blows up trying to compile CHIPClustersTestWrite.cpp.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes to test_cluster.zapt are the actual change we care about.
The change to WriteClient.h is to fix a pre-existing bug: it calls
Encode() without including all the headers that declare various
signatures of Encode().
The change to ClusterTestGeneration.js fixes a bug with the signature
of the success callback for writes: it had an extra arg with the type
of the attribute, whereas it should have no arg at all.
The change to CHIPClusters-src.zapt is to instantiate WriteAttribute
for all the non-writable attributes when compiling tests, because we
actually issue writes for those non-writable attributes (and test that
they fail).
Problem
We are using CHIPClusters APIs that don't support our full range of types.
Change overview
See above.
Testing
Read generated code to make sure it made sense. Ran tests job locally for TestClusterComplexTypes and it passed.