-
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
Support: FAT-597 quick fix for 0x5515 error #1741
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 30ca4d8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 32 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: 87.12% // Head: 46.97% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1741 +/- ##
============================================
- Coverage 87.12% 46.97% -40.15%
============================================
Files 12 690 +678
Lines 233 30462 +30229
Branches 45 8044 +7999
============================================
+ Hits 203 14311 +14108
- Misses 30 14920 +14890
- Partials 0 1231 +1231
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. |
c868381
to
90ff9cb
Compare
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.
lgtm
apps/ledger-live-desktop/src/renderer/components/DeviceAction/rendering.tsx
Show resolved
Hide resolved
50e8428
to
19814f0
Compare
19814f0
to
c6e4512
Compare
1b7922f
to
c461fcf
Compare
c461fcf
to
d73c510
Compare
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.
lgtm
d73c510
to
3e2d4a5
Compare
3e2d4a5
to
752d0a4
Compare
752d0a4
to
1aec02e
Compare
1aec02e
to
30ca4d8
Compare
📝 Description
A quick fix for all the
0x5515
locked device cases.Needed because the kind of fix made in this PR will take longer to implement everywhere, and do not work currently on LLD as is.
Edit: i actually had to modify a bit more stuff for 2 reasons:
1. Not all
0x5515
error are thrown during a device actionit turns out there are a lot of scenarios where we could have a
0x5515
error outside the usage of a device action. So the remapping insiderenderError
in both LLD and LLM would not catch those.To handle those scenarios: i remapped
TransportStatusError
+status-code = 0x5515
into a new errorLockedDeviceError
.I kept the specific rendering inside the device action
renderLockedDeviceError
so we have a nicer UI for the user.But outside a device action the component
TranslatedError
for both LLM and LLD will display an error, but with an informative message that is associated to the errorLockedDeviceError
Because of this mapping to
LockedDeviceError
, i had to modify a couple of other files that had conditions onStatusCodes.LOCKED_DEVICE
:libs/ledger-live-common/src/hw/connectApp.ts
libs/ledger-live-common/src/hw/connectManager.ts
libs/ledger-live-common/src/hw/getDeviceRunningMode.ts
2. isLocked status in LLD
Because since this PR we now handle
LockedDeviceError
for connectApp and connectManager, which update the status ofisLocked
, we need to also handle the cases wereisLocked
is set during a device action in LLD❓ Context
✅ Checklist
📸 Demo
📱 LLM
LLM inside a device action
LLM outside a device action
💻 LLD
LLD inside a device action
LLD outside a device action
🚀 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.