-
Notifications
You must be signed in to change notification settings - Fork 557
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 device API #2864
Add device API #2864
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
/** | ||
* The type of the device. | ||
*/ | ||
export type DeviceType = 'hid' | 'bluetooth'; |
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.
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.
We could, but I don't see the benefit in this case.
type?: Type, | ||
): Type extends DeviceType ? Struct<ScopedDeviceId<Type>> : Struct<DeviceId> { | ||
return refine(string(), 'device ID', (value) => { | ||
if (type) { |
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 not sure I follow what this struct is meant to validate
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.
Right now it just validates that a string consists of three parts (1:2:3
).
/** | ||
* The vendor ID of the device. | ||
*/ | ||
vendorId?: number; |
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.
Do we just expand this in the future for other devices in one big object?
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.
If there's some type-specific filters later, we could do a discriminated union type, with filters specific to each type.
{
type: 'some-new-type',
filters: FiltersForSomeNewType[];
}
/** | ||
* The result returned by the `snap_readDevice` method. | ||
*/ | ||
export type ReadDeviceResult = Hex; |
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 this implying that we will call bytesToString
on the return value from the device? Curious how that performs versus just passing an array versus whatever else
/** | ||
* The data to write to the device. | ||
*/ | ||
data: Hex; |
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.
Same question as read with regards to what we want this type to be and what is most performant
params: { | ||
type: 'hid', | ||
id: this.device.id, | ||
data: bytesToHex(block), |
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.
How large are these blocks? It might help us make a decision on the type for data
No description provided.