[unifi] Handle Protect/Access Thing Lifecycle correctly (#21346)

Signed-off-by: Dan Cunningham <dan@digitaldan.com>

Author: digitaldan

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/access/api/UnifiAccessApiClient.java

@@ -158,6 +158,23 @@ public synchronized void close() {
     private volatile long bootstrapCacheTimeMs;
     private static final long BOOTSTRAP_CACHE_TTL_MS = 30_000; // 30 seconds
 
+    /**
+     * True when the topology contains the floors section the doors/devices lists derive from;
+     * an absent section is an incomplete response, not an empty site.
+     */
+    public boolean isTopologyAuthoritative() throws UnifiAccessApiException {
+        JsonElement floors = getBootstrap().get("floors");
+        return floors != null && floors.isJsonArray();
+    }
+
+    /**
+     * Drop the cached bootstrap so the next sync fetches fresh topology, e.g. after a WebSocket
+     * event reports a device was deleted.
+     */
+    public synchronized void invalidateBootstrapCache() {
+        this.cachedBootstrap = null;
+    }
+
     /**
      * Fetches the bootstrap topology from the v2 API.
      * Caches the result for 30 seconds to avoid redundant calls during a sync cycle.

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/access/handler/UnifiAccessBaseHandler.java

@@ -73,6 +73,16 @@ void setOnline() {
         updateStatus(ThingStatus.ONLINE);
     }
 
+    /**
+     * Clears a GONE status when an authoritative topology proves the device exists again;
+     * connectivity handling then refines UNKNOWN to ONLINE/OFFLINE as information arrives.
+     */
+    void clearGone() {
+        if (getThing().getStatusInfo().getStatusDetail() == ThingStatusDetail.GONE) {
+            updateStatus(ThingStatus.UNKNOWN);
+        }
+    }
+
     protected void refreshState(String channelId) {
         State state = stateCache.get(channelId);
         if (state != null) {
@@ -97,17 +107,28 @@ protected void refreshState(String channelId) {
     protected void handleDeviceUpdate(DeviceUpdateData updateData) {
         updateState(UnifiAccessBindingConstants.CHANNEL_DEVICE_ONLINE,
                 updateData.isConnected ? OnOffType.ON : OnOffType.OFF);
-        if (!updateData.isConnected) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR,
-                    "@text/offline.device-offline");
-        }
+        applyOnlineState(updateData.isConnected);
     }
 
     protected void handleDeviceUpdateV2(Notification.DeviceUpdateV2Data updateData) {
+        // A partial update without the online field says nothing about connectivity
         Boolean online = updateData.online;
-        updateState(UnifiAccessBindingConstants.CHANNEL_DEVICE_ONLINE,
-                Boolean.TRUE.equals(online) ? OnOffType.ON : OnOffType.OFF);
-        if (!Boolean.TRUE.equals(online)) {
+        if (online == null) {
+            return;
+        }
+        updateState(UnifiAccessBindingConstants.CHANNEL_DEVICE_ONLINE, online ? OnOffType.ON : OnOffType.OFF);
+        applyOnlineState(online);
+    }
+
+    private void applyOnlineState(boolean online) {
+        if (getThing().getStatusInfo().getStatusDetail() == ThingStatusDetail.GONE) {
+            return;
+        }
+        if (online) {
+            if (getThing().getStatus() != ThingStatus.ONLINE) {
+                updateStatus(ThingStatus.ONLINE);
+            }
+        } else {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR,
                     "@text/offline.device-offline");
         }

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/access/handler/UnifiAccessBridgeHandler.java

@@ -13,8 +13,11 @@
 package org.openhab.binding.unifi.internal.access.handler;
 
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.RejectedExecutionException;
@@ -363,11 +366,21 @@ synchronized void syncDevices() {
                 discoveryService.discoverDevices(discoveryDevices);
             }
             logger.trace("Polled UniFi Access: {} doors, {} devices", doors.size(), devices.size());
+            Map<String, Device> devicesById = new HashMap<>();
+            for (Device device : devices) {
+                String id = device.id;
+                if (id != null) {
+                    devicesById.put(id, device);
+                }
+            }
             for (Door door : doors) {
                 logger.trace("Checking door: {}", door.id);
                 UnifiAccessDoorHandler dh = getDoorHandler(door.id);
                 if (dh != null) {
                     logger.trace("Updating door: {}", dh.deviceId);
+                    // Presence in the topology clears GONE; connectivity is judged separately
+                    dh.clearGone();
+                    setDoorStatus(dh, door, devices, devicesById);
                     dh.updateFromDoor(door);
                 }
             }
@@ -380,15 +393,20 @@ synchronized void syncDevices() {
                 if (dh != null) {
                     logger.debug("Syncing device {} (type={}, online={}, locationId={})", device.id, device.type,
                             device.isOnline, device.locationId);
-                    // Set online/offline based on device status, independent of settings
-                    boolean online = !Boolean.FALSE.equals(device.isOnline);
-                    dh.updateState(UnifiAccessBindingConstants.CHANNEL_DEVICE_ONLINE,
-                            online ? OnOffType.ON : OnOffType.OFF);
-                    if (!online) {
-                        dh.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR,
-                                "@text/offline.device-offline");
-                    } else if (dh.getThing().getStatus() != ThingStatus.ONLINE) {
-                        dh.setOnline();
+                    // Presence in the topology clears GONE; connectivity is judged separately
+                    dh.clearGone();
+                    // Set online/offline based on device status, independent of settings;
+                    // null means the API reported nothing, so the status is left unchanged
+                    Boolean online = device.isOnline;
+                    if (online != null) {
+                        dh.updateState(UnifiAccessBindingConstants.CHANNEL_DEVICE_ONLINE,
+                                online ? OnOffType.ON : OnOffType.OFF);
+                        if (!online) {
+                            dh.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR,
+                                    "@text/offline.device-offline");
+                        } else if (dh.getThing().getStatus() != ThingStatus.ONLINE) {
+                            dh.setOnline();
+                        }
                     }
                     // Update thing properties from device metadata
                     dh.updateDeviceProperties(device);
@@ -405,6 +423,12 @@ synchronized void syncDevices() {
                     }
                 }
             }
+            // An incomplete topology response must not mark things GONE
+            if (client.isTopologyAuthoritative()) {
+                markMissingChildrenGone(doors, devices);
+            } else {
+                logger.debug("Topology response incomplete, skipping gone reconciliation");
+            }
         } catch (UnifiAccessApiException e) {
             logger.debug("Polling error: {}", e.getMessage());
             if (e.getAuthState() == AuthState.REJECTED) {
@@ -416,6 +440,82 @@ synchronized void syncDevices() {
         }
     }
 
+    /**
+     * A door is only readable and commandable through its hub, so its status follows the hub:
+     * hub offline -> door offline, no hub bound at all -> the door can neither report state nor
+     * take commands, and a hub we cannot resolve -> leave the status alone.
+     */
+    private void setDoorStatus(UnifiAccessDoorHandler dh, Door door, List<Device> devices,
+            Map<String, Device> devicesById) {
+        Device hub = door.hubDeviceId != null ? devicesById.get(door.hubDeviceId) : null;
+        if (hub == null) {
+            hub = devices.stream().filter(d -> d.isHub() && door.id != null && door.id.equals(d.locationId)).findFirst()
+                    .orElse(null);
+        }
+        var info = dh.getThing().getStatusInfo();
+        if (hub != null) {
+            if (Boolean.FALSE.equals(hub.isOnline)) {
+                // Compare detail too, so a stale GONE or no hub message is corrected
+                if (info.getStatus() != ThingStatus.OFFLINE
+                        || info.getStatusDetail() != ThingStatusDetail.COMMUNICATION_ERROR) {
+                    dh.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                            "@text/offline.hub-offline");
+                }
+            } else if (Boolean.TRUE.equals(hub.isOnline)) {
+                if (info.getStatus() != ThingStatus.ONLINE) {
+                    dh.setOnline();
+                }
+            }
+            // isOnline == null: the topology omitted the hub's connectivity, say nothing
+        } else if (door.doorLockRelayStatus != null || door.doorPositionStatus != null) {
+            // No resolvable hub, but lock/position state is populated so something serves the
+            // door (e.g. a building level hub extension we can't link to a device)
+            if (info.getStatus() != ThingStatus.ONLINE) {
+                dh.setOnline();
+            }
+        } else if (Boolean.TRUE.equals(door.isBindHub)) {
+            // Devices exist (isBindHub is derived from a non-empty device group) but none
+            // resolve as the hub and no state is reported; don't guess either way
+            logger.debug("Door {} has devices but no resolvable hub; leaving status unchanged", door.id);
+        } else if (info.getStatusDetail() != ThingStatusDetail.CONFIGURATION_ERROR) {
+            // No devices and no state at all so the door can't report anything or take commands
+            dh.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "@text/offline.no-hub");
+        }
+    }
+
+    /**
+     * Mark child things GONE when their door/device is absent from the topology — removed from
+     * Access, so no update will ever arrive for it again.
+     */
+    private void markMissingChildrenGone(List<Door> doors, List<Device> devices) {
+        Set<String> doorIds = new HashSet<>();
+        doors.forEach(d -> {
+            String id = d.id;
+            if (id != null) {
+                doorIds.add(id);
+            }
+        });
+        Set<String> deviceIds = new HashSet<>();
+        devices.forEach(d -> {
+            String id = d.id;
+            if (id != null) {
+                deviceIds.add(id);
+            }
+        });
+        for (Thing child : getThing().getThings()) {
+            if (!(child.getHandler() instanceof UnifiAccessBaseHandler handler) || handler.deviceId.isEmpty()) {
+                continue;
+            }
+            Set<String> knownIds = handler instanceof UnifiAccessDoorHandler ? doorIds : deviceIds;
+            if (knownIds.contains(handler.deviceId)
+                    || child.getStatusInfo().getStatusDetail() == ThingStatusDetail.GONE) {
+                continue;
+            }
+            logger.debug("Device {} not present on controller, marking {} gone", handler.deviceId, child.getUID());
+            handler.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.GONE, "@text/offline.access-gone");
+        }
+    }
+
     public @Nullable UnifiAccessApiClient getApiClient() {
         return apiClient;
     }

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/access/handler/UnifiAccessDeviceHandler.java

