async support about rust-cast HOT 14 OPEN

I don't know how you feel about that and how committed you are to the current API

If there is a need to change the API to improve it, go for it. The initial interface was written ~7-8 years ago when I was just learning Rust, and not much has changed in this interface since then.

Options 2 and 3 sound like the best options to me. I don't have a preference for one over the other, but I believe there is still value in having sync APIs to make it much easier to get started. If we have to make breaking changes in the sync APIs along the way, it's fine too. We're at 0-major, so bumping the minor version because of breaking changes is reasonable.

from rust-cast.

Awesome, thanks for the update @fluffysquirrels! I've just merged #22 and also bumped dependencies' versions, so you might need to rebase your fork on main.

from rust-cast.

The current API goes either through receive or through receive_find_map. receive_find_map callers do the request-response association, but a client that calls receive in parallel and discards some messages could starve a function waiting for a response.

I think this was already the case if the thread-safe feature was used; one of the per-channel request-response function could be preempted between calling send and receive_find_map.

It's worth adding something that maps from request-ids to waiting tasks. Maybe even make the general receive case return only broadcasts. I'll look at your approach.

from rust-cast.

@azasypkin, these 2 branches and approaches are diverging and will not merge with each other. What do you want to do in this repo?

Thanks for the ping @fluffysquirrels! Assuming our common goal here is to have an async API in addition to the sync one as soon as possible, would it be feasible for you and @g2p to collaborate on picking and implementing a single approach that sounds sensible for both of you?

You both definitely use this library more than I do, and ideally it would make sense for you to shape it instead of me dictating the approach. With this in mind, I see three options:

1. If feasible, you both collaborate on a single feature branch, splitting the work and performing cross-reviews.
2. If someone has more desire and time to drive the implementation, the other one can serve as the primary reviewer for the first one.
3. If, for some reason, you cannot agree on either option 1 or 2 within a reasonable time frame, let's say by the end of next week (Mar 31st, 2024), I'll just go ahead and pick the approach that seems to be closer to the finish line to unblock the progress. We can always refine it in the follow-up.

I'd be happy to assist with additional review if needed or unblock us if we're stuck again for some reason.

How does that sound @fluffysquirrels @g2p?

from rust-cast.

Hey @fluffysquirrels ,

Thanks for filing the issue!

Would you open to collaborating on this?

