Feature/dial netflix bolt - #237
Open
yashaswini-rk wants to merge 5 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a dedicated DIAL/Xcast integration layer and refactors app lifecycle handling so the UI can register DIAL apps and respond to Xcast DIAL launch/resume/stop/state requests via AppController rather than bespoke logic embedded in App.js.
Changes:
- Add
DIALManagerto own Xcast activation, DIAL app registration, and Xcast event handling. - Extend
AppControllerwith lifecycle state tracking, listeners, and intent-based launch/send/close/terminate helpers. - Remove legacy Xcast listener/state-sync code from
App.js, removeXcastApi.supportedApps(), and update PackageManager callsign/metrics labeling.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| accelerator-home-ui/src/DIALManager.js | New manager for Xcast/DIAL activation, app registration, and DIAL event handling (launch/resume/stop/state). |
| accelerator-home-ui/src/AppController.js | Adds lifecycle state tracking + listener API and new launch/intent/close helpers used by DIALManager. |
| accelerator-home-ui/src/App.js | Switches from inlined Xcast wiring to DIALManager.get().start() and updates PackageManager plugin activation. |
| accelerator-home-ui/src/api/XcastApi.js | Removes supportedApps() helper (legacy mapping no longer used). |
| accelerator-home-ui/src/api/PackageManagerApi.js | Updates callsign to org.rdk.AppPackageManager and improves metrics/log labels. |
| accelerator-home-ui/src/api/AppManagerApi.js | Refactors to consistent async API with centralized ThunderError handling and clearer call wrappers. |
Suppressed comments (5)
accelerator-home-ui/src/DIALManager.js:156
- The ThunderError context string inside the setEnabled(true) catch block incorrectly references setStandbyBehavior("active"), which will mislead debugging/metrics.
await this.xcastApi.setEnabled(true).then(() => {
GLOBALS.LocalDeviceDiscoveryStatus = true;
}).catch(err => {
GLOBALS.LocalDeviceDiscoveryStatus = false;
const terr = new ThunderError(`xcastApi.setStandbyBehavior("active")`, err);
this.ERR(`DIALManager.start() ${terr}`);
});
accelerator-home-ui/src/DIALManager.js:184
- The ThunderError label for registerApplications has an extra ")" ("registerApplications())"), making logs harder to search and correlate.
await this.xcastApi.registerApplications(params).catch(err => {
const terr = new ThunderError(`xcastApi.registerApplications())`, err);
this.ERR(`DIALManager.start(): ${terr}`);
accelerator-home-ui/src/DIALManager.js:104
- AppController.getAppLifecycleState() can return undefined (Map#get) but setApplicationState() treats anything outside the known states as an "invalid state". This will happen for onApplicationStateRequest before any lifecycle event is observed, spamming errors and potentially reporting a wrong state. Handle undefined/null explicitly as "stopped" (or another default) without logging an invalid-state error.
switch (appControllerState) {
case 'APP_STATE_INITIALIZING':
break;
case 'APP_STATE_ACTIVE':
state = 'running';
break;
case 'APP_STATE_PAUSED':
case 'APP_STATE_SUSPENDED':
case 'APP_STATE_HIBERNATED':
state = 'suspended';
break;
case 'APP_STATE_TERMINATING':
break;
default:
this.ERR(`Received invalid state: ${appControllerState} for ${applicationName}`);
break;
}
accelerator-home-ui/src/AppController.js:119
- JSDoc says getAppLifecycleState() always returns AppState, but Map#get returns undefined when the id hasn’t been observed yet. This mismatch makes callers think the value is always defined.
/**
* @param {string} id
* @returns {AppState}
*/
getAppLifecycleState(id) {
return this.appLifecycleStates.get(id);
}
accelerator-home-ui/src/AppController.js:315
- AppController.launch() is called elsewhere with only the app id (no intent), and AppManager.launchApp already treats intent as optional. The JSDoc currently marks intent as a required string, which is misleading for callers.
/**
* @param {string} id
* @param {string} intent
* @returns {Promise<any>}
* @throws {ThunderError}
*/
async launch(id, intent) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+218
to
+221
| const pairingCode = notification.strPayLoad; | ||
| const additionalDataUrl = notification.strAddDataUrl; | ||
| const params = `&inApp=${this.appController.isLaunched(desc.id)}&launch=dial`; | ||
| const url = `${desc.url}?${pairingCode}&additionalDataUrl=${additionalDataUrl}${params}`; |
Comment on lines
+35
to
+39
| "Netflix": { | ||
| id: "com.rdkcentral.Netflix", | ||
| url: "", | ||
| cors: ".netflix.com", | ||
| } |
Comment on lines
+78
to
+83
| /** | ||
| * @param {string} id | ||
| * @returns {Promise<any>} | ||
| * @throws {ThunderError} | ||
| */ | ||
| async isPackageInstalled(id) { |
Comment on lines
+95
to
+100
| /** | ||
| * @param {string} appId | ||
| * @param {string} intent | ||
| * @returns {Promise<any>} | ||
| * @throws {ThunderError} | ||
| */ |
Comment on lines
+126
to
+137
| async start() { | ||
| if (this.started) { | ||
| throw new Error("DIALManager already started!"); | ||
| } | ||
|
|
||
| this.started = true; | ||
|
|
||
| if (!await this.xcastApi.activate()) { | ||
| throw new Error("XcastApi activatation failed"); | ||
| } | ||
|
|
||
| this.registerXcastListeners(); |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| this.WARN = debug.bind(null, console.warn); | ||
|
|
||
| if (DEBUG) { | ||
| this.INFO = debug.bind(null, console.info); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (5)
accelerator-home-ui/src/DIALManager.js:135
startedis set to true beforexcastApi.activate()succeeds; if activation throws/returns false, the manager stays in a permanently-started state and cannot be retried. Also fix the typo in the activation error message.
this.started = true;
if (!await this.xcastApi.activate()) {
throw new Error("XcastApi activatation failed");
}
accelerator-home-ui/src/DIALManager.js:222
desc.urlcan be empty (e.g. Netflix is configured with an empty string), which produces an invalid URL like?…and will likely fail to launch. AlsoadditionalDataUrlshould be URL-encoded to avoid breaking the query string.
const pairingCode = notification.strPayLoad;
const additionalDataUrl = notification.strAddDataUrl;
const params = `&inApp=${this.appController.isLaunched(desc.id)}&launch=dial`;
const url = `${desc.url}?${pairingCode}&additionalDataUrl=${additionalDataUrl}${params}`;
accelerator-home-ui/src/AppController.js:82
- The JSDoc for
isPackageInstalled()is inaccurate: it returns a boolean (wrapped in a Promise) and it swallows/handles errors instead of throwingThunderError. Keeping the docs aligned avoids incorrect assumptions by callers.
/**
* @param {string} id
* @returns {Promise<any>}
* @throws {ThunderError}
*/
accelerator-home-ui/src/DIALManager.js:154
- The ThunderError context string in the
setEnabled(true)error path incorrectly referencessetStandbyBehavior("active"), which makes troubleshooting misleading.
const terr = new ThunderError(`xcastApi.setStandbyBehavior("active")`, err);
this.ERR(`DIALManager.start() ${terr}`);
accelerator-home-ui/src/AppController.js:119
getAppLifecycleState()can returnundefineduntil the first lifecycle notification arrives, but the JSDoc claims it always returns anAppState. Returning a default avoids downstream "invalid state" logs and makes the return type accurate.
getAppLifecycleState(id) {
return this.appLifecycleStates.get(id);
}
This file contains hidden or 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
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.
No description provided.