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

Feat: FAT-43-100: new BLE pairing flow #1097

Merged
merged 13 commits into from
Aug 31, 2022

Conversation

alexandremgo
Copy link
Contributor

@alexandremgo alexandremgo commented Aug 30, 2022

📝 Description

Not yet user-facing. Only available from the debug menu.

Features:

  • scanning and pairing: one screen to go to from anywhere
  • navigate to after pairing success: configuration of the screen (and its associated navigator) with params and name of the route param that will have newly paired device info
  • scanning: filtering on device models
  • scanning: filtering out or displaying already known devices
  • pairing: new animation for pairing (lotties placeholders for now)
  • pairing: possibility to add (or not) the newly paired device to the "known devices" of the app (redux store)

Why adding the newly paired device info in the route params, and not adding a new pairedDevice in the redux store ?

  • Because we don't know when to clear this pairedDevice, and don't want to create a complex logic to do it.
  • Passing some info on the newly paired device as route params is easier to manage.

❓ Context

  • Impacted projects: LLM and live-common
  • Linked resource(s): ``

✅ Checklist

  • Test coverage: tested by QA
  • Atomic delivery
  • No breaking changes: updating the animation for Device Action

📸 Demo

🚀 Expectations to reach

Please make sure you follow these Important Steps.

Pull Requests must pass the CI and be internally validated in order to be merged.

@alexandremgo alexandremgo requested a review from a team as a code owner August 30, 2022 09:50
@vercel
Copy link

vercel bot commented Aug 30, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
live-common-tools ✅ Ready (Inspect) Visit Preview Aug 31, 2022 at 5:14PM (UTC)
3 Ignored Deployments
Name Status Preview Updated
ledger-live-github-bot ⬜️ Ignored (Inspect) Aug 31, 2022 at 5:14PM (UTC)
native-ui-storybook ⬜️ Ignored (Inspect) Aug 31, 2022 at 5:14PM (UTC)
react-ui-storybook ⬜️ Ignored (Inspect) Aug 31, 2022 at 5:14PM (UTC)

@changeset-bot
Copy link

changeset-bot bot commented Aug 30, 2022

🦋 Changeset detected

Latest commit: a98f1dd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
live-mobile Patch
@ledgerhq/live-common Patch
ledger-live-desktop Patch
@ledgerhq/live-cli Patch
live-common-tools Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added common Has changes in live-common mobile Has changes in LLM translations Translation files have been touched labels Aug 30, 2022
@github-actions
Copy link

github-actions bot commented Aug 30, 2022

@alexandremgo

Screenshots: ✅

There are no changes in the screenshots for this PR. If this is expected, you are good to go.

@codecov
Copy link

codecov bot commented Aug 31, 2022

Codecov Report

Merging #1097 (a98f1dd) into release (cb03728) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           release    #1097      +/-   ##
===========================================
+ Coverage    48.00%   48.01%   +0.01%     
===========================================
  Files          674      674              
  Lines        30016    30022       +6     
  Branches      7813     7815       +2     
===========================================
+ Hits         14410    14416       +6     
  Misses       15547    15547              
  Partials        59       59              
Flag Coverage Δ
test 48.01% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...r-live-common/src/ble/hooks/useBleDevicePairing.ts 100.00% <100.00%> (ø)
...live-common/src/ble/hooks/useBleDevicesScanning.ts 90.00% <100.00%> (+0.86%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@grsoares21 grsoares21 left a comment

Choose a reason for hiding this comment

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

Still haven't finished the review, will continue later

Copy link
Contributor

@grsoares21 grsoares21 left a comment

Choose a reason for hiding this comment

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

LGTM! Nice PR

@github-actions github-actions bot added desktop Has changes in LLD ledgerjs Has changes in the ledgerjs open source libs screenshots Screenshots have been updated tools Has changes in tools labels Aug 31, 2022
@alexandremgo alexandremgo force-pushed the feat/FAT-43-100-newBlePairingFlow branch from 04b17cd to 53c67f5 Compare August 31, 2022 16:32
@github-actions github-actions bot removed cli ledgerjs Has changes in the ledgerjs open source libs tools Has changes in tools automation CI/CD stuff labels Aug 31, 2022
thomasrogerlux and others added 13 commits August 31, 2022 19:00
* chore: generate changeset

* update screenshots (ubuntu-latest)

* update screenshots (windows-latest)

* update screenshots (macos-latest)

* fix(screenshots): make send modal wait for button to stop loading

* update screenshots (ubuntu-latest)

* update screenshots (windows-latest)

* update screenshots (macos-latest)

* chore: trigger CI

Co-authored-by: Team Live <[email protected]>
- consistent prop name filterByDeviceModelIds
- filter out devices by device ids during BLE scanning
- new ble pairing flow accessible from the debug menu
- scanning filtering on device models and on already known devices
- option to add paired device to known devices list
@alexandremgo alexandremgo force-pushed the feat/FAT-43-100-newBlePairingFlow branch from 53c67f5 to a98f1dd Compare August 31, 2022 17:09
@alexandremgo alexandremgo merged commit 936b6dc into release Aug 31, 2022
@alexandremgo alexandremgo deleted the feat/FAT-43-100-newBlePairingFlow branch August 31, 2022 17:40
@alexandremgo alexandremgo restored the feat/FAT-43-100-newBlePairingFlow branch August 31, 2022 17:41
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 desktop Has changes in LLD mobile Has changes in LLM screenshots Screenshots have been updated translations Translation files have been touched
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants