Live Location Sharing - Send location data #5697
Conversation
onurays
requested review from
a team and
ericdecanini
and removed request for
a team
| val room = activeSessionHolder.getSafeActiveSession()?.getRoom(roomArg.roomId) | ||
| room?.getStateEvent(EventType.STATE_ROOM_BEACON_INFO.first())?.let { beaconInfoEvent -> | ||
| room.sendLiveLocation( | ||
| beaconInfoEventId = beaconInfoEvent.eventId!!, |
There was a problem hiding this comment.
Nit: Would it make sense to chain this before the let to avoid the force unwrap? Looks like you're not reading anything else from the event here anyway.
There was a problem hiding this comment.
Nice catch, done. (Although I don't know why event id is nullable).
onurays
requested review from
a team and
ouchadam
and removed request for
ericdecanini and
a team
| @@ -49,7 +49,8 @@ object EventType { | |||
| const val STATE_ROOM_JOIN_RULES = "m.room.join_rules" | |||
| const val STATE_ROOM_GUEST_ACCESS = "m.room.guest_access" | |||
| const val STATE_ROOM_POWER_LEVELS = "m.room.power_levels" | |||
| private const val STATE_ROOM_BEACON_INFO_PREFIX = "org.matrix.msc3489.beacon_info." | |||
| val STATE_ROOM_BEACON_INFO = listOf("org.matrix.msc3672.beacon_info", "m.beacon_info") | |||
There was a problem hiding this comment.
out of interest, how come we've turned these into a list? looks like we're always using the first item~
There was a problem hiding this comment.
Spec introduced unstable prefixes recently. We use them until MSCs are merged. Then we will send stable ones, so we also need to support previously sent unstable ones.
I am thinking about a better approach. We have similar events for polls and there will be more coming.
There was a problem hiding this comment.
thanks for the explanation! could be nice to have way to say STATE_ROOM_BEACON_INFO.stable/unstable() instead of STATE_ROOM_BEACON_INFO.first()
There was a problem hiding this comment.
Nice idea, I will improve all in another PR
|
|
||
| // Emit location update to all rooms in which live location sharing is active | ||
| roomArgsList.forEach { roomArg -> | ||
| sendLiveLocation(roomArg.roomId, locationData) |
There was a problem hiding this comment.
is sendLiveLocation a blocking network call? it might need a io dispatchers~ anddd it might be safer to copy the roomArgsList to avoid also blocking the synchronised calls
val inputs = roomArgsList.toList()
job?.cancel()
job = scope.launch(io) {
inputs.forEach { sendLiveLocation() }
}There was a problem hiding this comment.
We use CoroutineScope in EventSenderProcessorCoroutine.launchWith(). Securing list is a good idea and done.
|
|
||
| @Json(name = "body") override val body: String = "", | ||
| @Json(name = "m.relates_to") override val relatesTo: RelationDefaultContent? = null, | ||
| @Json(name = "m.new_content") override val newContent: Content? = null, |
There was a problem hiding this comment.
Stupid question, sorry: This doesn't seem to be read or set anywhere in this diff. I also haven't seen this on any of the location sharing MSCs. What is this field used for?
Sends beacon pulse events with a reference to the initial beacon info event.