-
Notifications
You must be signed in to change notification settings - Fork 4
Basic integration with the RDK Reference DAC 2.0 App Store #152
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
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.
Pull request overview
This PR implements basic integration with the RDK Reference DAC 2.0 App Store. It refactors the existing LISA-based implementation to use the new RDK Reference DAC 2.0 services (PackageManager, DownloadManager, and AppManager) for app management operations.
Changes:
- Replaces LISA API with DAC 2.0 services for app installation, uninstallation, and status tracking
- Adds new API classes for DownloadManager and PackageManager with improved error handling
- Simplifies the app catalog retrieval logic by removing platform-specific URL replacements
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| accelerator-home-ui/src/api/DACApi.js | Complete refactoring to use DAC 2.0 services instead of LISA API; adds download manager integration and operation locking |
| accelerator-home-ui/src/api/DownloadManagerApi.js | New API wrapper for DownloadManager Thunder plugin with download progress tracking |
| accelerator-home-ui/src/api/PackageManagerApi.js | Refactored to improve error handling and standardize Thunder API calls |
| accelerator-home-ui/src/api/ThunderError.js | New custom error class for handling Thunder API errors |
| accelerator-home-ui/src/items/AppCatalogItem.js | Updated to use new isDACAppInstalled API and simplified installation logic |
| accelerator-home-ui/src/items/AppStoreItem.js | Removed unused icon fetching imports and logic |
| accelerator-home-ui/src/items/ManageAppItem.js | Removed unused icon fetching imports and logic; added explicit isUnInstalling flag before uninstall call |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2d1ffcb to
adc357a
Compare
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 22 comments.
Comments suppressed due to low confidence (1)
accelerator-home-ui/src/items/ManageAppItem.js:137
- The uninstallDACApp return value is being misinterpreted. The function returns true on success and false on failure (lines 289-318 in DACApi.js), but this code treats the return value as "inProgress" and checks
if (!inProgress)to determine if there was a failure.
This logic is inverted - when uninstallDACApp returns false (failure), the code resets isUnInstalling and shows an error. When it returns true (success), the code does nothing and leaves isUnInstalling = true, waiting for the $fireDACOperationFinished callback.
The variable should be renamed to success for clarity:
const success = await uninstallDACApp(this._app, this.tag('StatusProgress'))
if (!success) {
Or the logic should be adjusted to properly reflect that this is checking for success/failure, not whether the operation is still in progress.
const inProgress = await uninstallDACApp(this._app, this.tag('StatusProgress'))
if (!inProgress) {
this._app.isUnInstalling = false;
this.tag("OverlayText").text.text = Language.translate("Status") + ':' + (this._app.errorCode ?? -1);
this.tag("Overlay").alpha = 0.7
this.tag("OverlayText").alpha = 1
this.tag("Overlay").setSmooth('alpha', 0, { duration: 5 })
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adc357a to
3ba68b9
Compare
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ref: #RDKEAPPRT-538
3ba68b9 to
b747714
Compare
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ref: #RDKEAPPRT-538