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

prefix search for trezorctl -p #226

Merged
merged 13 commits into from
Mar 6, 2018
Merged

prefix search for trezorctl -p #226

merged 13 commits into from
Mar 6, 2018

Conversation

matejcik
Copy link
Contributor

@matejcik matejcik commented Mar 2, 2018

The new feature:
trezorctl -p hid will pick the first HID device, and trezorctl -p bridge:w will pick the first WebUSB device on bridge. It's still possible to specify a full path and that will be tried directly before prefix-searching (so it's possible to specify UDP addresses that are not the default one).

There's also a refactor of transports.

While the prefix search for paths is desirable, the refactor is not necessary, I just took the opportunity to clean up a little.

The main thing is removing device.py because it's very much useless. The functionality is now moved to a pair of functions in trezorlib.transport. This is an API change and will need to be fixed in users... which is why i'm sort of hesitant about it. OTOH, the original API doesn't make sense.

I'll probably add a backwards compatibility wrapper that shouts DeprecationWarnings.

(it's also easy to write backwards compatible code:

try:
    from trezorlib.transport import enumerate_devices
except ImportError:
    from trezorlib.device import TrezorDevice
    enumerate_devices = TrezorDevice.enumerate

)

Transports now have their own submodule, which seems cleaner but isn't really necessary either.

For UDP transport, it's useful to be able to specify a path that should be tried directly,
without enumerating first.
first, try exact path
second, try prefix search
last, fail :) with reporting used path (for people like me who forget to unset TREZOR_PATH
@matejcik
Copy link
Contributor Author

matejcik commented Mar 2, 2018

posting this now to solicit comments. It's still necessary to update most tools to use the new API, and do the DeprecationWarning wrapper.

@prusnak
Copy link
Member

prusnak commented Mar 3, 2018

@slush0 would you like to have a look?

@saleemrashid
Copy link
Contributor

Since you're changing the transports API, could you also update tools/*.py? As it is, they don't work with T1 >= 1.7.0 or T2.

@prusnak prusnak added this to the v0.9.1 milestone Mar 5, 2018
also drop some python2 compatibility things
…ne if no trezor is found,

because the IndexError should not be part of the traceback
@matejcik
Copy link
Contributor Author

matejcik commented Mar 5, 2018

@saleemrashid does this solve your problem?

also added the promised TrezorDevice deprecation wrapper.

@matejcik matejcik removed this from the v0.9.1 milestone Mar 5, 2018
this fixes issue #223 on Windows, where a device would be returned in two copies, only one of which works
@prusnak
Copy link
Member

prusnak commented Mar 6, 2018

LGTM

@matejcik matejcik merged commit e1e4194 into trezor:master Mar 6, 2018
@matejcik matejcik deleted the refactor-transport-nicediff branch March 6, 2018 14:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants