-
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
Implement DataModel based read support in the interaction model engine. #34419
Merged
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
This implements reads via ember and via codegen as well as a checked-mode where reads are validated as returning the same status codes and data size (because data content is not available)
pullapprove
bot
requested review from
andyg-apple,
anush-apple,
arkq,
axelnxp,
bauerschwan,
bzbarsky-apple,
carol-apple,
cecille,
chapongatien,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
doru91,
fessehaeve,
harimau-qirex,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
joonhaengHeo and
jtung-apple
July 19, 2024 17:23
bzbarsky-apple
approved these changes
Jul 23, 2024
Co-authored-by: Boris Zbarsky <[email protected]>
PR #34419: Size comparison from c0d76b3 to dd92d3e Full report (17 builds for cc13x4_26x4, cc32xx, mbed, nxp, qpg, stm32, tizen)
|
PR #34419: Size comparison from c0d76b3 to 008e44e Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
approved these changes
Jul 24, 2024
PR #34419: Size comparison from c0d76b3 to 5ea3356 Increases above 0.2%:
Full report (17 builds for cc13x4_26x4, cc32xx, mbed, nxp, qpg, stm32, tizen)
|
PR #34419: Size comparison from 6768739 to c91c21b Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nxp, psoc6, qpg, stm32, telink, tizen)
|
…to debug cast failures
PR #34419: Size comparison from 6768739 to 6019d0e Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nxp, psoc6, qpg, stm32, telink, tizen)
|
j-ororke
pushed a commit
to j-ororke/connectedhomeip
that referenced
this pull request
Jul 31, 2024
…e. (project-chip#34419) * Implement DataModel based read support in the interacton model engine. This implements reads via ember and via codegen as well as a checked-mode where reads are validated as returning the same status codes and data size (because data content is not available) * Comment update and logic update: we MAY get different sizes if data size changes for numbers * Fix typo for ember implementation while renaming things * Fix override markers * run_tv_casting_test should be executable * Do not report errors on chunking * Typo fix * move chipDie for tests only * move all chipdie to unit test only * fix comment and formatting * Update encoderstate logic a bit - code is cleaner, will have to see if CI tests pass * Restyle * Enable tracing of messages for tv tests, to see what is going on in CI * Restyle * Start adding some log line processing for the tv tests, to have propper timeouts and not block on IO buffers * Significant reformat and start refactoring the tv casting test * TV tests pass * Restyle * Fix ruff * Review comment update: set state in all cases * Added a TODO regarding the awkward "callback on success only" * Merge fix * Update src/app/reporting/Read-Checked.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Review updates * Fix placement of dm state * Restyle * Code review comments: double-check that IM not active when setting model, explain why we have the ifdef * Code review: comment why we did not re-use code * Code review feedback: warn if running in checked mode * Restyle * Avoid loop of err/out empty output * Support a log directory argument for the casting tests, so I can debug their content * Better debuggability and error reporting support for shell - this is to debug cast failures --------- Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
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.
This implements reads via ember and via codegen as well as a checked-mode where reads are validated as returning the same status codes and data size (because data content is not available)
Changes
src/app/reporting
.chip_use_data_model_interface
moved tocommon_flags.gni
so that the flag is accessible across various BUILD.gn files.InteractionModelEngine
now maintains aDataModel
that can be set/replaced and which will be used as the basis for reads. NOTE: this does NOT yet implement init/shutdown as I have yet to build the relevant interfaces. Those interfaces are relevant for writes and invokes mainly (ability to provide current exchange and dirty paths)ReadChecked coverage is low because the "there are differences" codepaths are never hit.