Verify device's preferred Thread dataset after import#6819
Draft
agners wants to merge 2 commits intohome-assistant:mainfrom
Draft
Verify device's preferred Thread dataset after import#6819agners wants to merge 2 commits intohome-assistant:mainfrom
agners wants to merge 2 commits intohome-assistant:mainfrom
Conversation
ThreadNetworkClient.addCredentials only stores a credential under the given border agent; it does not expose any way to set the preferred dataset, and Play Services may keep preferring an older app-added credential. The full sync's update branch was treating a successful addCredentials as "device updated" and reporting success, even when the device's preferred dataset was unchanged. The next sync detected the same mismatch and looped. After importDatasetFromServer succeeds, query isPreferredCredentials for the just-imported TLV and propagate the result via a new deviceNowPrefersCore field on AllHaveCredentials. The developer settings screen shows a dedicated message when the import didn't take effect so the user knows the loop won't resolve itself. Also fix a cosmetic log issue: the "device prefers app added dataset" line was printing the extended PAN ID via String(ByteArray), which emits control characters. Format it as upper-case hex instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the Thread credential “sync preferred dataset” flow by verifying whether Google Play Services actually switched the device’s preferred Thread dataset after importing Home Assistant Core’s preferred dataset, and reporting the outcome accurately in Developer Settings.
Changes:
- Add
deviceNowPrefersCore: Boolean?toThreadManager.SyncResult.AllHaveCredentialsto surface post-import preferred-dataset verification. - Update Developer Settings messaging to distinguish “updated” vs “updated but still not preferred” and mark the latter as non-success.
- Fix Thread debug logging to format the extended PAN ID as uppercase hex (instead of
String(ByteArray)control characters).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| common/src/main/res/values/strings.xml | Adds a new debug string for the “updated but still not preferred” state |
| app/src/main/kotlin/io/homeassistant/companion/android/thread/ThreadManager.kt | Extends AllHaveCredentials result with deviceNowPrefersCore |
| app/src/main/kotlin/io/homeassistant/companion/android/settings/developer/DeveloperSettingsPresenterImpl.kt | Uses deviceNowPrefersCore to choose message and success indicator |
| app/src/full/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt | Verifies preferred dataset after import; improves logging of EXTPAN |
Comment on lines
+176
to
+180
| deviceNowPrefersCore = try { | ||
| isPreferredDatasetByDevice(context, coreThreadDataset.datasetId, serverId) | ||
| } catch (e: Exception) { | ||
| Timber.w(e, "Unable to verify preferred dataset after import") | ||
| null |
…essage When the device still prefers a different network after import, look up which app-added credential is currently preferred and surface its network name so the user can identify the lingering credential. Refactor appAddedIsPreferredCredentials into appAddedPreferredCredential so the preferred credential is reused for both the boolean check and the network-name lookup, and propagate the name via a new devicePreferredNetworkName field on AllHaveCredentials. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After a Thread sync with
coreIsDevicePreferred == false && appIsDevicePreferred == true(i.e. the device prefers an app-added credential that doesn't match HA's preferred dataset),fullSyncPreferredDatasetdeletes mismatched border-agent IDs and re-imports HA's preferred TLV viaThreadNetworkClient.addCredentials. The path then unconditionally returnedupdated = trueand the developer settings screen reported "Updated network from Home Assistant on this device".ThreadNetworkClient.addCredentialsonly stores a credential under the supplied border agent — the Google Play Services Thread API does not expose any way to set the preferred dataset, and Play Services often keeps preferring the older app-added credential. As a result the device's preferred dataset was unchanged, the next sync re-detected the same mismatch, ran the same delete-and-import again, and reported success again. The user-visible outcome was an apparently successful sync that looped indefinitely without actually changing what Play Services preferred.This PR makes the report honest:
importDatasetFromServersucceeds,isPreferredCredentialsis queried for the just-imported TLV.deviceNowPrefersCore: Boolean?field onSyncResult.AllHaveCredentials.thread_debug_result_updated_not_preferred) when the device still prefers a different network, with a non-success indicator, so the user knows that another sync won't resolve the state.It also fixes a cosmetic log issue surfaced while debugging this: the "device prefers app added dataset" line was printing the extended PAN ID via
String(ByteArray), which produces control characters. It now formats the EXTPAN as upper-case hex.This is a companion to #6818, which addresses the symmetric problem on the server side (
thread/add_dataset_tlvdoes not promote a dataset to preferred either). Both stem from the same underlying issue: neither the server-side WS command (since home-assistant/core#108278) nor the Play ServicesaddCredentialscall promote the just-added dataset, so the app must verify and report rather than assume.A deeper follow-up is still needed on the device side:
getThreadBorderAgentIds()is replaced (not appended) on every successful import, so border-agent IDs HA used in earlier sessions are no longer tracked and the cleanup loop won't remove them. IteratingThreadNetworkClient.allCredentialsand removing every owned credential whose BA doesn't match the current core preferred would address that, but it's a more invasive change and is intentionally not part of this PR.Checklist
Screenshots
Link to pull request in documentation repositories
Any other notes
ThreadManager.SyncResult.AllHaveCredentialsgains a newdeviceNowPrefersCoreconstructor parameter. All in-tree callers are updated; the change is contained in the app module.