Skip to content

Conversation

jdabbech-ledger
Copy link
Contributor

@jdabbech-ledger jdabbech-ledger commented Jun 2, 2025

βœ… Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • ...

πŸ“ Description

Map device disconnected from DMK during CLS update

use of withTransport instead of withDevice to fix retry after device session removed

❓ Context


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

@jdabbech-ledger jdabbech-ledger requested a review from a team as a code owner June 2, 2025 10:18
Copy link

vercel bot commented Jun 2, 2025

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
ledger-live-github-bot ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2025 0:44am
native-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2025 0:44am
react-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2025 0:44am
web-tools ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2025 0:44am

@live-github-bot live-github-bot bot added the common Has changes in live-common label Jun 2, 2025
@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch from dd33de0 to 425ed34 Compare June 2, 2025 12:54
@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch from 425ed34 to 9fdd43a Compare June 5, 2025 13:07
@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch from 9fdd43a to 9db6fa1 Compare June 9, 2025 08:45
@jdabbech-ledger jdabbech-ledger requested a review from a team as a code owner June 9, 2025 08:45
@live-github-bot live-github-bot bot added desktop Has changes in LLD mobile Has changes in LLM translations Translation files have been touched labels Jun 9, 2025
@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch from 9db6fa1 to 8bf165e Compare June 9, 2025 08:45
@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch from 8bf165e to fe647a7 Compare June 10, 2025 14:43
@live-github-bot live-github-bot bot removed the desktop Has changes in LLD label Jun 10, 2025
@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch 3 times, most recently from 0adb632 to 746eb04 Compare June 18, 2025 16:00
Comment on lines 31 to 39
const translatedErrorStringId = `errors.${error._tag}.${field}`;
if (t(translatedErrorStringId) !== translatedErrorStringId) {
return <Text>{t(translatedErrorStringId)}</Text>;
} else {
const message =
field === "title"
? error._tag
: (error?.originalError as Error)?.message ?? error.message ?? error._tag;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const translatedErrorStringId = `errors.${error._tag}.${field}`;
if (t(translatedErrorStringId) !== translatedErrorStringId) {
return <Text>{t(translatedErrorStringId)}</Text>;
} else {
const message =
field === "title"
? error._tag
: (error?.originalError as Error)?.message ?? error.message ?? error._tag;
const translatedKey = `errors.${error._tag}.${field}`;
const translated = t(translatedKey);
if (translated !== translatedKey) {
return <Text>{translated}</Text>;
}
const fallbackMessage =
field === "title"
? error._tag
: (error?.originalError as Error)?.message ?? error.message ?? error._tag;
return <Text>{t(`errors.generic.${field}`, { message: fallbackMessage })}</Text>;

I think it has better readability and avoid repeated translation calls.

  • translatedKey and translated make the intent clearer.
  • We evaluate t(translatedKey) only once to avoid unnecessary function calls.

@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch from 746eb04 to 0be0f1a Compare June 19, 2025 09:00
Copy link

@Copilot Copilot AI left a 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 remaps the device disconnected error from DMK during a custom lock screen update and updates the associated error handling and device selection workflows. Key changes include:

  • Replacing withDevice with withTransport in customLockScreenLoad to address device session removal and updating error handling for DMK disconnect cases.
  • Updating tests in customLockScreenLoad.test to align with the new transport API.
  • Enhancing the device selection hook and UI in the mobile app by introducing selectDevice and registerDeviceSelection functions, along with updating related components and localization.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
libs/ledger-live-common/src/hw/customLockScreenLoad.ts Refactors device API usage by replacing withDevice with withTransport and adds DMK error type checking
libs/ledger-live-common/src/hw/customLockScreenLoad.test.ts Updates tests to support transportRef and DMK error scenarios
apps/ledger-live-mobile/src/newArch/features/DeviceSelection/screens/SelectDevice/useSelectDeviceViewModel.ts Adds a callback registration and updates device selection handling
apps/ledger-live-mobile/src/newArch/features/DeviceSelection/screens/SelectDevice/index.tsx Updates component props to use the new device selection API
apps/ledger-live-mobile/src/locales/en/common.json Extends localization for DMK errors
apps/ledger-live-mobile/src/components/TranslatedError/TranslatedError.tsx Adjusts error translation for DMK errors using new keys
.changeset/*.md Documents the changes with appropriate release notes
Comments suppressed due to low confidence (1)

apps/ledger-live-mobile/src/components/TranslatedError/TranslatedError.tsx:31

  • [nitpick] Consider adding a comment to explain the fallback behavior in error translation when a specific DMK error translation key is missing, so future developers understand the rationale behind calling the generic translation.
    const translatedKey = `errors.${error._tag}.${field}`;

Comment on lines 47 to 52
const isDmkDeviceDisconnectedError = (err: unknown): err is DeviceDisconnectedWhileSendingError =>
err !== null &&
(err instanceof DeviceDisconnectedWhileSendingError ||
(typeof err === "object" &&
"_tag" in err &&
err._tag === "DeviceDisconnectedWhileSendingError"));
Copy link
Preview

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

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

The type guard for DMK device disconnection error works as expected; consider ensuring that checking for 'err !== null' and property '_tag' is sufficient for all cases you expect, and document its intent for future maintainers.

Suggested change
const isDmkDeviceDisconnectedError = (err: unknown): err is DeviceDisconnectedWhileSendingError =>
err !== null &&
(err instanceof DeviceDisconnectedWhileSendingError ||
(typeof err === "object" &&
"_tag" in err &&
err._tag === "DeviceDisconnectedWhileSendingError"));
/**
* Type guard to check if the given error is a DeviceDisconnectedWhileSendingError.
* Ensures that the error is an object, is not null, and matches the expected structure.
* This is used to identify specific disconnection errors from the DMK device.
*/
const isDmkDeviceDisconnectedError = (err: unknown): err is DeviceDisconnectedWhileSendingError =>
typeof err === "object" &&
err !== null &&
(err instanceof DeviceDisconnectedWhileSendingError ||
("_tag" in err && err._tag === "DeviceDisconnectedWhileSendingError"));

Copilot uses AI. Check for mistakes.

aussedatlo
aussedatlo previously approved these changes Jun 19, 2025
@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch from 0be0f1a to 5a04fff Compare June 19, 2025 12:19
@live-github-bot live-github-bot bot added the desktop Has changes in LLD label Jun 19, 2025
use withTransport instead of withDevice to fix retry after device session removed
use withTransport instead of withDevice to fix retry after device session removed
@jdabbech-ledger jdabbech-ledger force-pushed the fix/live-18720-dmk-cls-disconnect-error branch from 5a04fff to 7b4329c Compare June 19, 2025 12:35
@live-github-bot live-github-bot bot removed the desktop Has changes in LLD label Jun 19, 2025
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
15.6% Coverage on New Code (required β‰₯ 80%)

See analysis details on SonarQube Cloud

@jdabbech-ledger jdabbech-ledger merged commit 9615906 into develop Jun 20, 2025
64 of 66 checks passed
@jdabbech-ledger jdabbech-ledger deleted the fix/live-18720-dmk-cls-disconnect-error branch June 20, 2025 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Has changes in live-common mobile Has changes in LLM translations Translation files have been touched
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants