Skip to content
This repository has been archived by the owner on Jun 16, 2022. It is now read-only.

LIVE-590 Remove device connection hold after timeout #4848

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

juan-cortes
Copy link
Contributor

🦒 Context (issues, jira)

https://ledgerhq.atlassian.net/browse/LIVE-590

💻 Description / Demo (image or video)

After literally two months of iterations and releases this is the first time the PR for testing this feature is actually pointing at released versions of the dependencies. The code for live-common v21.35.0 LedgerHQ/ledger-live-common#1735 made it to the last version and the changes at the transport level made it to v6.26.0 LedgerHQ/ledgerjs#797 (possible earlier, but they're definitely there)

Without this PR the functionality of the USB hold should remain unchanged. This PR introduces a locking/unlocking mechanism that is triggered when we start/end a withDevice code block. From my testing this allows us to not be able to connect while a long running task is happening such as an add account flow but as soon as the flow finishes +5s the hold is released and we no longer prevent users from connecting.

This was mostly targeting metamask users on mac in the first place.

🖤 Expectations to reach

PR must pass CI, rebase develop if conflicts. Thanks!

  • on QA: at least one of these two checkboxes must be checked:
    • a specific test planned is defined on Jira
    • this PR is covered by automatic UI test
  • on delivery: at least one of these two checkboxes must be checked:
    • Option 1: no impact: The changes of this PR have ZERO impact on the userland (invisible for users)
    • Option 2: atomic delivery: the changes is atomic and complete (no partial delivery)

How to test this (copied from #4764)

Same as the original PR, the context of this is still making sure that we don't keep control of the HID connection after we are done with the device. Just be sure that the failing tests (long add accounts on Cosmos, or a complex send) also work. The device should be connectable after 5 seconds when we are no longer interacting with it from https://repl.ledger.tools/ on chrome.

In particular, check what happens with flows if we leave the device idle for more than 5 seconds after the action is completed. I noticed the add accounts flow will throw an error when we disconnect even though we are no longer interacting with the device. Perhaps other flows have this same behavior and we'd need to adapt the UX to it (ie, if the action is done, ignore the error basically)

@juan-cortes juan-cortes requested review from a team as code owners March 17, 2022 11:34
@github-actions
Copy link

Thanks for your contribution.
To be groomed for next release, you need to:

  • pass the CI
  • if needed, run /generate-screenshots
  • have a dev review
  • have a QA review

Why /generate-screenshots ?

If your PR contains UI related changes,
it might be necessary to regenerate screenshots.

@github-actions
Copy link

Coverage report

St.
Category Percentage Covered / Total
🔴 Statements 8.74% 9/103
🔴 Branches 0% 0/19
🔴 Functions 2.94% 1/34
🔴 Lines 8.33% 8/96

Test suite run success

1 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from 531f623

Copy link
Contributor

@thomasrogerlux thomasrogerlux left a comment

Choose a reason for hiding this comment

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

Code looks good

@juan-cortes juan-cortes merged commit 19ff57a into develop Mar 24, 2022
@juan-cortes juan-cortes deleted the feat/LIVE-590 branch March 24, 2022 10:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants