-
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
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
…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.
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)
|
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.
Need to confirm that the nullable quality for each attributes where "null" was assigned as default in the .zap files.
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:
***@***.***>
|
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.
The problems identified were due to our zap version, so no action needs to be taken in this PR.
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)
|
…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).
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