Add support for DEMM 3.1,3.2,3.3 and EVSEM 3.1,3.2,3.3 test case by enabling StartUpMode and OnMode attributes #31653
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
…ode and OnMode attributes in DEMM and EVSEM on EP1 to allow DEMM3.1,3.2,3.3 and EVSEM3.1,3.2,3.3 to be tested.
jamesharrow
changed the title
|
PR #31653: Size comparison from a386c83 to b7e05b1 Increases (8 builds for esp32, linux, nrfconnect, psoc6, telink)
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)
|
pullapprove
bot
requested review from
andyg-apple,
anush-apple,
arkq,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
joonhaengHeo,
jtung-apple,
kkasperczyk-no,
kpschoedel,
ksperling-apple,
lazarkov and
lpbeliveau-silabs
pullapprove
bot
requested review from
younghak-hwang,
yufengwangca and
yunhanw-google
andy31415
approved these changes
Jan 24, 2024
examples/energy-management-app/energy-management-common/energy-management-app.zap
Outdated
Show resolved
Hide resolved
examples/energy-management-app/energy-management-common/energy-management-app.zap
Outdated
Show resolved
Hide resolved
examples/energy-management-app/energy-management-common/energy-management-app.zap
Outdated
Show resolved
Hide resolved
|
I think it's a process issue (not me seeing it change each time I run zap).
i.e. if someone else isn't doing an rm -rf .environment, and then running
bootstrap to pick up the latest ZAP, or if someone has a local copy of ZAP
installed.
When they generate the .zap files and check it in, then they commit
something with/without logs[] or defaultValue: null or "".
When someone else who DOES use the latest ZAP tool edits a file, and
re-runs regen_all, they check in a different set of changes.
The issue is no one knows what the right values SHOULD be.
Or the CI checkers (which presumably are always the latest ZAP) could check
this. But I think they only run the zap-cli and this can do the comparison
of the regen_all output (based on .zap file as an input).
But could it also open the zap file (as if it were the gui) and save it
again - and then it might force humans to update their ZAP tools on their
machines.
Or maybe there's a simpler way just to get the ZAP version embedded into
the .zap file and have a CI tool to check 'hey this version was committed
with an out of date zap tool'. Maybe that's already there...?
…On Tue, 30 Jan 2024 at 17:09, paulr34 ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
examples/energy-management-app/energy-management-common/energy-management-app.zap
<#31653 (comment)>
:
> @@ -3502,7 +3534,7 @@
"storageOption": "External",
"singleton": 0,
"bounded": 0,
- "defaultValue": "",
+ "defaultValue": null,
@jamesharrow <https://github.com/jamesharrow> have you personally seen
ZAP randomly change values or do you mean when different people use ZAP the
values change? This is important because either ZAP has an underlying issue
involving saving files (unresolved promise?) OR maybe we have a
distribution problem. Maybe people are using different versions of ZAP and
saving different ZAP files which in turn overwrite the data? Regardless, in
order to get to the bottom of the issue it would be highly productive if a
bug was filed with ZAP versions and steps to reproduce.
Thank you for pointing this issue out!
—
Reply to this email directly, view it on GitHub
<#31653 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AWMSBN3IXD3UDDIXNE3E3L3YRESMXAVCNFSM6AAAAABCIVXXGWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJRHA4TONZVGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
lpbeliveau-silabs
approved these changes
Jan 30, 2024
|
FYI -
$zap --version
Expected electron version: 17.4.x,18.x.x,24.x.x
Provided electron version: 27.0.3
WARNING: you are using different electron version that recommended.
Version: 2023.12.6
Feature level: 99
Hash: 0aed125529d4a5f4d2741a5c4bb446568b9e7bc4
Date: 2023-12-06T13:43:13.000Z
Mode: source
…On Tue, 30 Jan 2024 at 18:04, lpbeliveau-silabs ***@***.***> wrote:
***@***.**** approved this pull request.
The problems identified were due to our zap version, so no action needs to
be taken in this PR.
—
Reply to this email directly, view it on GitHub
<#31653 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AWMSBN7BDWAM24IUX7FVEYLYREY4FAVCNFSM6AAAAABCIVXXGWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJSGAYTEMZRG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
@jamesharrow could you merge to resolve conflicts? |
Sure - I had forgotten that this one hadn't merged... I'll also have to review the latest view on these mode clusters if some of the tests are still appropriate (e.g. EVSEM and DEMM) |
…E-and-STARTMODE-MODE-attributes-enabled-for-testing
…E-and-STARTMODE-MODE-attributes-enabled-for-testing
…app.zap that needed to be resolved due to merge conflict (turn on startupmode and onmode in EVSEM, DEMM).
|
PR #31653: Size comparison from 3418614 to ac83b5a Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
j-ororke
pushed a commit
to j-ororke/connectedhomeip
that referenced
this pull request
Jul 31, 2024
…nabling StartUpMode and OnMode attributes (project-chip#31653) * Updated all-clusters-app and energy-management-app to enable StartUpMode and OnMode attributes in DEMM and EVSEM on EP1 to allow DEMM3.1,3.2,3.3 and EVSEM3.1,3.2,3.3 to be tested. * Re-applied the changes to all-clusters-app.zap and energy-management-app.zap that needed to be resolved due to merge conflict (turn on startupmode and onmode in EVSEM, DEMM).
jamesharrow
deleted the
DEMM-and-EVSEM-should-have-ON-MODE-and-STARTMODE-MODE-attributes-enabled-for-testing
branch
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.
Updated all-clusters-app and energy-management-app to enable StartUpMode and OnMode attributes
in DEMM and EVSEM on EP1 to allow DEMM3.1,3.2,3.3 and EVSEM3.1,3.2,3.3 to be tested.
Changes