I'm definitely open to collaborating on this and believe that this would be a nice addition. Unfortunately, I don't have access to my usual development Chromecast device to test the changes at the moment (I happen to have Chromecast with Google TV (HD) but, unlike older Chromecasts, it doesn't seem to work without a proper HDMI output device). I'll get back to my devices in mid-February, and if it's not too late for you, I can help with the changes/review/testing then.

from rust-cast.

I'll get back to my devices in mid-February, and if it's not too late for you, I can help with the changes/review/testing then.

This is no problem, sounds like a good plan.

I've made some progress in a private branch getting an async client working. It's approximately the right design I think but still with rough edges. I have maybe 50 lines left to write before it can send the first request message and receive a response; the first I want to implement is getting the receiver status. Once that is working I'm planning to tidy it up slightly and push to my fork for you to look at.

One problem currently is that my current design is a fairly large departure from the existing sync code. Some of that is my opinions on how to write the code, some of it is required due to switching to async. I don't know how you feel about that and how committed you are to the current API. A few options I see to resolve this:

1. Major version increase for rust_cast crate, new async API, remove old sync API, provide migration instructions for users.
2. Keep the existing sync API, new async API in a sub-module, both share a common core implementation where practical (including many data structs, constants, most serialization and deserialization).
3. New async API in a sub-module, existing sync API is rewritten as a wrapper to use the async API internally.
4. async implementation goes in a new crate, links between the async and sync versions recommending each other for users that do or don't want to use async and tokio. I'm happy to collaborate on anything that could reasonablly be shared, e.g. automated tests, documentation, a common core crate.

from rust-cast.

I've made some progress on this, and have the core features I need working on a branch. That includes a basic test CLI app that can:

• Connect
• Get receiver status
• Set volume
• Launch the default media receiver app
• Get media receiver status
• Load a media file
• Stop a receiver app
• Stay connected and listen to status broadcasts from the receiver

These are enough features that I am confident the code design will work.

I am ready to start polishing it up and refactoring to get ready for review. There is no need for you to look at the code yet, but I thought I'd give a status update.

This is the diff between my async branch and your main branch

Note that my async branch was based on tsirysndr's fork. Their fork has not diverged much from your repo, but had switched to rustls from openssl for the TLS implementation, and I wanted to keep that.

from rust-cast.

I have a branch that adds async (draft PR #28), in a not too intrusive way (adds 93 lines net under src/). It's on top of my other two PRs (request ids and queue operations).

https://github.com/g2p/rust-cast/commits/async

I have not wrapped it back into a sync version, because it would require duplicating every function. Changing existing users to use async is mechanical: change callers to async fns and add .await everywhere the compiler asks.

For me the point was to send commands and listen for status updates on the same connection.

from rust-cast.

from rust-cast.

It's worth adding something that maps from request-ids to waiting tasks

Indeed, I have that in Task.requests_map (in case you want to look).

Another 2 reasons I added the spawned Task:

1. The client must keep responding to heartbeat PING messages from the Chromecast or the connection will be closed.
2. For my application I want a stream of status updates (using tokio::sync::broadcast) from the Chromecast while a video is playing.

While I could put the logic and event loop task for these 2 in my own application, it made much more sense to me to do that once (in a crate) that others could re-use. For a one-off command to the Chromecast from a short-running CLI app, the current API works well enough. But in a longer-running app, you need to write quite a chunk in the app yourself.

The style of the API on my branch was influenced by 2 JavaScript Chromecast libraries I used from another tool, which I found much easier to work with.

from rust-cast.

Another point on API.

In my own private application I want to discover and listen to all the Chromecasts on the LAN of the host device.

To implement that I have a wrapper -- currently called CastController -- that does the following:

• Poll mDNS regularly to discover any Chromecast on the LAN
• Open a connection to it
• Read status updates from the Chromecast
• Broadcast the status updates to other in-process receivers

Then in my application, I send those status updates over WebSockets for a web UI display.

CastController is a work in progress, but I was thinking about contributing that to this crate, either as a follow-up PR or in this one. Would others be interested in this functionality?

from rust-cast.

I have replaced receive_find_map with a new send_get_reply in my branch (g2p/rust-cast@queue-load...async,queue), now everything is plugged properly so that receive() only gets non-replies and wakes send_get_reply() futures, while send_get_reply() polls the stream for a matching requestId even if no receive() is running.

The internals were refactored, (removing code around deserialisation, reworking MessageManager) but the external channel interface is basically the same, just async. Maybe there's a way to add some per-channel behaviour like keeping track of media statuses, which would go beyond the current request-reply interfaces.

One thing that bugs me with the current request-reply pattern is that it's hard to track the latest media status (you have to track every response to a media channel method call, as well as messages from receive()), and you can't always get the latest receiver status (when calling set_volume(), the channel interface discards the reply to keep only a simplified volume struct, so you have to follow up with an extra get_status). It's motivation to add something that listens to every message on select channels.

from rust-cast.

One thing that bugs me with the current request-reply pattern is that it's hard to track the latest media status

I do this in my branch. I broadcast on a channel a variety of status messages, including media and receiver status, for the reason you describe.

I also updated the set volume method to return the receiver's status, not just the volume struct.

@g2p, I feel like between my branch and yours we are duplicating exactly the same work between us. Is there an alternative?

@azasypkin, these 2 branches and approaches are diverging and will not merge with each other. What do you want to do in this repo?

from rust-cast.

Hey @fluffysquirrels,

Assuming you haven't had any discussions with @g2p outside of this issue, and you still have the capacity and desire to push the async support over the finish line, let's proceed with your PR.

@g2p, it would be great if you could still be the primary reviewer for @fluffysquirrels' async PR so that it also caters to your use case.

Thanks.

from rust-cast.