@@ -127,9 +127,8 @@ protected void handleLocationState(LocationState locationState) {
     }
 
     protected void updateFromSettings(DeviceAccessMethodSettings settings) {
-        if (getThing().getStatus() != ThingStatus.ONLINE) {
-            updateStatus(ThingStatus.ONLINE);
-        }
+        // No status change here: the bridge poll sets ONLINE/OFFLINE from the device's reported
+        // isOnline, and settings arrive for offline devices too
         updateEnabledChannel(CHANNEL_DEVICE_NFC_ENABLED, settings.nfc);
         updateEnabledChannel(CHANNEL_DEVICE_PIN_ENABLED, settings.pinCode);
         updateEnabledChannel(CHANNEL_DEVICE_FACE_ENABLED, settings.face);

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/access/handler/UnifiAccessDoorHandler.java

@@ -226,16 +226,19 @@ protected void handleDeviceUpdateV2(Notification.DeviceUpdateV2Data updateData)
             updateData.locationStates.stream().filter(locationState -> locationState.locationId.equals(door.id))
                     .findFirst().ifPresent(this::handleLocationState);
         }
-        super.handleDeviceUpdateV2(updateData);
+        // A door's online status follows its hub only; these updates are routed by location, so
+        // another device at this door (e.g. a reader) must not flip the door's status
+        String hubDeviceId = door.hubDeviceId;
+        if (hubDeviceId != null && hubDeviceId.equals(updateData.id)) {
+            super.handleDeviceUpdateV2(updateData);
+        }
     }
 
     // Door specific update methods
     protected void updateFromDoor(Door door) {
         logger.debug("Updating door state from door: {}", door);
         this.door = door;
-        if (getThing().getStatus() != ThingStatus.ONLINE) {
-            updateStatus(ThingStatus.ONLINE);
-        }
+        // No status change here: the bridge poll sets ONLINE/OFFLINE from the door's hub state
         // Enrich thing properties
         Map<String, String> properties = new HashMap<>(editProperties());
         if (door.fullName != null) {

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/access/handler/UnifiAccessNotificationRouter.java

@@ -298,10 +298,16 @@ private void handleDeviceUpdateV2(DeviceUpdateV2Data updateData) {
     private void handleDeviceDeleteEvent(Notification notification) {
         String deviceId = notification.eventObjectId;
         if (deviceId != null) {
+            // The topology changed; a cached bootstrap would resurrect the deleted device on
+            // the next sync within the cache TTL
+            var apiClient = bridgeHandler.getApiClient();
+            if (apiClient != null) {
+                apiClient.invalidateBootstrapCache();
+            }
             UnifiAccessBaseHandler bh = bridgeHandler.getBaseHandler(deviceId);
             if (bh != null) {
                 bh.updateStatus(org.openhab.core.thing.ThingStatus.OFFLINE,
-                        org.openhab.core.thing.ThingStatusDetail.GONE, "Device removed from controller");
+                        org.openhab.core.thing.ThingStatusDetail.GONE, "@text/offline.access-gone");
             }
         }
     }

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/protect/api/priv/client/UniFiProtectPrivateClient.java

@@ -185,6 +185,14 @@ public CompletableFuture<Bootstrap> getBootstrap() {
         });
     }
 
+    /**
+     * Drop the cached bootstrap so the next fetch is fresh — used when a WebSocket remove
+     * reports a topology change the cached copy cannot reflect.
+     */
+    public void invalidateBootstrap() {
+        lastBootstrapRefresh = null;
+    }
+
     /**
      * Force refresh of bootstrap
      */