-
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
Update TV app to use the new IM changes #7906
Update TV app to use the new IM changes #7906
Conversation
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.
General architectural comment: We should have the "everyone has to do this" parts of cluster implementations in src/app/clusters
and make callouts to apps for logic the app needs to provide. We should not be requiring everyone who wants to build a tv app to reinvent a bunch of wheels like encoding the responses and whatnot that are really shared code.
examples/all-clusters-app/all-clusters-common/all-clusters-app.zap
Outdated
Show resolved
Hide resolved
examples/tv-app/linux/include/application-launcher/ApplicationLauncherManager.cpp
Outdated
Show resolved
Hide resolved
examples/tv-app/linux/include/application-launcher/ApplicationLauncherManager.cpp
Outdated
Show resolved
Hide resolved
examples/tv-app/linux/include/keypad-input/KeypadInputManager.cpp
Outdated
Show resolved
Hide resolved
examples/tv-app/linux/include/media-playback/MediaPlaybackManager.cpp
Outdated
Show resolved
Hide resolved
03ab61b
to
6da8656
Compare
e42ea9a
to
89a30df
Compare
examples/tv-app/linux/include/media-playback/MediaPlaybackManager.cpp
Outdated
Show resolved
Hide resolved
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.
Not a full review; since there are unaddressed changes from last time (e.g. about pointers into a stack frame that goes away) I wasn't sure whether one was desired yet.
src/app/zap-templates/zcl/data-model/chip/application-basic-cluster.xml
Outdated
Show resolved
Hide resolved
src/app/zap-templates/zcl/data-model/chip/application-launcher-cluster.xml
Outdated
Show resolved
Hide resolved
src/app/zap-templates/zcl/data-model/chip/media-playback-cluster.xml
Outdated
Show resolved
Hide resolved
src/app/zap-templates/zcl/data-model/chip/application-launcher-cluster.xml
Outdated
Show resolved
Hide resolved
src/app/zap-templates/zcl/data-model/chip/target-navigator-cluster.xml
Outdated
Show resolved
Hide resolved
d8b6202
to
6fb856f
Compare
d9be430
to
3709f75
Compare
3709f75
to
fa8fa8e
Compare
@bzbarsky-apple Seems to me that the Darwin build failing has nothing to do with this PR.
For example this line |
2ac636c
to
17246f8
Compare
d9f41c0
to
f556c0c
Compare
f556c0c
to
72c603d
Compare
7bf97c2
to
7c7b2af
Compare
Problem - After the IM changes TV app was not returning correct responses Summary of Changes - Updated all the TV clusters to return response using new TLVWriter
Problem - All cluster app does not have zap regenerated files Summary of Changes - Run zap_regen_all.py script
Problem - TV app is not covered with tests Summary of Changes - Added yaml files for all clusters that needs to be tested
Problem - Tests does not have zap regenerated files Summary of Changes - Run zap_regen_all.py script
7c7b2af
to
81f9195
Compare
Resolved conflicts. |
Size increase report for "esp32-example-build" from d1e04c6
Full report output
|
* Fix IM changes Problem - After the IM changes TV app was not returning correct responses Summary of Changes - Updated all the TV clusters to return response using new TLVWriter * Running zap generator Problem - All cluster app does not have zap regenerated files Summary of Changes - Run zap_regen_all.py script * Adding tests for TV app Problem - TV app is not covered with tests Summary of Changes - Added yaml files for all clusters that needs to be tested * Running zap generator Problem - Tests does not have zap regenerated files Summary of Changes - Run zap_regen_all.py script * Adding TV callbacks to all cluster example apps * Updated the PR latests comments * Update PR per latest comments * Updated the PR per latest comments
* Fix IM changes Problem - After the IM changes TV app was not returning correct responses Summary of Changes - Updated all the TV clusters to return response using new TLVWriter * Running zap generator Problem - All cluster app does not have zap regenerated files Summary of Changes - Run zap_regen_all.py script * Adding tests for TV app Problem - TV app is not covered with tests Summary of Changes - Added yaml files for all clusters that needs to be tested * Running zap generator Problem - Tests does not have zap regenerated files Summary of Changes - Run zap_regen_all.py script * Adding TV callbacks to all cluster example apps * Updated the PR latests comments * Update PR per latest comments * Updated the PR per latest comments
Problem
TV example app was not using the IM changes introduced by this commit:
#5945
Change overview
tv.zap
and enabled TV clusters- Updated theall-clusters.zap
and disabled TV clusterstest_tv_suites.sh
script for testign*.yaml
file for every TV cluster.Testing
How was this tested? (at least one bullet point required)
./gn_build.sh
to verify the building is successful- Run the./scripts/tests/test_tv_suites.sh
./scripts/tests/test_suites.sh -a tv