search

jamesmunns / postcard-rpc

An RPC layer for postcard based protocols
Apache License 2.0
48 stars 11 forks source link

WebUSB support and `Client` trait #17

Closed spookyvision closed 4 weeks ago

spookyvision commented 1 month ago

(this text will be edited, further discussion needed) currently this PR adds support for something that is vaguely shaped like WebUSB, however: - the actual WebUSB-specific part isn't done inside this repo (and maybe shouldn't be) - the implementation is rather generic and can probably be merged with nusb - there is a dependency on dioxus::spawn which violates separation of concerns in 2 ways - 1. dioxus should IMHO not be a dependency of prpc, even behind a feature gate, 2. Dioxus is orthogonal to WebUSB. WASM prpc apps without Dioxus are totally possible, but Dioxus apps require to run inside the Dioxus scope. The solution to that would probably be something like a generic Spawner field on HostClient. But I don't want to start working on that before we've answererd the question of how to use async traits instead of HostClient::new_manual.

this PR adds a Client trait to implement a wire protocol backend, associated helper functionality for HostClient, and a WebUSB implementation of Client.

dingari commented 1 month ago

Great work! I was able to use this to make a simple POC ping example with a minimal effort webusb "wire" implementation using web_sys in Rust.

One thing regarding the dioxus dependency. I'm not familiar with the framework at all, I'm just serving very simple HTML/JS with webpack as a POC. Using wasm_bindgen_futures::spawn_local works just fine in that context.

spookyvision commented 1 month ago

@dingari cool! My most recent commit also adds a wire implementation to relieve users of that implementation burden 😬 The dioxus dependency is required iff one is using dioxus, but the good news is that spawning can easily be delegated to a feature flag. However I'm not 100% happy with the way spawning is handled now (as per my latest commit, which moves prpc towards more code reuse regarding different wire backends), and have already raised this with James.

dingari commented 1 month ago

Awesome, works great!

For now, we just wrap WebUsbClient and impl Client on it to delegate spawning to wasm_bindgen_futures::spawn_local instead of dioxus::spawn.

spookyvision commented 1 month ago

good news, we don't need Dioxus anymore after all! an old version of my driver was using Dioxus signals to store state and that's probably what required the global Dioxus context. All gone now!