Fix missing home zone on the first location update after high accuracy mode turns off - #7351
Fix missing home zone on the first location update after high accuracy mode turns off#7351eyal0 wants to merge 6 commits into
Conversation
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
Bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR addresses incorrect zone reporting and location-update skipping behavior around geofence/high-accuracy transitions by centralizing zone resolution and refining skip logic.
Changes:
- Added
resolveInZones(...)to compute zone membership from GPS+accuracy and entered geofences, with deterministic sorting. - Added
evaluateLocationUpdateSkip(...)to prevent skipping geofence-triggered updates as duplicates/outdated. - Refactored
LocationSensorManagerto use the new helpers and adjusted the duplicate/skip key to include zones when relevant.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/ZoneAttributes.kt | Adds resolveInZones and distance tie-breaker helper for stable zone resolution. |
| common/src/test/kotlin/io/homeassistant/companion/android/common/data/integration/ZoneAttributesTest.kt | Adds Robolectric tests validating GPS/geofence zone resolution behavior. |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/LocationUpdateSkipEvaluator.kt | Introduces reusable logic for deciding whether to skip a location update. |
| app/src/test/kotlin/io/homeassistant/companion/android/sensors/LocationUpdateSkipEvaluatorTest.kt | Adds unit tests for geofence-related skip edge cases. |
| app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt | Refactors update creation + skipping, incorporates geofence-known zones into dedupe key. |
| app/src/main/res/xml/changelog_master.xml | Documents the user-facing fix for missing home zone after high accuracy mode turns off. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
I just used AI to fix the issue, I don't know really know what I'm doing here. If someone can explain to me how to load a new build based on this code into my phone, I would be able to test if this solves the problem. |
|
I figured out how to compile the app and run it on my phone. I can see that with the new debug image is solving the problem. The first state_changed after precision mode ends and regular mode begins now does have the correct zone. All the subsequent messages which previous had the correct zone continue to have the correct zone. This code does seem to solve the problem. |
Hello, thanks for contributing! The Open Home Foundation's AI policy requires you to review and understand the code you submit. Were you able to review the code since posting this message? If not, I'm afraid we cannot accept it, but will look at your issue report. |
Since writing that comment, I simplified the code a lot and it's short and understandable. It looks right to me but if you've got experts on this part of the code, I'd feel more comfortable having them review it. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:965
- In the
SEND_LOCATION_AS_EXACTbranch this now callsgetZones(serverId)on every location update, even when the server can’t acceptin_zones(core <2026.6.0) or when there are no entered geofences. BecausegetZonescan trigger a network call when the cache is stale, this adds avoidable overhead to the hot path. You can computeenteredZoneIdsdirectly fromlastEnteredGeoZonesand only when the server supportsin_zones.
val enteredZoneIds = getZones(serverId).map { it.entityId }.filter { entityId ->
lastEnteredGeoZones.contains("${serverId}_$entityId")
}
# This is the 1st commit message: Fix missing home zone on the first location update after high accuracy mode turns off # This is the commit message home-assistant#2: Potential fix for pull request finding Use junit without jupiter. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:929
lastEnteredGeoZonesis a mutableArrayListupdated fromsensorWorkerScope(inhandleGeoUpdate) but read here fromioScopeinsendLocationUpdate. This introduces a data race and can lead to inconsistentin_zones/zone-only resolution (and, in worst cases, runtime issues) right where the new behavior relies on it.
Consider making the collection thread-safe (e.g., a concurrent MutableSet) or confining all reads/writes to the same coroutine context and only using immutable snapshots in sendLocationUpdate.
val matchesGps = radius != null && it.containsWithAccuracy(location)
val matchesGeofence = lastEnteredGeoZones.contains("${serverId}_${it.entityId}")
return@filter matchesGps || matchesGeofence
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:1005
- This log line prints
$lastLocationSend(the whole map) instead of the last-send timestamp for the currentserverId, which makes debugging skip decisions difficult. Log the per-server value instead.
if (location.time < (lastLocationSend[serverId] ?: 0) && trigger?.isGeofence != true) {
Timber.d(
"Skipping old location update since time is before the last one we sent, received: ${location.time} last sent: $lastLocationSend",
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:965
- In exact mode, this calls
getZones(serverId)on every location update, even whenin_zonescan’t be sent (core <2026.6.0) or when there are no entered geofences. SincegetZonesmay refresh from the server when the cache is empty/stale, this adds unnecessary I/O and CPU on frequent updates.
Gate the zones lookup behind supportsInZones + lastEnteredGeoZones.isNotEmpty() and reuse the result for inZones.
val enteredZoneIds = getZones(serverId).map { it.entityId }.filter { entityId ->
lastEnteredGeoZones.contains("${serverId}_$entityId")
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:1006
- This log message interpolates
lastLocationSend(the whole map) instead of the last-send timestamp for the current server, which makes debugging harder. LoglastLocationSend[serverId](or similar) instead.
Timber.d(
"Skipping old location update since time is before the last one we sent, received: ${location.time} last sent: $lastLocationSend",
)
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:967
enteredZoneIdscallsgetZones(serverId)on every exact-mode location update for servers >=2026.6.0, even when no geofence is currently entered. SincegetZonesmay trigger a network refresh (and always allocates/maps), short-circuit whenlastEnteredGeoZonesis empty to avoid unnecessary work on the hot path.
val enteredZoneIds = if (serverManager.getServer(serverId)?.version?.isAtLeast(2026, 6, 0) == true) {
getZones(serverId).map { it.entityId }.filter { entityId ->
lastEnteredGeoZones.contains("${serverId}_$entityId")
}
} else {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:978
- In the exact-location branch,
inZonesis set tonullwhenenteredZoneIdsis empty even on servers that supportin_zones. PerUpdateLocation.inZonesKDoc,nullshould be reserved for Core <2026.6.0; for supported servers this should be an empty list to represent “not in any zones”.
val enteredZoneIds = if (lastEnteredGeoZones.isNotEmpty() &&
serverManager.getServer(serverId)?.version?.isAtLeast(2026, 6, 0) == true
) {
getZones(serverId).map { it.entityId }.filter { entityId ->
lastEnteredGeoZones.contains("${serverId}_$entityId")
}
} else {
emptyList()
}
updateLocation = UpdateLocation(
gps = listOf(location.latitude, location.longitude),
gpsAccuracy = accuracy,
locationName = null,
// Send `in_zones` only to versions that support it
// (https://github.com/home-assistant/architecture/discussions/1387).
inZones = enteredZoneIds.takeIf { it.isNotEmpty() },
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:1008
- The log message for skipped old updates prints
lastLocationSend(the entire map) instead of the last-sent timestamp for thisserverId, which makes the log hard to interpret when multiple servers are configured.
Timber.d(
"Skipping old location update since time is before the last one we sent, received: ${location.time} last sent: $lastLocationSend",
)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:967
- In the non-zone-only path,
enteredZoneIdscallsgetZones(serverId)just to translatelastEnteredGeoZonesinto zone entity IDs.getZones()can trigger a network call when the cache is stale (seegetZonesrefresh logic), which makes exact-mode location updates potentially block on I/O and can also yield an empty list on fetch failure (clearingin_zoneseven though geofencing state is known locally). Since geofence requestIds already include the full zone entityId (prefixed with${serverId}_), deriveenteredZoneIdsdirectly fromlastEnteredGeoZoneswithout fetching zones.
val enteredZoneIds = if (serverManager.getServer(serverId)?.version?.isAtLeast(2026, 6, 0) == true) {
if (lastEnteredGeoZones.isEmpty()) {
emptyList()
} else {
getZones(serverId).map { it.entityId }.filter { entityId ->
|
I'm doing some refactoring to make it more uniform. |
|
The calculation of inZones is moved out of the branch body so that it can be used in both forks of the branch. I have tested that this solves the problem. To test, go to Settings -> Tools -> Events and listen to event |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:923
getZones(serverId)+ filtering/sorting zones is now done on every location update, even when it can't affect the payload (e.g., exact-location updates against Core < 2026.6.0 whereinZonesmust be null). In high-accuracy mode this runs every few seconds and can become a noticeable CPU cost with many zones. Consider gating zone resolution behind either zone-only mode orsupportsInZones.
val updateLocationAs: String = getSendLocationAsSetting(serverId)
val zones = getZones(serverId)
val enteredZones = zones
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:928
lastEnteredGeoZonesis a mutableArrayListthat is written inhandleGeoUpdate(...)and read here during zone resolution. This cross-dispatcher access can race (including rareArrayIndexOutOfBoundsExceptionduring growth) and also makescontains(...)results non-deterministic under concurrent updates. Consider switching it to a thread-safe set (e.g.,CopyOnWriteArraySet/ConcurrentHashMap.newKeySet()) or guarding reads/writes with a coroutineMutex.
val radius = it.attributes["radius"] as? Number
val matchesGps = radius != null && it.containsWithAccuracy(location)
val matchesGeofence = lastEnteredGeoZones.contains("${serverId}_${it.entityId}")
return@filter matchesGps || matchesGeofence
app/src/full/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt:966
inZonesis now included in the exact-location payload, but duplicate detection still keys offupdateLocationString = updateLocation.gps.toString(). If zone membership changes without a coordinate change (possible when accuracy changes or when relying on geofence state), the update can be incorrectly suppressed as a duplicate.
updateLocation = UpdateLocation(
gps = listOf(location.latitude, location.longitude),
gpsAccuracy = accuracy,
locationName = null,
inZones = inZones,
speed = location.speed.toInt(),
This fixes #7349
Summary
Checklist
Select exactly one option that describes AI usage in this contribution:
Screenshots
Link to pull request in documentation repositories
User Documentation: home-assistant/companion.home-assistant#
Developer Documentation: home-assistant/developers.home-assistant#
Any other notes