Re-Implement App Install flow #34141
Open
+879
−122
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
andy31415,
andyg-apple,
anush-apple,
arkq,
bauerschwan,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
fessehaeve,
harimau-qirex and
harsha-rajendran
|
PR #34141: Size comparison from d033f8b to 1d3a69f Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34141: Size comparison from e7a153f to 5c4c4df Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
chrisdecenzo
approved these changes
Jul 1, 2024
...platform-app/src/main/java/com/matter/tv/server/handlers/ApplicationLauncherManagerImpl.java
Outdated
Show resolved
Hide resolved
...platform-app/src/main/java/com/matter/tv/server/handlers/ApplicationLauncherManagerImpl.java
Outdated
Show resolved
Hide resolved
| if (!matterAppEnabledIsInstalled && appIsInstalled) { | ||
| Log.i( | ||
| TAG, | ||
| "Matter enabled app is not installed, but app is installed. Launching app's install page"); |
There was a problem hiding this comment.
When would the app be installed but not matter enabled? And if that's the case, when would the app's install page help? Is this meant to address the situation where an older non-Matter-enabled version is installed but a newer Matter-enabled version is available?
There was a problem hiding this comment.
- When would the app be installed but not matter enabled?
- And if that's the case, when would the app's install page help?
- It can happen that app running on TV is obsolete and requires update after which it should support Matter. This logic aims to catch that use-case and to prompt user with app's detail page, where they can update the app.
- Is this meant to address the situation where an older non-Matter-enabled version is installed but a newer Matter-enabled version is available?
- Yes
examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter
Outdated
Show resolved
Hide resolved
|
PR #34141: Size comparison from e7a153f to d972065 Increases above 0.2%:
Full report (20 builds for cc13x4_26x4, cc32xx, mbed, nrfconnect, nxp, qpg, stm32, tizen)
|
|
PR #34141: Size comparison from 005f1b4 to 1e8b685 Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34141: Size comparison from 005f1b4 to 731d8f8 Full report (4 builds for cc32xx, mbed, stm32)
|
|
PR #34141: Size comparison from 71d9f61 to c1401ee Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34141: Size comparison from 4ef104b to 36eaa5f Full report (83 builds for bl602, bl702, bl702l, cc13x4_26x4, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34141: Size comparison from e5acdc9 to 1a8f583 Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
bzbarsky-apple
requested changes
Jul 3, 2024
Comment on lines
+83
to
+85
| <item name="PendingUserApproval" value="0x03"/> | ||
| <item name="Downloading" value="0x04"/> | ||
| <item name="Installing" value="0x05"/> |
There was a problem hiding this comment.
I don't see these in the spec anywhere. Where is this coming from?
There was a problem hiding this comment.
@bzbarsky-apple Chris (@chrisdecenzo) will file a CCB next week July 9th.
There was a problem hiding this comment.
The right order, generally, is to either change the spec or at least have a spec PR up, then point to that changed spec or pending PR in the SDK PR. Otherwise it's absolutely impossible to review the SDK change.
There was a problem hiding this comment.
Makes sense. Here's the spec PR https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/10014
|
PR #34141: Size comparison from 70744ed to 2a827a1 Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34141: Size comparison from 35eba86 to 3960e0e Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
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.
Re-Implementing application installation flow. Continuation of #33959
Problem
Current application installation flow blocks commissioning and has too crude functionality that cannot provide great UX (providing installation percentage etc.)
Change overview
Tests - Android - App Install & Get Catalog List
Build the Android TV app using command:
scripts/examples/gn_build_example.sh examples/tv-app/linux out/debug/Build the TV casting app using command:
scripts/examples/gn_build_example.sh examples/tv-casting-app/linux out/debug/Start TV Android App
Start TV Casting App
out/debug/chip-tv-casting-appSend Application Launcher command
cast cluster applicationlauncher launch-app 0 1 --Application {"catalogVendorID":123,"applicationID":"com.example.contentapp"}You should receive pending state:
{ status: 3, data: "" }Start the app install of content app and send application launcher command again, where you should receive installing state:
{ status: 4, data: "" }In case app install failed and you send application launcher command, you should then receive the state:
{ status: 3, data: 41707020696E7374616C6C206661696C65642E2054727920616761696E }The byte value stands for: "App install failed. Try again"
Finally you can test reading of catalog list by sending Application Launcher command
cast cluster applicationlauncher read catalog-list 0 1it results in: