diff --git a/app/src/full/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt b/app/src/full/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt index 6c2c5e70ce0..4f5a0abb5bf 100644 --- a/app/src/full/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt +++ b/app/src/full/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt @@ -295,19 +295,53 @@ class ThreadManagerImpl @Inject constructor( .addOnFailureListener { cont.resumeWithException(it) } } - override suspend fun sendThreadDatasetExportResult(result: ActivityResult, serverId: Int): String? { - if (result.resultCode == Activity.RESULT_OK && coreSupportsThread(serverId)) { - val threadNetworkCredentials = ThreadNetworkCredentials.fromIntentSenderResultData(result.data!!) - try { - val added = serverManager.webSocketRepository( - serverId, - ).addThreadDataset(threadNetworkCredentials.activeOperationalDataset) - if (added) return threadNetworkCredentials.networkName - } catch (e: Exception) { - Timber.e(e, "Error while executing server new Thread credentials request") + @OptIn(ExperimentalStdlibApi::class) + override suspend fun sendThreadDatasetExportResult( + result: ActivityResult, + serverId: Int, + ): ThreadManager.ExportResult { + if (result.resultCode != Activity.RESULT_OK || !coreSupportsThread(serverId)) { + return ThreadManager.ExportResult.NotSent + } + val credentials = ThreadNetworkCredentials.fromIntentSenderResultData(result.data!!) + val webSocket = serverManager.webSocketRepository(serverId) + val added = try { + webSocket.addThreadDataset(credentials.activeOperationalDataset) + } catch (e: Exception) { + Timber.e(e, "Error while executing server new Thread credentials request") + return ThreadManager.ExportResult.Failed + } + if (!added) return ThreadManager.ExportResult.Failed + + // thread/add_dataset_tlv never promotes a dataset to preferred on the server side. To + // surface an honest result we re-query and compare extended PAN IDs. If the server has + // no preferred dataset at all (e.g. no border router announcing one), promote the + // just-exported dataset; we never override an existing server preference. + val deviceExtPan = credentials.extendedPanId.toHexString(HexFormat.UpperCase) + val serverPrefersExported = try { + val datasets = webSocket.getThreadDatasets() + val preferred = datasets?.firstOrNull { it.preferred } + when { + preferred == null -> { + val ours = datasets?.firstOrNull { it.extendedPanId.equals(deviceExtPan, ignoreCase = true) } + if (ours != null && webSocket.setThreadPreferredDataset(ours.datasetId)) { + Timber.d("Thread: promoted exported dataset to preferred on the server") + true + } else { + false + } + } + preferred.extendedPanId.equals(deviceExtPan, ignoreCase = true) -> true + else -> false } + } catch (e: Exception) { + Timber.w(e, "Unable to verify Thread preferred dataset after export") + null } - return null + return ThreadManager.ExportResult.Sent( + networkName = credentials.networkName, + serverPrefersExported = serverPrefersExported, + ) } private suspend fun deleteOrphanedThreadCredentials(context: Context, serverId: Int) { diff --git a/app/src/main/kotlin/io/homeassistant/companion/android/settings/developer/DeveloperSettingsPresenterImpl.kt b/app/src/main/kotlin/io/homeassistant/companion/android/settings/developer/DeveloperSettingsPresenterImpl.kt index 47edcd6ddd9..264db9e2b4f 100644 --- a/app/src/main/kotlin/io/homeassistant/companion/android/settings/developer/DeveloperSettingsPresenterImpl.kt +++ b/app/src/main/kotlin/io/homeassistant/companion/android/settings/developer/DeveloperSettingsPresenterImpl.kt @@ -133,19 +133,39 @@ class DeveloperSettingsPresenterImpl @Inject constructor( ) { mainScope.launch { try { - val submitted = threadManager.sendThreadDatasetExportResult(result, serverId) - if (submitted != null) { - if (isDeviceOnly) { - view.onThreadDebugResult(context.getString(commonR.string.thread_debug_result_exported), true) - } else { - // If we got permission while both had a dataset, the device prefers a different network - val out = "${context.getString( - commonR.string.thread_debug_result_mismatch, - )} ${context.getString(commonR.string.thread_debug_result_mismatch_detail, submitted)}" - view.onThreadDebugResult(out, null) + when (val sent = threadManager.sendThreadDatasetExportResult(result, serverId)) { + is ThreadManager.ExportResult.Sent -> { + if (isDeviceOnly) { + view.onThreadDebugResult( + context.getString(commonR.string.thread_debug_result_exported), + true, + ) + } else if (sent.serverPrefersExported == true) { + view.onThreadDebugResult(context.getString(commonR.string.thread_debug_result_match), true) + } else if (sent.serverPrefersExported == false) { + // The dataset was added on the server but a different one is still + // preferred (typically a server-managed border router announces it). + val out = "${context.getString( + commonR.string.thread_debug_result_exported_not_preferred, + )} ${context.getString( + commonR.string.thread_debug_result_mismatch_detail, + sent.networkName, + )}" + view.onThreadDebugResult(out, null) + } else { + val out = "${context.getString( + commonR.string.thread_debug_result_mismatch, + )} ${context.getString( + commonR.string.thread_debug_result_mismatch_detail, + sent.networkName, + )}" + view.onThreadDebugResult(out, null) + } } - } else { - view.onThreadDebugResult(context.getString(commonR.string.thread_debug_result_error), false) + ThreadManager.ExportResult.NotSent, + ThreadManager.ExportResult.Failed, + -> + view.onThreadDebugResult(context.getString(commonR.string.thread_debug_result_error), false) } } catch (e: Exception) { view.onThreadDebugResult(context.getString(commonR.string.thread_debug_result_error), false) diff --git a/app/src/main/kotlin/io/homeassistant/companion/android/thread/ThreadManager.kt b/app/src/main/kotlin/io/homeassistant/companion/android/thread/ThreadManager.kt index 3af998d53ff..04f02d363a7 100644 --- a/app/src/main/kotlin/io/homeassistant/companion/android/thread/ThreadManager.kt +++ b/app/src/main/kotlin/io/homeassistant/companion/android/thread/ThreadManager.kt @@ -23,6 +23,27 @@ interface ThreadManager { object NoneHaveCredentials : SyncResult() } + sealed class ExportResult { + /** + * The activity result was not OK, the server doesn't support Thread, or there was no + * dataset to send. + */ + object NotSent : ExportResult() + + /** Sending the dataset to the server failed. */ + object Failed : ExportResult() + + /** + * The dataset was added on the server. + * @param networkName Network name of the dataset that was sent + * @param serverPrefersExported `true` if the server now reports the exported dataset as + * its preferred one, `false` if the server still prefers a different dataset (typically + * because a Thread border router managed by the server is announcing one), or `null` if + * the post-export state could not be determined. + */ + data class Sent(val networkName: String, val serverPrefersExported: Boolean?) : ExportResult() + } + /** * Indicates if the app on this device supports Thread credential management. */ @@ -82,7 +103,15 @@ interface ThreadManager { /** * Process the result from [syncPreferredDataset] or [getPreferredDatasetFromDevice]'s intent * and add the Thread dataset, if any, to the server. - * @return Network name that was sent and accepted, or `null` if not sent or accepted + * + * The `thread/add_dataset_tlv` server command never promotes a dataset to preferred. After a + * successful add the server is queried again to determine the post-export state: if the + * server has no preferred dataset, the just-exported one is promoted; if it has another + * preferred dataset (typically because a server-managed border router announces it), no + * override is performed. + * + * @return [ExportResult] describing whether the dataset was sent and, if so, whether the + * server now prefers it */ - suspend fun sendThreadDatasetExportResult(result: ActivityResult, serverId: Int): String? + suspend fun sendThreadDatasetExportResult(result: ActivityResult, serverId: Int): ExportResult } diff --git a/app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewPresenterImpl.kt b/app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewPresenterImpl.kt index f6c05491366..73e8b2e1cd6 100644 --- a/app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewPresenterImpl.kt +++ b/app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewPresenterImpl.kt @@ -603,12 +603,16 @@ class WebViewPresenterImpl @Inject constructor( mainScope.launch { val sent = threadUseCase.sendThreadDatasetExportResult(result, serverId) Timber.d( - "Thread ${if (!sent.isNullOrBlank()) "sent credential for $sent" else "did not send credential"}", + "Thread ${if (sent is ThreadManager.ExportResult.Sent) { + "sent credential for ${sent.networkName} (server prefers exported: ${sent.serverPrefersExported})" + } else { + "did not send credential" + }}", ) - if (sent.isNullOrBlank()) { - mutableMatterThreadStep.tryEmit(MatterThreadStep.THREAD_NONE) - } else { + if (sent is ThreadManager.ExportResult.Sent) { mutableMatterThreadStep.tryEmit(MatterThreadStep.THREAD_SENT) + } else { + mutableMatterThreadStep.tryEmit(MatterThreadStep.THREAD_NONE) } } } diff --git a/app/src/minimal/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt b/app/src/minimal/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt index 1676bf3479f..43be15d14e4 100644 --- a/app/src/minimal/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt +++ b/app/src/minimal/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt @@ -36,5 +36,8 @@ class ThreadManagerImpl @Inject constructor() : ThreadManager { throw IllegalStateException("Thread is not supported with the minimal flavor") } - override suspend fun sendThreadDatasetExportResult(result: ActivityResult, serverId: Int): String? = null + override suspend fun sendThreadDatasetExportResult( + result: ActivityResult, + serverId: Int, + ): ThreadManager.ExportResult = ThreadManager.ExportResult.NotSent } diff --git a/common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/WebSocketRepository.kt b/common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/WebSocketRepository.kt index 18b7336c3ee..975f7e2b86c 100644 --- a/common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/WebSocketRepository.kt +++ b/common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/WebSocketRepository.kt @@ -83,10 +83,21 @@ interface WebSocketRepository { /** * Add a new set of Thread network credentials to the server. + * + * Note: this command is idempotent for an already-stored TLV and never promotes the dataset + * to the server's preferred one. Use [setThreadPreferredDataset] to explicitly promote. + * * @return `true` if the server indicated success */ suspend fun addThreadDataset(tlv: ByteArray): Boolean + /** + * Set the server's preferred Thread dataset. + * @param datasetId The dataset ID as provided by the server + * @return `true` if the server indicated success + */ + suspend fun setThreadPreferredDataset(datasetId: String): Boolean + /** * Get an Assist response for the given text input. For core >= 2023.5, use [runAssistPipelineForText] * instead. diff --git a/common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketRepositoryImpl.kt b/common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketRepositoryImpl.kt index b3b8f4745aa..6efa1a03883 100644 --- a/common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketRepositoryImpl.kt +++ b/common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketRepositoryImpl.kt @@ -420,6 +420,16 @@ class WebSocketRepositoryImpl internal constructor( return response?.success == true } + override suspend fun setThreadPreferredDataset(datasetId: String): Boolean { + val response = webSocketCore.sendMessage( + mapOf( + "type" to "thread/set_preferred_dataset", + "dataset_id" to datasetId, + ), + ) + return response?.success == true + } + /** * Update server entry in [serverManager] with information from a [CurrentUserResponse] like user * name and admin status. diff --git a/common/src/main/res/values/strings.xml b/common/src/main/res/values/strings.xml index 8f2e214a324..b1b9d75b357 100644 --- a/common/src/main/res/values/strings.xml +++ b/common/src/main/res/values/strings.xml @@ -1170,6 +1170,7 @@ Syncing… An unexpected error occurred while syncing Added network from this device to Home Assistant + Added network from this device to Home Assistant, but Home Assistant still prefers a different network Added network from Home Assistant to this device Home Assistant and this device use the same network Home Assistant and this device prefer different networks