various

- remove unnecessary handler initialized tests
- resubscribe event if channel linked
- refactoring

Signed-off-by: Andrew Fiddian-Green <[email protected]>

Author: andrewfg

bundles/org.openhab.binding.homekit/src/main/java/org/openhab/binding/homekit/internal/handler/HomekitAccessoryHandler.java

@@ -70,7 +70,6 @@
 import org.openhab.core.thing.type.ChannelType;
 import org.openhab.core.thing.type.ChannelTypeRegistry;
 import org.openhab.core.thing.type.ChannelTypeUID;
-import org.openhab.core.thing.util.ThingHandlerHelper;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.RefreshType;
 import org.openhab.core.types.State;
@@ -906,11 +905,6 @@ protected IpTransport getIpTransport() throws IllegalAccessException {
         return super.getIpTransport();
     }
 
-    @Override
-    protected boolean bridgedThingsInitialized() {
-        return ThingHandlerHelper.isHandlerInitialized(thing); // no bridged accessories; return own status
-    }
-
     @Override
     protected void onConnectedThingAccessoriesLoaded() {
         createProperties();
@@ -928,7 +922,11 @@ public void onEvent(String json) {
      */
     @Override
     public void channelLinked(ChannelUID channelUID) {
+        boolean eventedCharacteristicsChanged = false;
         try {
+            if (!alreadyAtStartLevelComplete()) {
+                return; // item-channel-links not yet fully initialized
+            }
             final Channel channel = thing.getChannel(channelUID);
             if (channel == null) {
                 return; // OH core ensures this does not happen
@@ -969,6 +967,7 @@ public void channelLinked(ChannelUID channelUID) {
                             entry.aid = aid;
                             entry.iid = iid;
                             eventedCharacteristics.put(AID_IID_FORMAT.formatted(entry.aid, entry.iid), entry);
+                            eventedCharacteristicsChanged = true;
                         }
                         if (checkChannelLinkByIID) {
                             return; // unique match found; return directly
@@ -977,6 +976,9 @@ public void channelLinked(ChannelUID channelUID) {
                 }
             }
         } finally {
+            if (eventedCharacteristicsChanged) { // if evented list changes (re-) enable eventing (using new list)
+                scheduler.submit(() -> enableEvents(true));
+            }
             super.channelLinked(channelUID);
         }
     }

bundles/org.openhab.binding.homekit/src/main/java/org/openhab/binding/homekit/internal/handler/HomekitBaseAccessoryHandler.java

@@ -243,9 +243,6 @@ private void fetchAccessories() {
      * 'onThingOnline' methods (and its eventual overloaded implementations).
      */
     private void processBridgedThings() {
-        if (!bridgedThingsInitialized()) {
-            logger.warn("{} unexpected error: bridged Things not initialized.", thing.getUID());
-        }
         onConnectedThingAccessoriesLoaded();
         onThingOnline();
     }
@@ -291,10 +288,9 @@ public void initialize() {
             if (alreadyAtStartLevelComplete()) {
                 // schedule connection attempt immediately
                 scheduleConnectionAttempt();
-            } else {
+            } else if (bundle.getBundleContext() instanceof BundleContext ctx) {
                 // delay connection attempt until STARTLEVEL_COMPLETE is signalled via receive() method below
-                BundleContext context = bundle.getBundleContext();
-                eventSubscription = context.registerService(EventSubscriber.class.getName(), this, null);
+                eventSubscription = ctx.registerService(EventSubscriber.class.getName(), this, null);
             }
         }
         updateStatus(ThingStatus.UNKNOWN);
@@ -306,14 +302,16 @@ public void initialize() {
      * Note: STARTLEVEL_COMPLETE means all Thing handlers are instantiated and their initialize() methods have
      * been called, and the registries for item, thing, and item-channel-links have all been loaded.
      */
-    private boolean alreadyAtStartLevelComplete() {
-        BundleContext context = bundle.getBundleContext();
-        ServiceReference<StartLevelService> reference = context.getServiceReference(StartLevelService.class);
-        if (reference != null && context.getService(reference) instanceof StartLevelService service) {
-            try {
-                return service.getStartLevel() >= StartLevelService.STARTLEVEL_COMPLETE;
-            } finally {
-                context.ungetService(reference);
+    protected boolean alreadyAtStartLevelComplete() {
+        if (bundle.getBundleContext() instanceof BundleContext ctx) {
+            if (ctx.getServiceReference(StartLevelService.class) instanceof ServiceReference<StartLevelService> ref) {
+                if (ctx.getService(ref) instanceof StartLevelService svc) {
+                    try {
+                        return svc.getStartLevel() >= StartLevelService.STARTLEVEL_COMPLETE;
+                    } finally {
+                        ctx.ungetService(ref);
+                    }
+                }
             }
         }
         return false;
@@ -416,12 +414,11 @@ private String normalizePairingCode(String input) throws IllegalArgumentExceptio
     protected void scheduleConnectionAttempt() {
         if (getBridge() instanceof Bridge bridge && bridge.getHandler() instanceof HomekitBridgeHandler bridgeHandler) {
             bridgeHandler.scheduleConnectionAttempt();
-        } else {
-            ScheduledFuture<?> task = connectionAttemptTask;
-            if (task == null || task.isDone() || task.isCancelled()) {
-                connectionAttemptTask = scheduler.schedule(this::attemptConnect, connectionAttemptDelay,
-                        TimeUnit.SECONDS);
-            }
+            return;
+        }
+        ScheduledFuture<?> task = connectionAttemptTask;
+        if (task == null || task.isDone() || task.isCancelled()) {
+            connectionAttemptTask = scheduler.schedule(this::attemptConnect, connectionAttemptDelay, TimeUnit.SECONDS);
         }
     }
 
@@ -687,7 +684,11 @@ protected void createProperties() {
      *
      * @param enable true to enable events, false to disable
      */
-    private void enableEvents(boolean enable) {
+    protected void enableEvents(boolean enable) {
+        if (getBridge() instanceof Bridge bridge && bridge.getHandler() instanceof HomekitBridgeHandler bridgeHandler) {
+            bridgeHandler.enableEvents(true);
+            return;
+        }
         try {
             enableEventsOrThrow(enable);
         } catch (InterruptedException e) {
@@ -772,12 +773,6 @@ private synchronized void refresh() {
         }
     }
 
-    /**
-     * Checks if all bridged accessory things have the reached status UNKNOWN, OFFLINE, or ONLINE.
-     * Subclasses MUST override this to perform the check.
-     */
-    protected abstract boolean bridgedThingsInitialized();
-
     /**
      * Called when the connected thing has finished loading the accessories.
      * Subclasses MUST override this to perform any extra processing required.
@@ -863,11 +858,11 @@ private void cancelRefreshTasks() {
     protected void requestManualRefresh() {
         if (getBridge() instanceof Bridge bridge && bridge.getHandler() instanceof HomekitBridgeHandler bridgeHandler) {
             bridgeHandler.requestManualRefresh();
-        } else {
-            Future<?> task = manualRefreshTask;
-            if (task == null || task.isDone() || task.isCancelled()) {
-                manualRefreshTask = scheduler.schedule(this::refresh, MANUAL_REFRESH_DELAY_SECONDS, TimeUnit.SECONDS);
-            }
+            return;
+        }
+        Future<?> task = manualRefreshTask;
+        if (task == null || task.isDone() || task.isCancelled()) {
+            manualRefreshTask = scheduler.schedule(this::refresh, MANUAL_REFRESH_DELAY_SECONDS, TimeUnit.SECONDS);
         }
     }
 

bundles/org.openhab.binding.homekit/src/main/java/org/openhab/binding/homekit/internal/handler/HomekitBridgeHandler.java

@@ -32,7 +32,6 @@
 import org.openhab.core.thing.binding.ThingHandler;
 import org.openhab.core.thing.binding.ThingHandlerService;
 import org.openhab.core.thing.binding.builder.BridgeBuilder;
-import org.openhab.core.thing.util.ThingHandlerHelper;
 import org.openhab.core.types.Command;
 import org.osgi.framework.Bundle;
 
@@ -92,12 +91,6 @@ public void childHandlerDisposed(ThingHandler childHandler, Thing childThing) {
         // do nothing
     }
 
-    @Override
-    protected boolean bridgedThingsInitialized() {
-        return getThing().getThings().stream()
-                .allMatch(bridgedAccessory -> ThingHandlerHelper.isHandlerInitialized(bridgedAccessory));
-    }
-
     @Override
     protected void onConnectedThingAccessoriesLoaded() {
         createProperties();