Update Daily transport to 1.0.0 #22
Conversation
|
CI is expected to fail as the client library hasn't been published yet. |
| } | ||
| } | ||
|
|
||
| object Cameras { |
There was a problem hiding this comment.
This new camera handling code is a workaround for an issue reported on Discord -- it seems like there's a problem with daily-x failing to set cameras by device ID. We should look into the root cause, but for now this is more robust.
| } | ||
|
|
||
| override fun connect(authBundle: AuthBundle?): Future<Unit, RTVIError> = | ||
| override fun connect(transportParams: Value): Future<Unit, RTVIError> = |
There was a problem hiding this comment.
I'm not sure where/how the type is defined here but I wanted to point out that an update I made to the daily transport in client-js was that the parameter passed in here is the same as the type that daily-js.join takes. That doesn't work quite the same for daily-x since join takes a couple parameters, but the idea would be to make it such that we can support folks doing advanced things and overriding settings at join time.
There was a problem hiding this comment.
Spent a while thinking about how to do this, and eventually came up with a type-safe way, which we can extend with more parameters in future:
Client lib: pipecat-ai/pipecat-client-android@20d3120
Daily transport: 8e45e3a
Demo app: pipecat-ai/pipecat-examples@1dc4b9b
The downside is that PipecatClient now requires two generic parameters (transport type and connect params type), but I've added a simple type alias PipecatClientDaily for this.
mattieruth
left a comment
mattieruth
left a comment
There was a problem hiding this comment.
other than the one comment, i think this looks good. my kotlin is not great though, so this is a higher-level non-syntactical review :)
|
Thanks for all the reviews @mattieruth! |
No description provided.