-
Notifications
You must be signed in to change notification settings - Fork 360
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
bugfix/FAT-588 Improve LLM background device communication #1871
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 3718f98 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
Codecov ReportBase: 0.00% // Head: 42.85% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1871 +/- ##
============================================
+ Coverage 0 42.85% +42.85%
============================================
Files 0 624 +624
Lines 0 25895 +25895
Branches 0 7159 +7159
============================================
+ Hits 0 11097 +11097
- Misses 0 13640 +13640
- Partials 0 1158 +1158
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Screenshots: ✅
There are no changes in the screenshots for this PR. If this is expected, you are good to go. |
5223475
to
f64cd28
Compare
f64cd28
to
d0b9100
Compare
d0b9100
to
dbf4724
Compare
libs/ledgerjs/packages/react-native-hw-transport-ble/src/BleTransport.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 - just maybe a comment on the delay to explain why
dbf4724
to
7043863
Compare
34a1ce9
to
9d43aa8
Compare
9d43aa8
to
3718f98
Compare
📝 Description
This PR introduces a fix on the runners that allows the internal errors that can occur while installing an app to bubble up and reach the surface device action instead of being treated as false completion positives.
It also introduces a forceful disconnection timeout when we try to close the connection with a device on the BleTransport, preventing endless awaits if the JavaScript thread is paused in the middle of an exchange. This unlocks cases where we were left in a soft brick state where we couldn't communicate with the device anymore until we rebooted or restarted LLM.
Lastly, it enables (like on BIM) background BLE communication capabilities, allowing us to communicate with the device over BLE for some time making the UX a bit better than abruptly breaking.
❓ Context
ledger-live-mobile, react-native-hw-transport-ble, ledger-live-common
https://ledgerhq.atlassian.net/browse/FAT-588
✅ Checklist
🚀 Expectations to reach
We shouldn't get stuck in a flow regardless of a failure to communicate or complete the flow because of a background or disconnect.