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
Improve usb device selector output #680
Conversation
3884a00
to
aeb7f91
Compare
src/cmd/usb-util.js
Outdated
const id = await _getDeviceId(d); | ||
const mode = await _getDeviceMode(d); |
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 might be good if we open the device once and get the mode and the id in one step?
Also that will allow us to have a parallel
behavior here:
[{id, mode}, name] = Promise.all[_getDeviceInfo(d), _getDeviceName({...})]
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.
Good suggestion
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.
@hugomontero Added a _getDeviceInfo
instead of two different methods. But to write everything in a single Promise.all() , the _getDeviceName()
requires id
which is available from the first call. Let me know if I am missing something.
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.
Is like you said hehe. 👍
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 Worked great!! I Just put a minor 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.
Looks good. I made a suggestion to handle unknown and no name
src/cmd/usb-util.js
Outdated
const device = await getDevice({ id, api, auth, ui }); | ||
return device.name; | ||
} catch (err) { | ||
// ignore error |
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.
Can you see if you can distinguish the 2 cases in this error handler and return the following names:
<no name>
: the device hasn't been named yet
<unknown>
: if the API request failed (errored with 404 Not Found because you don't have access to the device or if the API request couldn't be performed because the computer is offline and the device attributes are not in cache)
particle usb list
does show <no name>
$ particle usb list
<no name> [18002b000247353138383138] (Photon, LISTENING)
extra_p2 [0a10aced202194944a02c5f4] (Photon 2 / P2)
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.
yes - let me try that
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.
Hope this looks correct - added in the change
---> If connection is lost
? Which device would you like to select? (Use arrow keys)
❯ <unknown> [0a10aced202194944a02c4d4] (Photon 2 / P2, DFU)
<unknown> [e00fce6819ef5f971ea9563a] (Argon)
---> No name
? Which device would you like to select? (Use arrow keys)
❯ seal_penguin [0a10aced202194944a02c4d4] (Photon 2 / P2, DFU)
<no name> [e00fce6819ef5f971ea9563a] (Argon)
Description
Device selector will now show more details
With user creds and api connectivity available
Without api connectivity, it does not show the name
How to Test
npm start -- flash --usb app.bin
Related Issues / Discussions
NA
Completeness