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

Removes call to reset after call to open #609

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

humanumbrella
Copy link
Contributor

An attempt to fix issue #607

An attempt to fix Chrome 91
Copy link
Contributor

@gre gre left a comment

Choose a reason for hiding this comment

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

Thanks for this iteration. We started the same exploration yesterday on our end and we are validating the solution today.

I was wondering why you kept it in close()?
I think we'll have to kill the reset usage completely.

@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #609 (1138252) into master (e6d6f2a) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #609      +/-   ##
==========================================
+ Coverage   40.76%   40.77%   +0.01%     
==========================================
  Files          70       70              
  Lines        4006     4005       -1     
  Branches      707      707              
==========================================
  Hits         1633     1633              
+ Misses       2121     2120       -1     
  Partials      252      252              
Impacted Files Coverage Δ
...ackages/hw-transport-webusb/src/TransportWebUSB.js 0.00% <ø> (ø)

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 e6d6f2a...1138252. Read the comment docs.

Copy link
Contributor

@gre gre left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.
We did assess that it indeed is fixing the problem so it can be merged.
we are still trying to understand if we also will need to keep it for close(). for now we keep it and we will test more in depth with the next release.

@gre gre merged commit 3d2e728 into LedgerHQ:master Jun 3, 2021
gre added a commit that referenced this pull request Jun 3, 2021
We have identified the fix can cause more issues than it solves in some circumstances. It fixes the problem in the nominal scenario but there are cases where it actually makes the hardware device not "re open() able".

We are still working on finding better solutions but reverting this work is a safer move to avoid upgrading to a non-complete fix for this.

This reverts commit 3d2e728.
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.

2 participants