Skip to content
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

[question] Why are WebHID calls to sendReport queued with concurrency 1? #111

Closed
loucadufault opened this issue Nov 29, 2024 · 4 comments
Closed

Comments

@loucadufault
Copy link
Contributor

I am looking to perform multiple writes to the device in a synchronous manner. The current wrapper method implementation sequentially queues the sendReport promises:

public write(data: number[]): void {
this.reportQueue
.add(async () => {
await this.device.sendReport(data[0], new Uint8Array(data.slice(1)))
})
.catch((err) => {
this.emit('error', err)
})
}

Is there a reason this is done? Wondering if there were any observed issues or concerns with firing for example a few dozens sendReport calls without waiting for any of them to resolve (before firing off the next one).

The context is I am looking to perform some cleanup of the device on beforeunload, which must be sync.

@nytamin
Copy link
Member

nytamin commented Nov 29, 2024

Honestly, I don't quite remember why I wrapped it in a promise queue. Perhaps one could do a synthetic test and send multiple commands and see what happens?

However, if you're looking to send something in a synchronous manner, I don't think you'll be able to do that anyway, since the webhidDevice.sendReport() is asynchronous in itself, so you'll have no way of knowing if the commands have actually been sent, right?

@Julusian
Copy link
Member

Julusian commented Dec 2, 2024

I think this is a little related to #106

I suspect what happened is this code was based on the streamdeck library, which uses a queue because it sends messages in multiple packets that need to not be interleaved with other calls to write https://github.com/Julusian/node-elgato-stream-deck/blob/6240d8853272a4f651b835f332ef14f77c512ee5/packages/webhid/src/hid-device.ts#L45-L51 (Added in commit fix: ensure order of hid.sendReport calls when image is multiple packets. concurrent draws previously would result in some corruption)

For the xkeys where the writes are only ever a single hid packet, it does look unnecessary

@michaelhatPIengineering
Copy link
Collaborator

The simple commands for setting LEDs and such are only a single packet. However, there are cases when sending multiple packet writes are required with X-keys. Since we support the generic "write data" command we can use web hid for maintenance of X-keys firmware for example. These operation would require multiple writes in the proper order.

@loucadufault
Copy link
Contributor Author

loucadufault commented Dec 3, 2024

However, if you're looking to send something in a synchronous manner, I don't think you'll be able to do that anyway, since the webhidDevice.sendReport() is asynchronous in itself, so you'll have no way of knowing if the commands have actually been sent, right?

That's acceptable for my use case, it's a fire and forget as the app is closing.

Based on the info in this thread given that I am only sending commands for setting LEDs I will go ahead with this approach.

@nytamin nytamin closed this as completed Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants