-
Notifications
You must be signed in to change notification settings - Fork 58
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 chat API #436
Add chat API #436
Conversation
livekit-ffi/src/proto.rs
Outdated
include!("livekit.proto.rs"); | ||
|
||
impl From<ChatMessage> for RoomChatMessage { |
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.
currently defining these conversions both in livekit-ffi and in livekit, which feels wrong, but couldn't make it work otherwise.
@@ -40,3 +40,4 @@ futures-util = { version = "0.3", default-features = false, features = ["sink"] | |||
thiserror = "1.0" | |||
lazy_static = "1.4" | |||
log = "0.4" | |||
chrono = "0.4.38" |
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.
needed for timestamping chat messages
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.
this makes sense to me
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.
lgtm!
message EditChatMessageRequest { | ||
uint64 local_participant_handle = 1; | ||
string edit_text = 2; | ||
ChatMessage original_message = 3; |
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 need the whole ChatMessage? Is it possible to just use the message_id?
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.
in an ideal world, yeah, the ID would be enough.
We don't have a central authority for chat messages though. So for an edited message we'd still want to keep the e.g. original timestamp of the message, so passing the whole message seemed like the best idea.
But would be happy about alternative suggestions
No description provided.