-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add setup_device function #55
Conversation
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.
Thank you so much for your contribution! At the moment the CI is not passing, as there are some little issues in the code. Have you tried running the tests with a device? I'm trying to test with my trezor emulator, but with no luck...
src/lib.rs
Outdated
let devices = HWIClient::enumerate().unwrap(); | ||
let unsupported = ["ledger", "coldcard", "jade"]; | ||
for device in devices { | ||
let device = device.unwrap(); |
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.
I'm testing with a non initialized trezor emulator, and this unwrap actually panics: hwi enumerate
first result is, in fact, an error:
[{"type": "trezor", "path": "udp:127.0.0.1:21324", "label": null, "model": "trezor_t_simulator", "needs_pin_sent": false, "needs_passphrase_sent": false, "error": "Not initialized", "code": -18}]
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.
Ok, we need find_device function instead of enumerate for not initialized devices. I'll open a new PR to add find_device function.
I did some tests and I saw that the setup_device command does not work on emulators but only on physical devices (at least my emulator returns the error "PIN requested but no sequence was configured").
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.
Ok, we need find_device function instead of enumerate for not initialized devices. I'll open a new PR to add find_device function.
This would be great, thanks!
I did some tests and I saw that the setup_device command does not work on emulators but only on physical devices (at least my emulator returns the error "PIN requested but no sequence was configured").
I'm hitting the same error, I just opened bitcoin-core/HWI#646 :)
The code looks good, can you please squash your commits together? Also, were you able to test this with a physical trezor device? |
Yes, I tested on a physical device (Trezor model T) and works. |
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.
So cool!
ACK 9cc3d50
No description provided.