From 28965607ad0f19f1fad2181678e43eed80a8e9d5 Mon Sep 17 00:00:00 2001 From: Laurent Garnier Date: Wed, 25 Sep 2024 20:13:54 +0200 Subject: [PATCH 1/2] Add a new optional input parameter to discovery services Related to #4388 Signed-off-by: Laurent Garnier --- .../discovery/AbstractDiscoveryService.java | 72 ++++++++++++++++-- .../AbstractThingHandlerDiscoveryService.java | 11 ++- .../config/discovery/DiscoveryService.java | 40 ++++++++++ .../discovery/DiscoveryServiceRegistry.java | 15 +++- .../DiscoveryServiceRegistryImpl.java | 28 ++++--- .../DiscoveryConsoleCommandExtension.java | 16 ++-- .../AbstractDiscoveryServiceTest.java | 46 +++++++++++- .../rest/core/discovery/DiscoveryInfoDTO.java | 35 +++++++++ .../internal/discovery/DiscoveryResource.java | 74 ++++++++++++++++++- .../DiscoveryServiceRegistryOSGiTest.java | 41 ++++++---- 10 files changed, 332 insertions(+), 46 deletions(-) create mode 100644 bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/discovery/DiscoveryInfoDTO.java diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractDiscoveryService.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractDiscoveryService.java index 0f980bdb6e7..c9a52d4d72f 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractDiscoveryService.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractDiscoveryService.java @@ -49,6 +49,7 @@ * @author Kai Kreuzer - Refactored API * @author Dennis Nobel - Added background discovery configuration through Configuration Admin * @author Andre Fuechsel - Added removeOlderResults + * @author Laurent Garnier - Added discovery with an optional input parameter */ @NonNullByDefault public abstract class AbstractDiscoveryService implements DiscoveryService { @@ -64,6 +65,9 @@ public abstract class AbstractDiscoveryService implements DiscoveryService { private boolean backgroundDiscoveryEnabled; + private @Nullable String scanInputLabel; + private @Nullable String scanInputDescription; + private final Map cachedResults = new HashMap<>(); private final Set supportedThingTypes; @@ -84,20 +88,44 @@ public abstract class AbstractDiscoveryService implements DiscoveryService { * service automatically stops its forced discovery process (>= 0). * @param backgroundDiscoveryEnabledByDefault defines, whether the default for this discovery service is to * enable background discovery or not. + * @param scanInputLabel the label of the optional input parameter to start the discovery or null if no input + * parameter supported + * @param scanInputDescription the description of the optional input parameter to start the discovery or null if no + * input parameter supported * @throws IllegalArgumentException if {@code timeout < 0} */ protected AbstractDiscoveryService(@Nullable Set supportedThingTypes, int timeout, - boolean backgroundDiscoveryEnabledByDefault) throws IllegalArgumentException { + boolean backgroundDiscoveryEnabledByDefault, @Nullable String scanInputLabel, + @Nullable String scanInputDescription) throws IllegalArgumentException { if (timeout < 0) { throw new IllegalArgumentException("The timeout must be >= 0!"); } this.supportedThingTypes = supportedThingTypes == null ? Set.of() : Set.copyOf(supportedThingTypes); this.timeout = timeout; this.backgroundDiscoveryEnabled = backgroundDiscoveryEnabledByDefault; + this.scanInputLabel = scanInputLabel; + this.scanInputDescription = scanInputDescription; + } + + /** + * Creates a new instance of this class with the specified parameters and no input parameter supported to start the + * discovery. + * + * @param supportedThingTypes the list of Thing types which are supported (can be null) + * @param timeout the discovery timeout in seconds after which the discovery + * service automatically stops its forced discovery process (>= 0). + * @param backgroundDiscoveryEnabledByDefault defines, whether the default for this discovery service is to + * enable background discovery or not. + * @throws IllegalArgumentException if {@code timeout < 0} + */ + protected AbstractDiscoveryService(@Nullable Set supportedThingTypes, int timeout, + boolean backgroundDiscoveryEnabledByDefault) throws IllegalArgumentException { + this(supportedThingTypes, timeout, backgroundDiscoveryEnabledByDefault, null, null); } /** - * Creates a new instance of this class with the specified parameters and background discovery enabled. + * Creates a new instance of this class with the specified parameters and background discovery enabled + * and no input parameter supported to start the discovery. * * @param supportedThingTypes the list of Thing types which are supported (can be null) * @param timeout the discovery timeout in seconds after which the discovery service @@ -111,7 +139,8 @@ protected AbstractDiscoveryService(@Nullable Set supportedThingTyp } /** - * Creates a new instance of this class with the specified parameters and background discovery enabled. + * Creates a new instance of this class with the specified parameters and background discovery enabled + * and no input parameter supported to start the discovery. * * @param timeout the discovery timeout in seconds after which the discovery service * automatically stops its forced discovery process (>= 0). @@ -133,6 +162,21 @@ public Set getSupportedThingTypes() { return supportedThingTypes; } + @Override + public boolean isScanInputSupported() { + return scanInputLabel != null && scanInputDescription != null; + } + + @Override + public @Nullable String getScanInputLabel() { + return scanInputLabel; + } + + @Override + public @Nullable String getScanInputDescription() { + return scanInputDescription; + } + /** * Returns the amount of time in seconds after which the discovery service automatically * stops its forced discovery process. @@ -168,7 +212,16 @@ public void removeDiscoveryListener(@Nullable DiscoveryListener listener) { } @Override - public synchronized void startScan(@Nullable ScanListener listener) { + public void startScan(@Nullable ScanListener listener) { + startScanInternal(null, listener); + } + + @Override + public void startScan(String input, @Nullable ScanListener listener) { + startScanInternal(input, listener); + } + + private synchronized void startScanInternal(@Nullable String input, @Nullable ScanListener listener) { synchronized (this) { // we first stop any currently running scan and its scheduled stop // call @@ -194,7 +247,11 @@ public synchronized void startScan(@Nullable ScanListener listener) { timestampOfLastScan = Instant.now(); try { - startScan(); + if (isScanInputSupported() && input != null) { + startScan(input); + } else { + startScan(); + } } catch (Exception ex) { scheduledStop = this.scheduledStop; if (scheduledStop != null) { @@ -232,6 +289,11 @@ public synchronized void abortScan() { */ protected abstract void startScan(); + // An abstract method would have required a change in all existing bindings implementing a DiscoveryService + protected void startScan(String input) { + logger.warn("Discovery with input parameter not implemented by service '{}'!", this.getClass().getName()); + } + /** * This method cleans up after a scan, i.e. it removes listeners and other required operations. */ diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractThingHandlerDiscoveryService.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractThingHandlerDiscoveryService.java index 0dd7aa12594..2924db68394 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractThingHandlerDiscoveryService.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractThingHandlerDiscoveryService.java @@ -32,6 +32,7 @@ * It handles the injection of the {@link ThingHandler} * * @author Jan N. Klug - Initial contribution + * @author Laurent Garnier - Added discovery with an optional input parameter */ @NonNullByDefault public abstract class AbstractThingHandlerDiscoveryService extends AbstractDiscoveryService @@ -45,12 +46,18 @@ public abstract class AbstractThingHandlerDiscoveryService thingClazz, @Nullable Set supportedThingTypes, - int timeout, boolean backgroundDiscoveryEnabledByDefault) throws IllegalArgumentException { - super(supportedThingTypes, timeout, backgroundDiscoveryEnabledByDefault); + int timeout, boolean backgroundDiscoveryEnabledByDefault, @Nullable String scanInputLabel, + @Nullable String scanInputDescription) throws IllegalArgumentException { + super(supportedThingTypes, timeout, backgroundDiscoveryEnabledByDefault, scanInputLabel, scanInputDescription); this.thingClazz = thingClazz; this.backgroundDiscoveryEnabled = backgroundDiscoveryEnabledByDefault; } + protected AbstractThingHandlerDiscoveryService(Class thingClazz, @Nullable Set supportedThingTypes, + int timeout, boolean backgroundDiscoveryEnabledByDefault) throws IllegalArgumentException { + this(thingClazz, supportedThingTypes, timeout, backgroundDiscoveryEnabledByDefault, null, null); + } + protected AbstractThingHandlerDiscoveryService(Class thingClazz, @Nullable Set supportedThingTypes, int timeout) throws IllegalArgumentException { this(thingClazz, supportedThingTypes, timeout, true); diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryService.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryService.java index 14eb273b9b9..f6b91d37b05 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryService.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryService.java @@ -39,6 +39,7 @@ * @author Michael Grammling - Initial contribution * @author Kai Kreuzer - Refactored API * @author Dennis Nobel - Added background discovery configuration through Configuration Admin + * @author Laurent Garnier - Added discovery with an optional input parameter * * @see DiscoveryListener * @see DiscoveryServiceRegistry @@ -60,6 +61,31 @@ public interface DiscoveryService { */ Collection getSupportedThingTypes(); + /** + * Returns {@code true} if the discovery supports an optional input parameter to run, otherwise {@code false}. + * + * @return true if the discovery supports an optional input parameter to run, otherwise false + */ + boolean isScanInputSupported(); + + /** + * Returns the label of the supported input parameter to start the discovery. + * + * @return the label of the supported input parameter to start the discovery or null if input parameter not + * supported + */ + @Nullable + String getScanInputLabel(); + + /** + * Returns the description of the supported input parameter to start the discovery. + * + * @return the description of the supported input parameter to start the discovery or null if input parameter not + * supported + */ + @Nullable + String getScanInputDescription(); + /** * Returns the amount of time in seconds after which an active scan ends. * @@ -87,6 +113,20 @@ public interface DiscoveryService { */ void startScan(@Nullable ScanListener listener); + /** + * Triggers this service to start an active scan for new devices using an input parameter for that.
+ * This method must not block any calls such as {@link #abortScan()} and + * must return fast. + *

+ * If started, any registered {@link DiscoveryListener} must be notified about {@link DiscoveryResult}s. + *

+ * If there is already a scan running, it is aborted and a new scan is triggered. + * + * @param input an input parameter to be used during discovery scan + * @param listener a listener that is notified about errors or termination of the scan + */ + void startScan(String input, @Nullable ScanListener listener); + /** * Stops an active scan for devices.
* This method must not block any calls such as {@link #startScan} and must diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistry.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistry.java index 19b6248d672..289fa357aa9 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistry.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistry.java @@ -13,6 +13,7 @@ package org.openhab.core.config.discovery; import java.util.List; +import java.util.Set; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -29,6 +30,7 @@ * * @author Michael Grammling - Initial contribution * @author Ivaylo Ivanov - Added getMaxScanTimeout + * @author Laurent Garnier - Added discovery with an optional input parameter * * @see DiscoveryService * @see DiscoveryListener @@ -44,6 +46,7 @@ public interface DiscoveryServiceRegistry { * * @param thingTypeUID the Thing type UID pointing to collection of discovery * services to be forced to start a discovery + * @param input an optional input parameter to be used during discovery scan, can be null. * @param listener a callback to inform about errors or termination, can be null. * If more than one discovery service is started, the {@link ScanListener#onFinished()} callback is * called after all @@ -54,7 +57,7 @@ public interface DiscoveryServiceRegistry { * @return true if a t least one discovery service could be found and forced * to start a discovery, otherwise false */ - boolean startScan(ThingTypeUID thingTypeUID, @Nullable ScanListener listener); + boolean startScan(ThingTypeUID thingTypeUID, @Nullable String input, @Nullable ScanListener listener); /** * Forces the associated {@link DiscoveryService}s to start a discovery for @@ -65,6 +68,7 @@ public interface DiscoveryServiceRegistry { * * @param bindingId the binding id pointing to one or more discovery services to * be forced to start a discovery + * @param input an optional input parameter to be used during discovery scan, can be null. * @param listener a callback to inform about errors or termination, can be null. * If more than one discovery service is started, the {@link ScanListener#onFinished()} callback is * called after all @@ -75,7 +79,7 @@ public interface DiscoveryServiceRegistry { * @return true if a t least one discovery service could be found and forced * to start a discovery, otherwise false */ - boolean startScan(String bindingId, @Nullable ScanListener listener); + boolean startScan(String bindingId, @Nullable String input, @Nullable ScanListener listener); /** * Aborts a started discovery on all {@link DiscoveryService}s for the given @@ -163,6 +167,13 @@ public interface DiscoveryServiceRegistry { */ List getSupportedBindings(); + /** + * Returns the list of all {@link DiscoveryService}s, that discover thing types of the given binding id. + * + * @return list of discovery services, that discover thing types of the given binding id + */ + Set getDiscoveryServices(String bindingId) throws IllegalStateException; + /** * Returns the maximum discovery timeout from all discovery services registered for the specified thingTypeUID * diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java index 0601c3a305c..1d7290437ed 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java @@ -56,6 +56,7 @@ * @author Kai Kreuzer - Refactored API * @author Andre Fuechsel - Added removeOlderResults * @author Ivaylo Ivanov - Added getMaxScanTimeout + * @author Laurent Garnier - Added discovery with an optional input parameter * * @see DiscoveryServiceRegistry * @see DiscoveryListener @@ -189,7 +190,7 @@ public void addDiscoveryListener(DiscoveryListener listener) throws IllegalState } @Override - public boolean startScan(ThingTypeUID thingTypeUID, @Nullable ScanListener listener) throws IllegalStateException { + public boolean startScan(ThingTypeUID thingTypeUID, @Nullable String input, @Nullable ScanListener listener) { Set discoveryServicesForThingType = getDiscoveryServices(thingTypeUID); if (discoveryServicesForThingType.isEmpty()) { @@ -197,11 +198,11 @@ public boolean startScan(ThingTypeUID thingTypeUID, @Nullable ScanListener liste return false; } - return startScans(discoveryServicesForThingType, listener); + return startScans(discoveryServicesForThingType, input, listener); } @Override - public boolean startScan(String bindingId, final @Nullable ScanListener listener) throws IllegalStateException { + public boolean startScan(String bindingId, @Nullable String input, @Nullable ScanListener listener) { final Set discoveryServicesForBinding = getDiscoveryServices(bindingId); if (discoveryServicesForBinding.isEmpty()) { @@ -209,7 +210,7 @@ public boolean startScan(String bindingId, final @Nullable ScanListener listener return false; } - return startScans(discoveryServicesForBinding, listener); + return startScans(discoveryServicesForBinding, input, listener); } @Override @@ -326,7 +327,8 @@ private boolean abortScans(Set discoveryServices) { return allServicesAborted; } - private boolean startScans(Set discoveryServices, @Nullable ScanListener listener) { + private boolean startScans(Set discoveryServices, @Nullable String input, + @Nullable ScanListener listener) { boolean atLeastOneDiscoveryServiceHasBeenStarted = false; if (discoveryServices.size() > 1) { @@ -334,7 +336,7 @@ private boolean startScans(Set discoveryServices, @Nullable Sc AggregatingScanListener aggregatingScanListener = new AggregatingScanListener(discoveryServices.size(), listener); for (DiscoveryService discoveryService : discoveryServices) { - if (startScan(discoveryService, aggregatingScanListener)) { + if (startScan(discoveryService, input, aggregatingScanListener)) { atLeastOneDiscoveryServiceHasBeenStarted = true; } else { logger.debug( @@ -343,7 +345,7 @@ private boolean startScans(Set discoveryServices, @Nullable Sc } } } else { - if (startScan(discoveryServices.iterator().next(), listener)) { + if (startScan(discoveryServices.iterator().next(), input, listener)) { atLeastOneDiscoveryServiceHasBeenStarted = true; } } @@ -351,13 +353,18 @@ private boolean startScans(Set discoveryServices, @Nullable Sc return atLeastOneDiscoveryServiceHasBeenStarted; } - private boolean startScan(DiscoveryService discoveryService, @Nullable ScanListener listener) { + private boolean startScan(DiscoveryService discoveryService, @Nullable String input, + @Nullable ScanListener listener) { Collection supportedThingTypes = discoveryService.getSupportedThingTypes(); try { logger.debug("Triggering scan for thing types '{}' on '{}'...", supportedThingTypes, discoveryService.getClass().getSimpleName()); - discoveryService.startScan(listener); + if (discoveryService.isScanInputSupported() && input != null) { + discoveryService.startScan(input, listener); + } else { + discoveryService.startScan(listener); + } return true; } catch (Exception ex) { logger.error("Cannot trigger scan for thing types '{}' on '{}'!", supportedThingTypes, @@ -380,7 +387,8 @@ private synchronized Set getDiscoveryServices(ThingTypeUID thi return discoveryServices; } - private synchronized Set getDiscoveryServices(String bindingId) throws IllegalStateException { + @Override + public synchronized Set getDiscoveryServices(String bindingId) throws IllegalStateException { Set discoveryServices = new HashSet<>(); for (DiscoveryService discoveryService : this.discoveryServices) { diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/console/DiscoveryConsoleCommandExtension.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/console/DiscoveryConsoleCommandExtension.java index 31c98d7f8b2..2ec68426c3d 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/console/DiscoveryConsoleCommandExtension.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/console/DiscoveryConsoleCommandExtension.java @@ -18,6 +18,7 @@ import java.util.List; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.config.discovery.DiscoveryService; import org.openhab.core.config.discovery.DiscoveryServiceRegistry; import org.openhab.core.io.console.Console; @@ -37,6 +38,7 @@ * * @author Kai Kreuzer - Initial contribution * @author Dennis Nobel - Added background discovery commands + * @author Laurent Garnier - Updated command to start discovery with a new optional input parameter */ @Component(immediate = true, service = ConsoleCommandExtension.class) @NonNullByDefault @@ -69,9 +71,9 @@ public void execute(String[] args, Console console) { String arg1 = args[1]; if (arg1.contains(":")) { ThingTypeUID thingTypeUID = new ThingTypeUID(arg1); - runDiscoveryForThingType(console, thingTypeUID); + runDiscoveryForThingType(console, thingTypeUID, args.length > 2 ? args[2] : null); } else { - runDiscoveryForBinding(console, arg1); + runDiscoveryForBinding(console, arg1, args.length > 2 ? args[2] : null); } } else { console.println("Specify thing type id or binding id to discover: discovery " @@ -123,18 +125,18 @@ private void configureBackgroundDiscovery(Console console, String discoveryServi } } - private void runDiscoveryForThingType(Console console, ThingTypeUID thingTypeUID) { - discoveryServiceRegistry.startScan(thingTypeUID, null); + private void runDiscoveryForThingType(Console console, ThingTypeUID thingTypeUID, @Nullable String input) { + discoveryServiceRegistry.startScan(thingTypeUID, input, null); } - private void runDiscoveryForBinding(Console console, String bindingId) { - discoveryServiceRegistry.startScan(bindingId, null); + private void runDiscoveryForBinding(Console console, String bindingId, @Nullable String input) { + discoveryServiceRegistry.startScan(bindingId, input, null); } @Override public List getUsages() { return List.of( - buildCommandUsage(SUBCMD_START + " ", + buildCommandUsage(SUBCMD_START + " []", "runs a discovery on a given thing type or binding"), buildCommandUsage(SUBCMD_BACKGROUND_DISCOVERY_ENABLE + " ", "enables background discovery for the discovery service with the given PID"), diff --git a/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/AbstractDiscoveryServiceTest.java b/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/AbstractDiscoveryServiceTest.java index 6e19183f784..83ed03d8c14 100644 --- a/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/AbstractDiscoveryServiceTest.java +++ b/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/AbstractDiscoveryServiceTest.java @@ -15,7 +15,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsMapContaining.hasEntry; -import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.*; import java.util.Collection; import java.util.Locale; @@ -35,6 +35,7 @@ * Tests the {@link DiscoveryResultBuilder}. * * @author Laurent Garnier - Initial contribution + * @author Laurent Garnier - Added test for discovery with an input parameter */ @NonNullByDefault public class AbstractDiscoveryServiceTest implements DiscoveryListener { @@ -46,6 +47,7 @@ public class AbstractDiscoveryServiceTest implements DiscoveryListener { private static final ThingUID THING_UID2 = new ThingUID(THING_TYPE_UID, "thingId2"); private static final ThingUID THING_UID3 = new ThingUID(THING_TYPE_UID, BRIDGE_UID, "thingId3"); private static final ThingUID THING_UID4 = new ThingUID(THING_TYPE_UID, "thingId4"); + private static final ThingUID THING_UID5 = new ThingUID(THING_TYPE_UID, BRIDGE_UID, "thingId5"); private static final String KEY1 = "key1"; private static final String KEY2 = "key2"; private static final String VALUE1 = "value1"; @@ -58,9 +60,12 @@ public class AbstractDiscoveryServiceTest implements DiscoveryListener { private static final String DISCOVERY_LABEL = "Result Test"; private static final String DISCOVERY_LABEL_KEY1 = "@text/test"; private static final String DISCOVERY_LABEL_KEY2 = "@text/test2 [ \"50\", \"number\" ]"; + private static final String DISCOVERY_LABEL_CODE = "Result Test with pairing code"; private static final String PROPERTY_LABEL1 = "Label from property (text key)"; private static final String PROPERTY_LABEL2 = "Label from property (infered key)"; private static final String PROPERTY_LABEL3 = "Label from property (parameters 50 and number)"; + private static final String PAIRING_CODE_LABEL = "Pairing Code"; + private static final String PAIRING_CODE_DESCR = "The pairing code"; private TranslationProvider i18nProvider = new TranslationProvider() { @Override @@ -134,6 +139,28 @@ protected void startScan() { } } + class TestDiscoveryServiceWithRequiredCode extends AbstractDiscoveryService { + + public TestDiscoveryServiceWithRequiredCode(TranslationProvider i18nProvider, LocaleProvider localeProvider) + throws IllegalArgumentException { + super(Set.of(THING_TYPE_UID), 1, false, PAIRING_CODE_LABEL, PAIRING_CODE_DESCR); + this.i18nProvider = i18nProvider; + this.localeProvider = localeProvider; + } + + @Override + protected void startScan() { + } + + @Override + protected void startScan(String input) { + DiscoveryResult discoveryResult = DiscoveryResultBuilder.create(THING_UID5).withThingType(THING_TYPE_UID) + .withProperties(properties).withRepresentationProperty(KEY1).withBridge(BRIDGE_UID) + .withLabel(DISCOVERY_LABEL_CODE).build(); + thingDiscovered(discoveryResult); + } + } + @Override public void thingDiscovered(DiscoveryService source, DiscoveryResult result) { assertThat(result.getThingTypeUID(), is(THING_TYPE_UID)); @@ -156,6 +183,9 @@ public void thingDiscovered(DiscoveryService source, DiscoveryResult result) { } else if (THING_UID4.equals(result.getThingUID())) { assertNull(result.getBridgeUID()); assertThat(result.getLabel(), is(PROPERTY_LABEL3)); + } else if (THING_UID5.equals(result.getThingUID())) { + assertThat(result.getBridgeUID(), is(BRIDGE_UID)); + assertThat(result.getLabel(), is(DISCOVERY_LABEL_CODE)); } } @@ -172,7 +202,21 @@ public void thingRemoved(DiscoveryService source, ThingUID thingUID) { @Test public void testDiscoveryResults() { TestDiscoveryService discoveryService = new TestDiscoveryService(i18nProvider, localeProvider); + assertFalse(discoveryService.isScanInputSupported()); + assertNull(discoveryService.getScanInputLabel()); + assertNull(discoveryService.getScanInputDescription()); discoveryService.addDiscoveryListener(this); discoveryService.startScan(); } + + @Test + public void testDiscoveryResultsWhenCodeRequired() { + TestDiscoveryServiceWithRequiredCode discoveryService = new TestDiscoveryServiceWithRequiredCode(i18nProvider, + localeProvider); + assertTrue(discoveryService.isScanInputSupported()); + assertThat(discoveryService.getScanInputLabel(), is(PAIRING_CODE_LABEL)); + assertThat(discoveryService.getScanInputDescription(), is(PAIRING_CODE_DESCR)); + discoveryService.addDiscoveryListener(this); + discoveryService.startScan("code"); + } } diff --git a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/discovery/DiscoveryInfoDTO.java b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/discovery/DiscoveryInfoDTO.java new file mode 100644 index 00000000000..958a590f014 --- /dev/null +++ b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/discovery/DiscoveryInfoDTO.java @@ -0,0 +1,35 @@ +/** + * Copyright (c) 2010-2024 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.io.rest.core.discovery; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; + +/** + * This is a data transfer object that is used to serialize the information about binding discovery. + * + * @author Laurent Garnier - Initial contribution + */ +@NonNullByDefault +public class DiscoveryInfoDTO { + + public boolean inputSupported; + public @Nullable String inputLabel; + public @Nullable String inputDescription; + + public DiscoveryInfoDTO(boolean inputSupported, @Nullable String inputLabel, @Nullable String inputDescription) { + this.inputSupported = inputSupported; + this.inputLabel = inputLabel; + this.inputDescription = inputDescription; + } +} diff --git a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/discovery/DiscoveryResource.java b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/discovery/DiscoveryResource.java index 83a7fb640db..248df7b3e40 100644 --- a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/discovery/DiscoveryResource.java +++ b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/discovery/DiscoveryResource.java @@ -14,23 +14,37 @@ import java.util.Collection; import java.util.LinkedHashSet; +import java.util.Locale; +import java.util.Set; import javax.annotation.security.RolesAllowed; import javax.ws.rs.GET; +import javax.ws.rs.HeaderParam; import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import javax.ws.rs.core.Response.Status; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.auth.Role; +import org.openhab.core.config.discovery.DiscoveryService; import org.openhab.core.config.discovery.DiscoveryServiceRegistry; import org.openhab.core.config.discovery.ScanListener; +import org.openhab.core.i18n.I18nUtil; +import org.openhab.core.i18n.TranslationProvider; +import org.openhab.core.io.rest.JSONResponse; +import org.openhab.core.io.rest.LocaleService; import org.openhab.core.io.rest.RESTConstants; import org.openhab.core.io.rest.RESTResource; +import org.openhab.core.io.rest.core.discovery.DiscoveryInfoDTO; +import org.osgi.framework.Bundle; +import org.osgi.framework.FrameworkUtil; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; @@ -62,6 +76,7 @@ * @author Franck Dechavanne - Added DTOs to ApiResponses * @author Markus Rathgeb - Migrated to JAX-RS Whiteboard Specification * @author Wouter Born - Migrated to OpenAPI annotations + * @author Laurent Garnier - Added discovery with an optional input parameter */ @Component(service = { RESTResource.class, DiscoveryResource.class }) @JaxrsResource @@ -81,10 +96,15 @@ public class DiscoveryResource implements RESTResource { private final Logger logger = LoggerFactory.getLogger(DiscoveryResource.class); private final DiscoveryServiceRegistry discoveryServiceRegistry; + private final LocaleService localeService; + private final TranslationProvider i18nProvider; @Activate - public DiscoveryResource(final @Reference DiscoveryServiceRegistry discoveryServiceRegistry) { + public DiscoveryResource(final @Reference DiscoveryServiceRegistry discoveryServiceRegistry, + final @Reference TranslationProvider translationProvider, final @Reference LocaleService localeService) { this.discoveryServiceRegistry = discoveryServiceRegistry; + this.i18nProvider = translationProvider; + this.localeService = localeService; } @GET @@ -96,13 +116,59 @@ public Response getDiscoveryServices() { return Response.ok(new LinkedHashSet<>(supportedBindings)).build(); } + @GET + @Path("/bindings/{bindingId}/info") + @Produces(MediaType.APPLICATION_JSON) + @Operation(operationId = "getDiscoveryServicesInfo", summary = "Gets information about the discovery services for a binding.", responses = { + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = DiscoveryInfoDTO.class))), + @ApiResponse(responseCode = "404", description = "Discovery service not found") }) + public Response getDiscoveryServicesInfo( + @PathParam("bindingId") @Parameter(description = "binding Id") final String bindingId, + @HeaderParam(HttpHeaders.ACCEPT_LANGUAGE) @Parameter(description = "language") @Nullable String language) { + final Locale locale = localeService.getLocale(language); + String label = null; + String description = null; + boolean supported = false; + Set discoveryServices = discoveryServiceRegistry.getDiscoveryServices(bindingId); + + if (discoveryServices.isEmpty()) { + return JSONResponse.createResponse(Status.NOT_FOUND, null, + "No discovery service found for binding " + bindingId); + } + + for (DiscoveryService discoveryService : discoveryServices) { + if (discoveryService.isScanInputSupported()) { + Bundle bundle = FrameworkUtil.getBundle(discoveryService.getClass()); + label = discoveryService.getScanInputLabel(); + if (label != null) { + label = i18nProvider.getText(bundle, I18nUtil.stripConstant(label), label, locale); + } + description = discoveryService.getScanInputDescription(); + if (description != null) { + description = i18nProvider.getText(bundle, I18nUtil.stripConstant(description), description, + locale); + } + supported = true; + break; + } + } + return Response.ok(new DiscoveryInfoDTO(supported, label, description)).build(); + } + @POST @Path("/bindings/{bindingId}/scan") @Produces(MediaType.TEXT_PLAIN) @Operation(operationId = "scan", summary = "Starts asynchronous discovery process for a binding and returns the timeout in seconds of the discovery operation.", responses = { - @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = Integer.class))) }) - public Response scan(@PathParam("bindingId") @Parameter(description = "bindingId") final String bindingId) { - discoveryServiceRegistry.startScan(bindingId, new ScanListener() { + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = Integer.class))), + @ApiResponse(responseCode = "404", description = "Discovery service not found") }) + public Response scan(@PathParam("bindingId") @Parameter(description = "binding Id") final String bindingId, + @QueryParam("input") @Parameter(description = "input parameter to start the discovery") @Nullable String input) { + if (discoveryServiceRegistry.getDiscoveryServices(bindingId).isEmpty()) { + return JSONResponse.createResponse(Status.NOT_FOUND, null, + "No discovery service found for binding " + bindingId); + } + + discoveryServiceRegistry.startScan(bindingId, input, new ScanListener() { @Override public void onErrorOccurred(@Nullable Exception exception) { logger.error("Error occurred while scanning for binding '{}'", bindingId, exception); diff --git a/itests/org.openhab.core.config.discovery.tests/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistryOSGiTest.java b/itests/org.openhab.core.config.discovery.tests/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistryOSGiTest.java index 44de79656e2..3b77e09bc19 100644 --- a/itests/org.openhab.core.config.discovery.tests/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistryOSGiTest.java +++ b/itests/org.openhab.core.config.discovery.tests/src/main/java/org/openhab/core/config/discovery/DiscoveryServiceRegistryOSGiTest.java @@ -160,17 +160,19 @@ public void afterEach() throws Exception { @Test public void testStartScanNonExisting() { - assertFalse(discoveryServiceRegistry.startScan(new ThingTypeUID("bindingId", "thingType"), null)); + assertFalse(discoveryServiceRegistry.startScan(new ThingTypeUID("bindingId", "thingType"), null, null)); } @Test public void testStartScanExisting() { - assertTrue(discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null)); + assertTrue( + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, null)); } @Test public void testScanFaulty() { - assertFalse(discoveryServiceRegistry.startScan(new ThingTypeUID(FAULTY_BINDING_ID, FAULTY_THING_TYPE), null)); + assertFalse( + discoveryServiceRegistry.startScan(new ThingTypeUID(FAULTY_BINDING_ID, FAULTY_THING_TYPE), null, null)); } @Test @@ -182,7 +184,7 @@ public void testAbortScanNonExisting() { public void testAbortScanKnown() { ScanListener mockScanListener = mock(ScanListener.class); - assertTrue(discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), + assertTrue(discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, mockScanListener)); assertTrue(discoveryServiceRegistry.abortScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1))); @@ -195,7 +197,8 @@ public void testThingDiscovered() { ScanListener mockScanListener = mock(ScanListener.class); discoveryServiceRegistry.addDiscoveryListener(discoveryListenerMock); - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), mockScanListener); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, + mockScanListener); waitForAssert(() -> verify(mockScanListener, times(1)).onFinished()); verify(discoveryListenerMock, times(1)).thingDiscovered(any(), any()); @@ -221,8 +224,10 @@ public void testRemoveOlderResultsWorks() { ScanListener mockScanListener2 = mock(ScanListener.class); discoveryServiceRegistry.addDiscoveryListener(discoveryListenerMock); - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), mockScanListener1); - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_2, ANY_THING_TYPE_2), mockScanListener2); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, + mockScanListener1); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_2, ANY_THING_TYPE_2), null, + mockScanListener2); waitForAssert(() -> verify(mockScanListener1, times(1)).onFinished()); waitForAssert(() -> verify(mockScanListener2, times(1)).onFinished()); @@ -234,7 +239,8 @@ public void testRemoveOlderResultsWorks() { assertThat(inbox.getAll().size(), is(2)); // start discovery again - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), mockScanListener1); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, + mockScanListener1); waitForAssert(() -> verify(mockScanListener1, times(2)).onFinished()); verify(discoveryListenerMock, times(3)).thingDiscovered(any(), any()); @@ -250,7 +256,8 @@ public void testRemoveOlderResultsOnlySameService() { ScanListener mockScanListener1 = mock(ScanListener.class); discoveryServiceRegistry.addDiscoveryListener(discoveryListenerMock); - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), mockScanListener1); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, + mockScanListener1); waitForAssert(() -> verify(mockScanListener1, times(1)).onFinished()); verify(discoveryListenerMock, times(1)).thingDiscovered(any(), any()); @@ -264,7 +271,8 @@ public void testRemoveOlderResultsOnlySameService() { anotherDiscoveryServiceMockForBinding1, null)); // start discovery again - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), mockScanListener1); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, + mockScanListener1); waitForAssert(() -> verify(mockScanListener1, times(2)).onFinished()); verify(discoveryListenerMock, times(3)).thingDiscovered(any(), any()); @@ -288,7 +296,7 @@ public void testRemoveOlderResultsOnlyOfSpecificBridge() { ScanListener mockScanListener1 = mock(ScanListener.class); discoveryServiceRegistry.addDiscoveryListener(discoveryListenerMock); - discoveryServiceRegistry.startScan(ANY_BINDING_ID_3_ANY_THING_TYPE_3_UID, mockScanListener1); + discoveryServiceRegistry.startScan(ANY_BINDING_ID_3_ANY_THING_TYPE_3_UID, null, mockScanListener1); waitForAssert(() -> verify(mockScanListener1, times(1)).onFinished()); verify(discoveryListenerMock, times(2)).thingDiscovered(any(), any()); @@ -311,7 +319,8 @@ public void testRemoveOlderResultsOnlyOfSpecificBridge() { assertThat(inbox.getAll().size(), is(2)); // start discovery again - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_3, ANY_THING_TYPE_3), mockScanListener1); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_3, ANY_THING_TYPE_3), null, + mockScanListener1); waitForAssert(() -> verify(mockScanListener1, times(1)).onFinished()); verify(discoveryListenerMock, times(4)).thingDiscovered(any(), any()); @@ -343,7 +352,8 @@ public void testThingDiscoveredRemovedListener() { ScanListener mockScanListener1 = mock(ScanListener.class); discoveryServiceRegistry.addDiscoveryListener(discoveryListenerMock); discoveryServiceRegistry.removeDiscoveryListener(discoveryListenerMock); - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), mockScanListener1); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, + mockScanListener1); waitForAssert(() -> verify(mockScanListener1, times(1)).onFinished()); verifyNoMoreInteractions(discoveryListenerMock); @@ -357,7 +367,8 @@ public void testStartScanTwoDiscoveryServices() { serviceRegs.add( bundleContext.registerService(DiscoveryService.class.getName(), anotherDiscoveryServiceMock, null)); discoveryServiceRegistry.addDiscoveryListener(discoveryListenerMock); - discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), mockScanListener1); + discoveryServiceRegistry.startScan(new ThingTypeUID(ANY_BINDING_ID_1, ANY_THING_TYPE_1), null, + mockScanListener1); waitForAssert(mockScanListener1::onFinished); verify(discoveryListenerMock, times(2)).thingDiscovered(any(), any()); @@ -366,7 +377,7 @@ public void testStartScanTwoDiscoveryServices() { @Test public void testStartScanBindingId() { ScanListener mockScanListener1 = mock(ScanListener.class); - discoveryServiceRegistry.startScan(ANY_BINDING_ID_1, mockScanListener1); + discoveryServiceRegistry.startScan(ANY_BINDING_ID_1, null, mockScanListener1); waitForAssert(() -> verify(mockScanListener1, times(1)).onFinished()); } From a80378efbaee0aeeb6f52a28f65ee933e2e9d596 Mon Sep 17 00:00:00 2001 From: Laurent Garnier Date: Sun, 29 Sep 2024 10:09:35 +0200 Subject: [PATCH 2/2] Review comment: restore IllegalStateException Signed-off-by: Laurent Garnier --- .../discovery/internal/DiscoveryServiceRegistryImpl.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java index 1d7290437ed..c7a88d73755 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryServiceRegistryImpl.java @@ -190,7 +190,8 @@ public void addDiscoveryListener(DiscoveryListener listener) throws IllegalState } @Override - public boolean startScan(ThingTypeUID thingTypeUID, @Nullable String input, @Nullable ScanListener listener) { + public boolean startScan(ThingTypeUID thingTypeUID, @Nullable String input, @Nullable ScanListener listener) + throws IllegalStateException { Set discoveryServicesForThingType = getDiscoveryServices(thingTypeUID); if (discoveryServicesForThingType.isEmpty()) { @@ -202,7 +203,8 @@ public boolean startScan(ThingTypeUID thingTypeUID, @Nullable String input, @Nul } @Override - public boolean startScan(String bindingId, @Nullable String input, @Nullable ScanListener listener) { + public boolean startScan(String bindingId, @Nullable String input, @Nullable ScanListener listener) + throws IllegalStateException { final Set discoveryServicesForBinding = getDiscoveryServices(bindingId); if (discoveryServicesForBinding.isEmpty()) {