diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 4104998d..ceb80d91 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -37,8 +37,8 @@ android { minSdk = 33 targetSdk = 35 // Remain newer than the 0.6.3 rendezvous candidate (code 26). - versionCode = 31 - versionName = "0.6.8" + versionCode = 32 + versionName = "0.6.9" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" diff --git a/app/src/androidTest/kotlin/dev/forgesworn/kithmoot/storage/PersistentGroupUiTest.kt b/app/src/androidTest/kotlin/dev/forgesworn/kithmoot/storage/PersistentGroupUiTest.kt index b357559e..02ca830a 100644 --- a/app/src/androidTest/kotlin/dev/forgesworn/kithmoot/storage/PersistentGroupUiTest.kt +++ b/app/src/androidTest/kotlin/dev/forgesworn/kithmoot/storage/PersistentGroupUiTest.kt @@ -9,6 +9,7 @@ import androidx.test.platform.app.InstrumentationRegistry import dev.forgesworn.kithmoot.KithMootApplication import dev.forgesworn.kithmoot.MainActivity import dev.forgesworn.kithmoot.protocol.* +import dev.forgesworn.kithmoot.relay.RelayChoice import dev.forgesworn.kithmoot.ui.RoomViewModel import kotlinx.serialization.json.* import okhttp3.Response @@ -43,15 +44,10 @@ class PersistentGroupUiTest { } private fun chooseRelay(url: String) { - var previous = emptyList() - activity.scenario.onActivity { previous = ViewModelProvider(it)[RoomViewModel::class.java].accountRelayChoices().map { relay -> relay.url } } - ui.click("Sign in") - ui.click("Relays") - previous.forEach { ui.click("Remove relay $it") } - ui.replace("Add relay URL", url) - ui.click("Add relay") - ui.click("Save relay choices") - ui.click("Done") + activity.scenario.onActivity { + val model = ViewModelProvider(it)[RoomViewModel::class.java] + assertNull(model.saveAccountRelays(model.accountRelayChoices() + RelayChoice(url))) + } } private fun webLink(server: StoredGroupRelay): String { diff --git a/app/src/main/kotlin/dev/forgesworn/kithmoot/media/Negotiation.kt b/app/src/main/kotlin/dev/forgesworn/kithmoot/media/Negotiation.kt index 5f15c2a8..e316efb4 100644 --- a/app/src/main/kotlin/dev/forgesworn/kithmoot/media/Negotiation.kt +++ b/app/src/main/kotlin/dev/forgesworn/kithmoot/media/Negotiation.kt @@ -677,7 +677,7 @@ class PeerLink( makingOffer = true val local = connection.setLocalDescription() outstandingOfferSeq = seq - Log.i(NEGOTIATION_LOG, "offer sent peer=${remoteDevice.take(8)} seq=${seq ?: "-"} profile=${if (splitGuard) 2 else 1}") + Log.i(NEGOTIATION_LOG, "offer sent peer=${remoteDevice.take(8)} seq=${seq ?: "-"} profile=$callProfile") send(SignalEnvelope(remoteDevice, SignalType.OFFER, roomId, sdp = local.sdp, seq = seq)) } finally { makingOffer = false diff --git a/app/src/main/kotlin/dev/forgesworn/kithmoot/media/WebRtcEngine.kt b/app/src/main/kotlin/dev/forgesworn/kithmoot/media/WebRtcEngine.kt index 38f23e3c..d3634292 100644 --- a/app/src/main/kotlin/dev/forgesworn/kithmoot/media/WebRtcEngine.kt +++ b/app/src/main/kotlin/dev/forgesworn/kithmoot/media/WebRtcEngine.kt @@ -166,12 +166,21 @@ class WebRtcEngine( scope.launch { session.remoteDevices.collect { reconcile(it) } } scope.launch { session.signals.collect { signal -> + val target = managedLinkFor(signal.from) ?: return@collect try { - linkFor(signal.from)?.onRemoteSignal(inboundEnvelope(signal.from, signal.body)) + target.onRemoteSignal(inboundEnvelope(signal.from, signal.body)) } catch (cancelled: CancellationException) { throw cancelled } - catch (_: Exception) { - // One failed negotiation must not cancel every other device's media collectors. - _connections.update { it + (signal.from to "failed") } + catch (failure: Exception) { + // A signal can finish after its link was deliberately + // replaced. That old link's exception must not relabel the + // replacement as failed. Check identity and publish the + // state while holding the same lock used for replacement. + synchronized(lock) { + if (callActive && links[signal.from] === target) { + Log.w("KithMootMedia", "signal failed peer=${signal.from.take(8)}", failure) + _connections.update { it + (signal.from to "failed") } + } + } } } } @@ -241,7 +250,7 @@ class WebRtcEngine( } } - private fun linkFor(device: String): PeerLink? = synchronized(lock) { links[device]?.link } + private fun managedLinkFor(device: String): ManagedLink? = synchronized(lock) { links[device] } /** * Throw a pair's connection away and open another. @@ -444,7 +453,7 @@ class WebRtcEngine( // reaches until it has answered. See refreshRemoteTracks. override fun onSignalingChange(state: PeerConnection.SignalingState?) = Unit override fun onIceConnectionChange(state: PeerConnection.IceConnectionState?) { - if (!closed && state != null) _connections.update { it + (device to state.name.lowercase()) } + if (state != null) updateConnectionState(state.name.lowercase()) } /** * The transport's own verdict, which used to be recorded and @@ -458,7 +467,7 @@ class WebRtcEngine( override fun onConnectionChange(state: PeerConnection.PeerConnectionState?) { if (closed || state == null) return val name = state.name.lowercase() - _connections.update { it + (device to name) } + updateConnectionState(name) if (profileTwo) health.onConnectionState(name, System.currentTimeMillis()) } override fun onIceConnectionReceivingChange(receiving: Boolean) = Unit @@ -477,7 +486,7 @@ class WebRtcEngine( scope.launch { try { link.onNegotiationNeeded() } catch (cancelled: CancellationException) { throw cancelled } - catch (_: Exception) { if (!closed) _connections.update { it + (device to "failed") } } + catch (_: Exception) { updateConnectionState("failed") } } } @@ -546,7 +555,19 @@ class WebRtcEngine( gen = generation, ) } catch (cancelled: CancellationException) { throw cancelled } - catch (_: Exception) { if (!closed) _connections.update { it + (device to "failed") } } + catch (_: Exception) { updateConnectionState("failed") } + } + } + } + + /** Publish state only while this is still the device's current link. + * Native WebRTC callbacks can arrive after close/rebuild; without the + * identity check an old callback can overwrite the new link's state. + */ + private fun updateConnectionState(state: String) { + synchronized(lock) { + if (!closed && callActive && links[device] === this) { + _connections.update { it + (device to state) } } } } @@ -667,6 +688,10 @@ class WebRtcEngine( } } + suspend fun onRemoteSignal(body: SignalEnvelope) { + if (!closed) link.onRemoteSignal(body) + } + fun close() { closed = true senders.clear() diff --git a/app/src/main/kotlin/dev/forgesworn/kithmoot/session/CallProfile.kt b/app/src/main/kotlin/dev/forgesworn/kithmoot/session/CallProfile.kt index 5d3c03b5..9dc03390 100644 --- a/app/src/main/kotlin/dev/forgesworn/kithmoot/session/CallProfile.kt +++ b/app/src/main/kotlin/dev/forgesworn/kithmoot/session/CallProfile.kt @@ -15,16 +15,15 @@ package dev.forgesworn.kithmoot.session const val CALL_PROFILE_2: Int = 2 /** - * The one switch, and it is off. + * The one switch, enabled for this release. * - * Off, nothing changes on the wire for anybody: this device's roster entry - * carries no `callProfile`, so no far end will open a profile-2 pair with it, - * and this device opens none either. On, it advertises profile 2 and uses it - * with any pair whose far end advertises it too - both ends, never one. + * This device advertises profile 2 and uses it only with a far end that also + * advertises it. Older clients therefore remain on profile 1 automatically; + * a mixed-version room never reaches the fixed-slot signalling by accident. * * A build-time constant rather than a setting because it must be impossible to * reach halfway: a device that advertised the profile and then could not * carry it would leave every pair it touched waiting for an offer that was * never coming. */ -const val CALL_PROFILE_2_ENABLED: Boolean = false +const val CALL_PROFILE_2_ENABLED: Boolean = true diff --git a/app/src/test/kotlin/dev/forgesworn/kithmoot/ui/RelayInputTest.kt b/app/src/test/kotlin/dev/forgesworn/kithmoot/ui/RelayInputTest.kt index 9d3a4f7f..bef48335 100644 --- a/app/src/test/kotlin/dev/forgesworn/kithmoot/ui/RelayInputTest.kt +++ b/app/src/test/kotlin/dev/forgesworn/kithmoot/ui/RelayInputTest.kt @@ -38,5 +38,6 @@ class RelayInputTest { fun `the defaults parse`() { assertEquals(DEFAULT_RELAYS, parseRelays(DEFAULT_RELAYS.joinToString("\n"))) assertTrue(DEFAULT_RELAYS.isNotEmpty()) + assertTrue("wss://relay.trotters.cc" in DEFAULT_RELAYS) } }