Skip to content
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

fix: improved non token supported device handling #597

Merged
merged 7 commits into from
May 21, 2024
Merged

fix: improved non token supported device handling #597

merged 7 commits into from
May 21, 2024

Conversation

KazuCocoa
Copy link
Contributor

@KazuCocoa KazuCocoa commented May 20, 2024

  • Do not raise an error while no app found in driver.js for older TVs
  • Add isTokenSupportedDevice to check if the device supports token
  • Modify pair's log message
  • Prevent raising exception for non-token response by the device for old TVs (e.g. Year 2016 model)

@KazuCocoa KazuCocoa changed the title chore: end the pairing without token generation fix: improved non token supported device handling May 21, 2024
@KazuCocoa KazuCocoa requested a review from jlipps May 21, 2024 01:08
@KazuCocoa KazuCocoa marked this pull request as ready for review May 21, 2024 01:08
throw new Error(`Could not retrieve token; please try allowing the remote again`);
}

console.log('The device may not token supported device. Allowing the notification is sufficient.'); // eslint-disable-line no-console
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.log('The device may not token supported device. Allowing the notification is sufficient.'); // eslint-disable-line no-console
console.log('The device may not support token-based authentication. Allowing the pop-up notification is sufficient.'); // eslint-disable-line no-console

* @param {any} jsonBody
* @returns {'true'|'false'|undefined}
*/
_getTokenSupportDevice(jsonBody) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_getTokenSupportDevice(jsonBody) {
_getDeviceSupportsTokens(jsonBody) {

maybe?>

throw new Error(`Could not get token; server responded with: ${format('%O', res)}`);
}
this.#debug('The device may not support token as old model.');
return undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return undefined;
return;

@KazuCocoa KazuCocoa merged commit fe51c87 into main May 21, 2024
10 checks passed
@KazuCocoa KazuCocoa deleted the no-token branch May 21, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants