WIP: Live 1792: Synchronized onboarding logic #214

alexandremgo · 2022-05-30T08:14:37Z

❓ Context

This PR is WIP (don't bother with all the console.log) but ready for comments and feedbacks👷

Impacted projects: LIVE-1792
Linked resource(s):

The main features are:

Function to extract the device onboarding state from byte flags
Polling mechanism to retrieve the device onboarding state
Make the polling mechanism available as a react hook for LLM and LLD
First implementation/POC of the synchronous onboarding in LLM:
- BLE pairing of not seeded device
- Screen to display the current onboarding state of the device using the above hook

🕵️‍♂️ Compared to the previous PR on the live-common and ledger-live-mobile, this feature fixed (parts, some are still WIP) comments from this PR and:

In polling mechanism: replace timer that would push for a request every period time, without waiting for the previous request -> now custom Observable is created with a repeat
- the new Observable way has been chosen, compared to just used rxjs operators etc., with a system to not delay for the first run (and then repeat with a delay of X ms) because repeat({ delay: (count) => count > 0 ? timer(periodMs) : timer(0) }) does not exist in rxjs v6
Refactor the retry strategy: before on error was a "black box" where the system was not notified of an error, but was just retrying -> now a allowedError and a fatalError is returned from the polling mechanism and the hook so the app (LLM and LLD) are notified when an error occurred

✅ Checklist

Test coverage: Did you write any tests to cover the changes introduced by this pull request?
- [x]: Unit tests for the extract function & the hook polling function
Atomic delivery: Is this pull request standalone? In order words, does it depend on nothing else?
No breaking changes: Does this pull request contain breaking changes of any kind? If so, please explain why.

📸 Demo

Will record a new demo video after on slack

🚀 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.

vercel · 2022-05-30T08:14:41Z

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

Name	Status	Preview	Updated
live-common-tools	✅ Ready (Inspect)	Visit Preview	Jun 9, 2022 at 3:34PM (UTC)

3 Ignored Deployments

Name	Status	Updated
ledger-live-github-bot	⬜️ Ignored (Inspect)	Jun 9, 2022 at 3:34PM (UTC)
native-ui-storybook	⬜️ Ignored (Inspect)	Jun 9, 2022 at 3:34PM (UTC)
react-ui-storybook	⬜️ Ignored (Inspect)	Jun 9, 2022 at 3:34PM (UTC)

changeset-bot · 2022-05-30T08:14:41Z

⚠️ No Changeset found

Latest commit: e75cc11

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

codecov · 2022-05-30T08:25:09Z

Codecov Report

Merging #214 (e75cc11) into develop (113be36) will increase coverage by 14.58%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           develop     #214       +/-   ##
============================================
+ Coverage    56.23%   70.81%   +14.58%     
============================================
  Files          471       63      -408     
  Lines        20945     4067    -16878     
  Branches      5322      762     -4560     
============================================
- Hits         11778     2880     -8898     
+ Misses        9115     1178     -7937     
+ Partials        52        9       -43

Flag	Coverage Δ
bot	`?`
test	`70.81% <ø> (?)`

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

Impacted Files	Coverage Δ
...-live-common/src/families/algorand/js-broadcast.ts
...-common/src/families/algorand/api/indexer.types.ts
...n/src/families/filecoin/deviceTransactionConfig.ts
...r-live-common/src/families/hedera/hw-getAddress.ts
...edger-live-common/src/families/hedera/bridge/js.ts
...r-live-common/src/families/solana/bridge/bridge.ts
libs/ledger-live-common/src/mock/account.ts
...live-common/src/families/polkadot/hw-getAddress.ts
.../ledger-live-common/src/families/solana/tx-fees.ts
...dger-live-common/src/families/solana/api/logged.ts
... and 524 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 113be36...e75cc11. Read the comment docs.

github-actions · 2022-05-30T08:34:15Z

@alexandremgo

Screenshots: ❌

It seems this PR contains screenshots that are different from the base branch.
If you are sure all those changes are correct, you can comment on this PR with /generate-screenshots to update those screenshots.

Make sure all the changes are correct before running the command, as it will commit and push the new result to the PR.

windows

Actual	Diff	Expected
market-btc-buy-page-actual	market-btc-buy-page-diff	market-btc-buy-page-expected

gre

really cool to see this happening 👏

gre · 2022-05-30T16:19:40Z

apps/ledger-live-mobile/src/screens/SyncOnboarding/index.tsx

+        setStepIndex(7);
+        break;
+      default:
+        setStepIndex(0);


there may be a way to simplify this with an array containing all the steps that you are currently switch()ing on.

const steps = [OnboardingStep.WelcomeScreen, ...] const stepIndex = steps.indexOf(onboardingState?.currentOnboardingStep) + 1

Yeah, definitely I'll try to make something simpler like this ! Right now for this screen it was to demo and show that the polling mechanism was working ;)

grsoares21

Looking good. Just a few comments on code clarity / style.

libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.ts

libs/ledger-live-common/src/hw/extractOnboardingState.ts

alexandremgo · 2022-05-31T09:12:10Z

FYI: i will move the onboardingStatePolling used by the hook to live-common/src/hw/getOnboardingStatePolling to keep the file containing the hook useOnboardingStatePolling.ts simple

- polling device status and retry strategy - typed navigator for sync onboarding

It throws if an error occurred

…or observable And follows rxjs naming convention with a "$" suffix for Observables

juan-cortes

lgtm, so many tests, cleanup the logs and such though

juan-cortes · 2022-06-09T15:34:13Z

apps/ledger-live-mobile/fastlane/Fastfile

@@ -109,7 +109,7 @@ platform :ios do

    build_number = latest_testflight_build_number(
      version: trim_version_number(package["version"]),
-      app_identifier: MY_APP_BUNDLE_ID
+      app_identifier: "com.ledger.live"


Why is this here?

it was a fixed to be able to build iOS that Valentin told me to try -> as this PR won't be merged, i did not put any comment on it. But Valentin merged this fix on develop yesterday ;)

juan-cortes · 2022-06-09T15:35:17Z

apps/ledger-live-mobile/src/screens/PairDevices/index.js

+            from(getVersion(t)),
+          ).toPromise();
+        } catch (_) {
+          // Silently swwallowing error, we only want to trigger the native ble pairing step


🙈 sorry forgot about this feedback - i'm thinking on working on the BLE part in a new PR as my next step (after merging stuff from live-common in a new/clean PR)

juan-cortes · 2022-06-09T15:35:38Z

apps/ledger-live-mobile/src/screens/PairDevices/index.js

-      route.params?.onDone?.(device);
+      // To avoid passing a onDone function param that is not serializable
+      if (route.params?.onDoneNavigateTo === ScreenName.SyncOnboardingWelcome) {
+        console.log("PairDevices: 🦮 navigate directly to SyncOnboarding");


juan-cortes · 2022-06-09T15:36:07Z

apps/ledger-live-mobile/src/screens/SyncOnboarding/index.tsx

+  // it does not matter.
+  // So as long as the device is not onboarded, the LLM will ask for a pairing.
+  useEffect(() => {
+    console.log("SyncOnboarding: navigate to pairDevices");


juan-cortes · 2022-06-09T15:36:47Z

apps/ledger-live-mobile/src/screens/SyncOnboarding/index.tsx

+        setStepIndex(7);
+        break;
+      default:
+        setStepIndex(0);


github-actions bot assigned alexandremgo May 30, 2022

github-actions bot added common Has changes in live-common ledgerjs Has changes in the ledgerjs open source libs mobile Has changes in LLM translations Translation files have been touched labels May 30, 2022

vercel bot deployed to Preview – live-common-tools May 30, 2022 15:17 View deployment

gre reviewed May 30, 2022

View reviewed changes

grsoares21 reviewed May 31, 2022

View reviewed changes

vercel bot deployed to Preview – live-common-tools May 31, 2022 08:34 View deployment

vercel bot deployed to Preview – live-common-tools May 31, 2022 16:00 View deployment

vercel bot deployed to Preview – live-common-tools June 6, 2022 08:12 View deployment

vercel bot deployed to Preview – live-common-tools June 8, 2022 16:12 View deployment

alexandremgo force-pushed the feat/LIVE-1792 branch from b256386 to 8154ab1 Compare June 9, 2022 07:57

github-actions bot added the cli label Jun 9, 2022

vercel bot deployed to Preview – live-common-tools June 9, 2022 08:00 View deployment

vercel bot deployed to Preview – live-common-tools June 9, 2022 08:25 View deployment

alexandremgo added 9 commits June 9, 2022 13:16

feat(llc/hw): extract onboarding state function

6651535

feat(llm): sync onboarding logic and screen

fd508af

- polling device status and retry strategy - typed navigator for sync onboarding

feat(llm): WIP select a nano for sync onboarding

05fc5f7

chore(llm): isolate deep linking for already onboarded devices

3f4306f

feat(llm): enable deep linking to onboarding steps

42c9d49

feat(llc): up-to-date onboarding state extract

3d36f57

feat(llc): extractOnboardingState does not return null

87a11f7

It throws if an error occurred

feat(llc): onboarding state polling logic in a hook

9955aaa

feat(llc): sync onboarding cli

dbb03c7

alexandremgo added 9 commits June 9, 2022 13:16

feat(llm): SyncOnboarding screen uses onboarding state polling hook

269e209

feat(llm): sync onboarding reacts to allowed and fatal errors

28440e3

chore(llc): enum types for seed phrasse and onboarding step

7e8adec

feat(llc): timeout on request to device during polling

b6c6dba

chore(llc): unit test for timeout feature

f453674

feat(llc): add being able to stop onboarding state polling

87ac892

refactor(llc): split observable into a firmwareInfo and an allowedErr…

36b3326

…or observable And follows rxjs naming convention with a "$" suffix for Observables

fix(llc): no any type for isAllowedOnboardingStatePollingError

a96aed9

refactor(llc): Boolean() instead of !!

8d40a79

alexandremgo force-pushed the feat/LIVE-1792 branch from cbd4501 to 9296d68 Compare June 9, 2022 11:17

vercel bot deployed to Preview – live-common-tools June 9, 2022 11:17 View deployment

vercel bot deployed to Preview – live-common-tools June 9, 2022 14:29 View deployment

alexandremgo added 5 commits June 9, 2022 17:18

feat(llc): getOnboardingStatePolling logic

d69e116

refactor(llc): useOnboardingStatePolling uses getOnboardingStatePolling

4aa4911

refactor(llc): SeedPhraseType in types/manager

ec989ad

chore(llm): for QA & demo

448911f

chore(llm): config app_identifier on Fastfile

e75cc11

alexandremgo force-pushed the feat/LIVE-1792 branch from 6de3923 to e75cc11 Compare June 9, 2022 15:32

vercel bot had a problem deploying to Preview – live-common-tools June 9, 2022 15:34 Failure

juan-cortes reviewed Jun 9, 2022

View reviewed changes

alexandremgo mentioned this pull request Jun 13, 2022

Synchronous onboarding on Mobile #330

Merged

3 tasks

alexandremgo closed this Jun 14, 2022

alexandremgo mentioned this pull request Jun 14, 2022

feat/FAT-13 Sync Onboarding logic #346

Merged

3 tasks

valpinkman deleted the feat/LIVE-1792 branch February 23, 2023 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Live 1792: Synchronized onboarding logic #214

WIP: Live 1792: Synchronized onboarding logic #214

alexandremgo commented May 30, 2022 •

edited

Loading

vercel bot commented May 30, 2022 •

edited

Loading

changeset-bot bot commented May 30, 2022 •

edited

Loading

codecov bot commented May 30, 2022 •

edited

Loading

github-actions bot commented May 30, 2022 •

edited

Loading

gre left a comment

gre May 30, 2022

alexandremgo May 31, 2022

juan-cortes Jun 9, 2022

grsoares21 left a comment

alexandremgo commented May 31, 2022

juan-cortes left a comment

juan-cortes Jun 9, 2022

alexandremgo Jun 10, 2022

juan-cortes Jun 9, 2022

alexandremgo Jun 10, 2022

juan-cortes Jun 9, 2022

juan-cortes Jun 9, 2022

juan-cortes Jun 9, 2022

WIP: Live 1792: Synchronized onboarding logic #214

WIP: Live 1792: Synchronized onboarding logic #214

Conversation

alexandremgo commented May 30, 2022 • edited Loading

❓ Context

✅ Checklist

📸 Demo

🚀 Expectations to reach

vercel bot commented May 30, 2022 • edited Loading

changeset-bot bot commented May 30, 2022 • edited Loading

⚠️ No Changeset found

codecov bot commented May 30, 2022 • edited Loading

Codecov Report

github-actions bot commented May 30, 2022 • edited Loading

gre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grsoares21 left a comment

Choose a reason for hiding this comment

alexandremgo commented May 31, 2022

juan-cortes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexandremgo commented May 30, 2022 •

edited

Loading

vercel bot commented May 30, 2022 •

edited

Loading

changeset-bot bot commented May 30, 2022 •

edited

Loading

codecov bot commented May 30, 2022 •

edited

Loading

github-actions bot commented May 30, 2022 •

edited

Loading