-
Notifications
You must be signed in to change notification settings - Fork 233
Move device enumeration to device specific drivers and load device drivers without explicit import #66
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
Conversation
d252fa4 to
46f92c2
Compare
instagibbs
left a comment
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.
minor commit history nit, looks pretty good.
7fcbb8d to
dd0bac8
Compare
dd0bac8 to
113da86
Compare
|
Ok Ledger stuff at least works now. limited tACK |
113da86 to
409ef39
Compare
Sjors
left a comment
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.
tACK 409ef39 for Trezor Model T on macOS. enumerate finds it, getmasterxpub, getxpub and signtx work. displayaddress and sign_message aren't implemented yet, so I didn't test those. I didn't test the password feature.
| return client | ||
|
|
||
| # Get a list of all available hardware wallets | ||
| def enumerate(args): |
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.
It would be nice if enumerate didn't ignore --device-type= as it's a bit slow.
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 like the suggestion but obviously should wait until this is merged
Instead of creating an HID device and passing that in, pass in the path. Also pass in the password for each of the clients so that the interface is consistent.
Instead of enumerating only HID devices globally, have each device specific section do its own device enumeration so that we can get devices which use other protocols without effecting global device enumeration code.
This will help later when doing dynamic object creation
enumerate() and getclient() now will get devices and client objects by calling the respective functions and constructors in the files in the hwilib/devices/ folder. It will do this automatically with any device specific drivers implemented there. So explicitly naming the devices and hardcoding their constructors in hwilib/commands.py is no longer necessary.
409ef39 to
e86669c
Compare
…d device drivers without explicit import e86669c Remove unnecessary device_ids.py (Andrew Chow) 64c1953 Enumerate devices and get client without hardcoding device modules (Andrew Chow) 15b8cbd Rename classes to fit pattern of first uppercase letter, then lowercase (Andrew Chow) ce2fd26 Move device enumeration to device specific code (Andrew Chow) c625e03 Unify device client constructor to path and password (Andrew Chow) 1c334c5 Move device specific things to devices sub folder (Andrew Chow) Pull request description: This PR moves device enumeration from the `enumerate()` function to `enumerate()` functions within each device driver. These device specific `enumerate()` functions allow us to enumerate those devices with whatever device specific enumeration required, instead of only enumerating with hid. This allows us to interact with devices connected through other methods such as udp (trezor emulator), webusb (trezor t), unix pipes (coldcard emulator), etc. but without requiring us to add that enumeration to the actual `enumerate()` function. Because there can be different types of device interfaces, the creation of the device is now handled by the device client class. The device path will be passed in and the client class uses that to know what type of device interface to create. Furthermore, device drivers have been moved to `hwilib/devices/` and both `getclient()` and `enumerate()` will call their respective functions and constructors without explicitly importing the device specific drivers. This allows us to easily add a new device driver by simply adding its implementation to the `hwilib/devices/` folder. In order to facilitate this automatic discovery and import of device drivers, the client class names and arguments have been normalized. The class name must be the device name (as will be specified in the `--device-type` parameter), begin with a capital letter, have the rest of the device name in lower case, and followed by `Client`. Classes must also take two positional arguments `path` and `password`, in that order. This change should enable the use of the Trezor T (so fixes #55) as well as device simulators and emulators so that automated tests can be done. Tree-SHA512: 057709291194ab2aa9bab138350970f8f6d410216ba5500300ef32c85c06cadc74a2996ed12140849a11f669d9ff01bfe7de4e32a2c7864b29e49606899dcf54
…ravis CI fd19704 Travis config (Andrew Chow) ced3087 Document tests and how to run them (Andrew Chow) ddcf842 extra for tests dependencies (Andrew Chow) 8aa0471 Add scripts to run tests and prepare test environments (Andrew Chow) 993b6e8 Update test_bech32.py shebang (Andrew Chow) a277ac2 Fix psbt serializations to always be consistent (Andrew Chow) d01cbae Add tests for Coldcard using Coldcard simulator (Andrew Chow) 2272428 Add tests for trezor using the emulator (Andrew Chow) ccd5f2a Use the DebugLink for the Trezor emulator in order to have tests (Andrew Chow) 64816cb Remove unneeded BIP 32 test (Andrew Chow) Pull request description: This PR is based on top of #66. It adds two test cases for the Trezor and Coldcard implementations by using their respective emulators/simulators. `test_bip32.py` was removed as the BIP 32 implementation was removed and no longer needed. Additionally a test running (`run_tests.py`), a script for building the Trezor emulator, Coldcard simulator, and bitcoind, and instructions for running the tests were all added. Lastly a Travis config has been added and travis builds for this branch are available at https://travis-ci.org/achow101/HWI Tree-SHA512: 71da24ee39bd845a822d48e83d4c6980cda741544e6cdf04d257db116bc1f7133b3370e235100a66b4fe7c69ecbd0298a4df7879733b75cebeb6a88467f4bf5f
This PR moves device enumeration from the
enumerate()function toenumerate()functions within each device driver. These device specificenumerate()functions allow us to enumerate those devices with whatever device specific enumeration required, instead of only enumerating with hid. This allows us to interact with devices connected through other methods such as udp (trezor emulator), webusb (trezor t), unix pipes (coldcard emulator), etc. but without requiring us to add that enumeration to the actualenumerate()function. Because there can be different types of device interfaces, the creation of the device is now handled by the device client class. The device path will be passed in and the client class uses that to know what type of device interface to create.Furthermore, device drivers have been moved to
hwilib/devices/and bothgetclient()andenumerate()will call their respective functions and constructors without explicitly importing the device specific drivers. This allows us to easily add a new device driver by simply adding its implementation to thehwilib/devices/folder. In order to facilitate this automatic discovery and import of device drivers, the client class names and arguments have been normalized. The class name must be the device name (as will be specified in the--device-typeparameter), begin with a capital letter, have the rest of the device name in lower case, and followed byClient. Classes must also take two positional argumentspathandpassword, in that order.This change should enable the use of the Trezor T (so fixes #55) as well as device simulators and emulators so that automated tests can be done.