[astro] Discovery service with constructor injection (openhab#7850)

* [astro] Discovery service with constructor injection Null annotations added. * Call activate in the constructor Signed-off-by: Laurent Garnier <[email protected]>
andrewfg · Aug 31, 2020 · 2a5ba74 · 2a5ba74
1 parent b2a6444
commit 2a5ba74
Showing 1 changed file with 21 additions and 59 deletions.
diff --git a/...tro/src/main/java/org/openhab/binding/astro/internal/discovery/AstroDiscoveryService.java b/...tro/src/main/java/org/openhab/binding/astro/internal/discovery/AstroDiscoveryService.java
@@ -21,6 +21,8 @@
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.smarthome.config.discovery.AbstractDiscoveryService;
 import org.eclipse.smarthome.config.discovery.DiscoveryResultBuilder;
 import org.eclipse.smarthome.config.discovery.DiscoveryService;
@@ -32,8 +34,6 @@
 import org.eclipse.smarthome.core.thing.ThingUID;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
-import org.osgi.service.component.annotations.Deactivate;
-import org.osgi.service.component.annotations.Modified;
 import org.osgi.service.component.annotations.Reference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -44,41 +44,31 @@
  * @author Gerhard Riegler - Initial Contribution
  * @author Stefan Triller - Use configured location
  */
+@NonNullByDefault
 @Component(service = DiscoveryService.class, immediate = true, configurationPid = "discovery.astro")
 public class AstroDiscoveryService extends AbstractDiscoveryService {
-    private final Logger logger = LoggerFactory.getLogger(AstroDiscoveryService.class);
     private static final int DISCOVER_TIMEOUT_SECONDS = 2;
     private static final int LOCATION_CHANGED_CHECK_INTERVAL = 60;
-    private LocationProvider locationProvider;
-    private ScheduledFuture<?> astroDiscoveryJob;
-    private PointType previousLocation;
 
     private static final ThingUID SUN_THING = new ThingUID(THING_TYPE_SUN, LOCAL);
     private static final ThingUID MOON_THING = new ThingUID(THING_TYPE_MOON, LOCAL);
 
-    /**
-     * Creates a AstroDiscoveryService with enabled autostart.
-     */
-    public AstroDiscoveryService() {
-        super(new HashSet<>(Arrays.asList(new ThingTypeUID(BINDING_ID, "-"))), DISCOVER_TIMEOUT_SECONDS, true);
-    }
+    private final Logger logger = LoggerFactory.getLogger(AstroDiscoveryService.class);
 
-    @Override
-    @Activate
-    protected void activate(Map<String, Object> configProperties) {
-        super.activate(configProperties);
-    }
+    private final LocationProvider locationProvider;
 
-    @Override
-    @Modified
-    protected void modified(Map<String, Object> configProperties) {
-        super.modified(configProperties);
-    }
+    private @Nullable ScheduledFuture<?> astroDiscoveryJob;
+    private @Nullable PointType previousLocation;
 
-    @Override
-    @Deactivate
-    protected void deactivate() {
-        super.deactivate();
+    @Activate
+    public AstroDiscoveryService(final @Reference LocationProvider locationProvider,
+            final @Reference LocaleProvider localeProvider, final @Reference TranslationProvider i18nProvider,
+            @Nullable Map<String, @Nullable Object> configProperties) {
+        super(new HashSet<>(Arrays.asList(new ThingTypeUID(BINDING_ID, "-"))), DISCOVER_TIMEOUT_SECONDS, true);
+        this.locationProvider = locationProvider;
+        this.localeProvider = localeProvider;
+        this.i18nProvider = i18nProvider;
+        activate(configProperties);
     }
 
     @Override
@@ -97,7 +87,7 @@ protected void startBackgroundDiscovery() {
         if (astroDiscoveryJob == null) {
             astroDiscoveryJob = scheduler.scheduleWithFixedDelay(() -> {
                 PointType currentLocation = locationProvider.getLocation();
-                if (!Objects.equals(currentLocation, previousLocation)) {
+                if (currentLocation != null && !Objects.equals(currentLocation, previousLocation)) {
                     logger.debug("Location has been changed from {} to {}: Creating new discovery results",
                             previousLocation, currentLocation);
                     createResults(currentLocation);
@@ -111,12 +101,11 @@ protected void startBackgroundDiscovery() {
     @Override
     protected void stopBackgroundDiscovery() {
         logger.debug("Stopping Astro device background discovery");
-        if (astroDiscoveryJob != null && !astroDiscoveryJob.isCancelled()) {
-            if (astroDiscoveryJob.cancel(true)) {
-                astroDiscoveryJob = null;
-                logger.debug("Stopped Astro device background discovery");
-            }
+        ScheduledFuture<?> discoveryJob = astroDiscoveryJob;
+        if (discoveryJob != null) {
+            discoveryJob.cancel(true);
         }
+        astroDiscoveryJob = null;
     }
 
     public void createResults(PointType location) {
@@ -128,31 +117,4 @@ public void createResults(PointType location) {
         thingDiscovered(DiscoveryResultBuilder.create(MOON_THING).withLabel("Local Moon")
                 .withProperty("geolocation", propGeolocation).withRepresentationProperty("geolocation").build());
     }
-
-    @Reference
-    protected void setLocationProvider(LocationProvider locationProvider) {
-        this.locationProvider = locationProvider;
-    }
-
-    protected void unsetLocationProvider(LocationProvider locationProvider) {
-        this.locationProvider = null;
-    }
-
-    @Reference
-    protected void setLocaleProvider(final LocaleProvider localeProvider) {
-        this.localeProvider = localeProvider;
-    }
-
-    protected void unsetLocaleProvider(final LocaleProvider localeProvider) {
-        this.localeProvider = null;
-    }
-
-    @Reference
-    protected void setTranslationProvider(TranslationProvider i18nProvider) {
-        this.i18nProvider = i18nProvider;
-    }
-
-    protected void unsetTranslationProvider(TranslationProvider i18nProvider) {
-        this.i18nProvider = null;
-    }
 }