10386: Implemented review remarks

Signed-off-by: Sönke Küper <[email protected]>

CODEOWNERS

@@ -9,6 +9,7 @@
 /bundles/org.openhab.automation.jythonscripting/ @openhab/add-ons-maintainers
 /bundles/org.openhab.automation.pidcontroller/ @fwolter
 /bundles/org.openhab.binding.adorne/ @theiding
+/bundles/org.openhab.binding.ahawastecollection/ @soenkekueper
 /bundles/org.openhab.binding.airquality/ @kubawolanin
 /bundles/org.openhab.binding.airvisualnode/ @3cky
 /bundles/org.openhab.binding.alarmdecoder/ @bobadair @billfor

bom/openhab-addons/pom.xml

@@ -36,6 +36,11 @@
       <artifactId>org.openhab.binding.adorne</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.openhab.addons.bundles</groupId>
+      <artifactId>org.openhab.binding.ahawastecollection</artifactId>
+      <version>${project.version}</version>
+    </dependency>
     <dependency>
       <groupId>org.openhab.addons.bundles</groupId>
       <artifactId>org.openhab.binding.airquality</artifactId>

bundles/org.openhab.binding.ahawastecollection/README.md

@@ -1,29 +1,48 @@
 # aha Waste Collection Binding
 
-This Binding allows requesting the next waste collection days from the aha waste collection schedule, reachable at [aha Abfuhrkalender](https://www.aha-region.de/abholtermine/abfuhrkalender).
+This binding provides information about the upcoming waste collection dates for places, that are served by aha, the waste collection company for the region of Hannover. The values are retrieved from the online aha waste collection schedule available at: [aha Abfuhrkalender](https://www.aha-region.de/abholtermine/abfuhrkalender).
 
 ## Supported Things
 
-The binding supports one thing type **aha Waste Collection Schedule** with four channels for the different waste types.
+-   **collectionSchedule:** Represents the connection to the **aha Waste Collection Schedule** with four channels for the different waste types.
 
 ## Discovery
 
 Discovery is not possible, due some form input values from the website above are required.
 
 ## Thing Configuration
 
-To configure an **aha Waste Collection Schedule** thing, you need first to get the required parameters from to get them follow the instructions below:
-the form at https://www.aha-region.de/abholtermine/abfuhrkalender.
+For configuration of the **collectionSchedule** thing, you need the form inputs from the aha collections schedule web page. Follow the steps below to get the required configuration parameters from the form input values.
+
 
 1. Open [aha Abfuhrkalender](https://www.aha-region.de/abholtermine/abfuhrkalender) in your favorite brower with developer-console.
-2. Open the developer console and switch to network tab (for example press F12 in chrome / edge / firefox)
-3. Fill in the form: Select your commune, Street and Housenumber and hit "Suchen".
-4. Grab the form input fields from the first POST request to https://www.aha-region.de/abholtermine/abfuhrkalender.
-5. Fill in the values from the form input in thing configuration.
+2. Open the developer console and switch to network tab (for example press F12 in chrome / edge / firefox).
+3. Fill in the form: Select your commune, Street and house number and hit "Suchen".
+4. Select the first request to https://www.aha-region.de/abholtermine/abfuhrkalender (see first screenshot below)
+5. Check the form data at the end of the request for the form values (see second screenshot below)
+5. Fill in the values from the form input in thing configuration (see examples below)
+
+![Chrome Developer Console Top](doc/images/ChromeDevconsoleTop.png "Chrome Developer Console showing request URL")
+
+*Check if you've selected the correct request, that contains the form data*
+
+![Chrome Developer Console Bottom](doc/images/ChromeDevconsoleBottom.png "Chrome Developer Console showing form inputs")
+
+*Grab the values for the configuration parameters from the form data section at the end of the request*
+
+**collectionSchedule** parameters:
+
+| Property | Default | Required | Description |
+|-|-|-|-|
+| `commune` | | Yes | The selected commune, taken from the form field `gemeinde`. |
+| `street` | | Yes | The selected street, taken from the form field `strasse`. This value must look like 67269@Rosmarinweg+/+Kirchhorst@Kirchhorst |
+| `houseNumber` |  | Yes | The selected house number, taken from the form field `hausnr`. |
+| `houseNumberAddon` | | No | The selected house number addon, taken from the form field `hausnraddon`, may be empty. |
+| `collectionPlace` | | Yes | Form value for the collection place, taken from the form field `ladeort`. This value must look like 67269-0010+ |
 
 ## Channels
 
-The thing **aha Waste Collection Schedule** provides four channels for the next day of waste collection from the different waste types.
+The thing **aha Waste Collection Schedule** provides four channels for the upcoming day of waste collection for the different waste types.
 
 
 | channel  | type   | description                  |
@@ -38,13 +57,13 @@ The thing **aha Waste Collection Schedule** provides four channels for the next
 
 wasteCollection.things
 
-```xtend
+```
 Thing ahawastecollection:collectionSchedule:wasteCollectionSchedule "aha Abfuhrkalender" [ commune="Isernhagen", street="67269@Rosmarinweg+/+Kirchhorst@Kirchhorst", houseNumber="10", houseNumberAddon="", collectionPlace="67269-0010+" ] 
 ```
 
 wasteCollection.items
 
-```xtend
+```
 DateTime collectionDay_generalWaste "Next genral waste collection" {channel="ahawastecollection:collectionSchedule:wasteCollectionSchedule:generalWaste"}
 DateTime collectionDay_leightweightPackaging "Next lightweight packaging collection" {channel="ahawastecollection:collectionSchedule:wasteCollectionSchedule:leightweightPackaging"}
 DateTime collectionDay_bioWaste "Next bio waste collection" {channel="ahawastecollection:collectionSchedule:wasteCollectionSchedule:bioWaste"}

bundles/org.openhab.binding.ahawastecollection/pom.xml

@@ -12,7 +12,7 @@
 
   <artifactId>org.openhab.binding.ahawastecollection</artifactId>
 
-  <name>openHAB Add-ons :: Bundles :: AhaWasteCollection Binding</name>
+  <name>openHAB Add-ons :: Bundles :: aha Waste Collection Binding</name>
 
   <dependencies>
     <dependency>

bundles/org.openhab.binding.ahawastecollection/src/main/feature/feature.xml

@@ -2,7 +2,7 @@
 <features name="org.openhab.binding.ahawastecollection-${project.version}" xmlns="http://karaf.apache.org/xmlns/features/v1.4.0">
 	<repository>mvn:org.openhab.core.features.karaf/org.openhab.core.features.karaf.openhab-core/${ohc.version}/xml/features</repository>
 
-	<feature name="openhab-binding-ahawastecollection" description="AhaWasteCollection Binding" version="${project.version}">
+	<feature name="openhab-binding-ahawastecollection" description="aha Waste Collection Binding" version="${project.version}">
 		<feature>openhab-runtime-base</feature>
 		<bundle start-level="80">mvn:org.openhab.addons.bundles/org.openhab.binding.ahawastecollection/${project.version}</bundle>
 	</feature>

bundles/org.openhab.binding.ahawastecollection/src/main/java/org/openhab/binding/ahawastecollection/internal/AhaCollectionScheduleFactory.java

@@ -16,7 +16,7 @@
 
 /**
  * Factory for creating an {@link AhaCollectionSchedule}.
- * 
+ *
  * @author Sönke Küper - Initial contribution
  */
 @NonNullByDefault
@@ -25,6 +25,6 @@ public interface AhaCollectionScheduleFactory {
     /**
      * Creates an new {@link AhaCollectionSchedule} for the given location.
      */
-    public AhaCollectionSchedule create(final String gemeinde, final String strasse, final String hausnr,
-            final String hausnraddon, final String ladeort);
+    public AhaCollectionSchedule create(final String commune, final String street, final String houseNumber,
+            final String houseNumberAddon, final String collectionPlace);
 }

bundles/org.openhab.binding.ahawastecollection/src/main/java/org/openhab/binding/ahawastecollection/internal/AhaCollectionScheduleImpl.java

@@ -37,7 +37,7 @@
 
 /**
  * Schedule that returns the next collection dates from the aha website.
- * 
+ *
  * @author Sönke Küper - Initial contribution
  */
 @NonNullByDefault
@@ -48,33 +48,33 @@ final class AhaCollectionScheduleImpl implements AhaCollectionSchedule {
     private static final String WEBSITE_URL = "https://www.aha-region.de/abholtermine/abfuhrkalender/";
 
     private final Logger logger = LoggerFactory.getLogger(AhaCollectionScheduleImpl.class);
-    private final String gemeinde;
-    private final String strasse;
-    private final String hausnr;
-    private final String hausnraddon;
-    private final String ladeort;
+    private final String commune;
+    private final String street;
+    private final String houseNumber;
+    private final String houseNumberAddon;
+    private final String collectionPlace;
 
     /**
      * Creates an new {@link AhaCollectionScheduleImpl} for the given location.
      */
-    public AhaCollectionScheduleImpl(final String gemeinde, final String strasse, final String hausnr,
-            final String hausnraddon, final String ladeort) {
-        this.gemeinde = gemeinde;
-        this.strasse = strasse;
-        this.hausnr = hausnr;
-        this.hausnraddon = hausnraddon;
-        this.ladeort = ladeort;
+    public AhaCollectionScheduleImpl(final String commune, final String street, final String houseNumber,
+            final String houseNumberAddon, final String collectionPlace) {
+        this.commune = commune;
+        this.street = street;
+        this.houseNumber = houseNumber;
+        this.houseNumberAddon = houseNumberAddon;
+        this.collectionPlace = collectionPlace;
     }
 
     @Override
     public Map<WasteType, CollectionDate> getCollectionDates() throws IOException {
         final Document doc = Jsoup.connect(WEBSITE_URL) //
                 .method(Method.POST) //
-                .data("gemeinde", this.gemeinde) //
-                .data("strasse", this.strasse) //
-                .data("hausnr", this.hausnr) //
-                .data("hausnraddon", this.hausnraddon) //
-                .data("ladeort", this.ladeort) //
+                .data("gemeinde", this.commune) //
+                .data("strasse", this.street) //
+                .data("hausnr", this.houseNumber) //
+                .data("hausnraddon", this.houseNumberAddon) //
+                .data("ladeort", this.collectionPlace) //
                 .data("anzeigen", "Suchen") //
                 .get();
         final Elements tableRows = doc.select("table").select("tr");
@@ -98,11 +98,10 @@ public Map<WasteType, CollectionDate> getCollectionDates() throws IOException {
 
     /**
      * Parses the {@link CollectionDate} from the given {@link Element table row}.
-     * 
+     *
      * @return The {@link CollectionDate} or <code>null</code> if no dates could be parsed.
      */
-    @Nullable
-    private CollectionDate parseRow(final Element row) {
+    private @Nullable CollectionDate parseRow(final Element row) {
         final Elements columns = row.select("td");
         if (columns.size() != 5) {
             this.logger.debug("Could not parse row: {}", row.toString());
@@ -143,7 +142,7 @@ private List<Date> parseTimes(final Element element) {
             try {
                 result.add(DATE_FORMAT.parse(dateValue));
             } catch (final ParseException e) {
-                this.logger.warn("Could not parse date: {}", dateValue); //$NON-NLS-1$
+                this.logger.warn("Could not parse date: {}", dateValue);
             }
         }
         return result;

bundles/org.openhab.binding.ahawastecollection/src/main/java/org/openhab/binding/ahawastecollection/internal/AhaWasteCollectionHandler.java

@@ -64,10 +64,10 @@ public class AhaWasteCollectionHandler extends BaseThingHandler {
 
     private @Nullable ScheduledCompletableFuture<?> dailyJob;
 
-    private AhaCollectionScheduleFactory scheduleFactory;
+    private final AhaCollectionScheduleFactory scheduleFactory;
 
     public AhaWasteCollectionHandler(final Thing thing, final CronScheduler scheduler,
-            final TimeZoneProvider timeZoneProvider, AhaCollectionScheduleFactory scheduleFactory) {
+            final TimeZoneProvider timeZoneProvider, final AhaCollectionScheduleFactory scheduleFactory) {
         super(thing);
         this.cronScheduler = scheduler;
         this.timeZoneProvider = timeZoneProvider;
@@ -77,8 +77,10 @@ public AhaWasteCollectionHandler(final Thing thing, final CronScheduler schedule
 
     private Map<WasteType, CollectionDate> loadCollectionDates() {
         try {
-            return this.collectionSchedule.getCollectionDates();
-        } catch (IOException e) {
+            final Map<WasteType, CollectionDate> collectionDates = this.collectionSchedule.getCollectionDates();
+            this.updateStatus(ThingStatus.ONLINE);
+            return collectionDates;
+        } catch (final IOException e) {
             this.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
             return Collections.emptyMap();
         }
@@ -97,19 +99,20 @@ public void handleCommand(final ChannelUID channelUID, final Command command) {
     public void initialize() {
         this.config = this.getConfigAs(AhaWasteCollectionConfiguration.class);
 
-        final String gemeinde = this.config.commune;
+        final String commune = this.config.commune;
         final String street = this.config.street;
-        final String hausnr = this.config.houseNumber;
-        final String hausnraddon = this.config.houseNumberAddon;
-        final String ladeort = this.config.collectionPlace;
+        final String houseNumber = this.config.houseNumber;
+        final String houseNumberAddon = this.config.houseNumberAddon;
+        final String collectionPlace = this.config.collectionPlace;
 
-        if (gemeinde.isBlank() || street.isBlank() || hausnr.isBlank() || ladeort.isBlank()) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
+        if (commune.isBlank() || street.isBlank() || houseNumber.isBlank() || collectionPlace.isBlank()) {
+            this.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
                     "Parameters are mandatory and must be configured");
             return;
         }
 
-        this.collectionSchedule = scheduleFactory.create(gemeinde, street, hausnr, hausnraddon, ladeort);
+        this.collectionSchedule = this.scheduleFactory.create(commune, street, houseNumber, houseNumberAddon,
+                collectionPlace);
 
         this.updateStatus(ThingStatus.UNKNOWN);
 
@@ -133,7 +136,7 @@ private void restartJob() {
                 this.dailyJob = this.cronScheduler.schedule(this::updateCollectionDates, DAILY_MIDNIGHT);
                 this.logger.debug("Scheduled {} at midnight", this.dailyJob);
                 // Execute daily startup job immediately
-                updateCollectionDates();
+                this.updateCollectionDates();
             }
         } finally {
             this.monitor.unlock();
@@ -164,8 +167,6 @@ private boolean updateCollectionDates() {
 
         this.logger.debug("Retrieved {} collection entries.", collectionDates.size());
         this.updateChannels(collectionDates);
-
-        this.updateStatus(ThingStatus.ONLINE);
         return true;
     }
 
@@ -178,7 +179,7 @@ private void updateChannels(final Map<WasteType, CollectionDate> collectionDates
 
             final CollectionDate collectionDate = collectionDates.get(wasteType);
             if (collectionDate == null) {
-                this.logger.warn("No collection dates found for waste type: {}", wasteType); //$NON-NLS-1$
+                this.logger.warn("No collection dates found for waste type: {}", wasteType);
                 continue;
             }
 

bundles/org.openhab.binding.ahawastecollection/src/main/java/org/openhab/binding/ahawastecollection/internal/AhaWasteCollectionHandlerFactory.java

@@ -62,9 +62,10 @@ public AhaWasteCollectionHandlerFactory(final @Reference CronScheduler scheduler
             final AhaCollectionScheduleFactory factory = new AhaCollectionScheduleFactory() {
 
                 @Override
-                public AhaCollectionScheduleImpl create(final String gemeinde, final String strasse, String hausnr,
-                        String hausnraddon, String ladeort) {
-                    return new AhaCollectionScheduleImpl(gemeinde, strasse, hausnr, hausnraddon, ladeort);
+                public AhaCollectionScheduleImpl create(final String commune, final String street,
+                        final String houseNumber, final String houseNumberAddon, final String collectionPlace) {
+                    return new AhaCollectionScheduleImpl(commune, street, houseNumber, houseNumberAddon,
+                            collectionPlace);
                 }
             };
             return new AhaWasteCollectionHandler(thing, this.scheduler, this.timeZoneProvider, factory);

bundles/org.openhab.binding.ahawastecollection/src/main/resources/OH-INF/binding/binding.xml

@@ -4,7 +4,10 @@
 	xsi:schemaLocation="https://openhab.org/schemas/binding/v1.0.0 https://openhab.org/schemas/binding-1.0.0.xsd">
 
 	<name>aha WasteCollection Binding</name>
-	<description>This binding requests the next waste collection dates from the aha collection schedule served by
-		https://www.aha-region.de/abholtermine/abfuhrkalender.</description>
+	<description>This binding provides information about the upcoming waste collection dates for places, that are served by
+		aha,
+		the waste collection company of the region Hannover. The values are retrieved from the online
+		aha waste collection
+		schedule available at: https://www.aha-region.de/abholtermine/abfuhrkalender.</description>
 
 </binding:binding>

bundles/org.openhab.binding.ahawastecollection/src/main/resources/OH-INF/thing/thing-types.xml

@@ -19,23 +19,25 @@
 		<config-description>
 			<parameter name="commune" type="text" required="true">
 				<label>Commune</label>
-				<description>Commune from form input</description>
+				<description>The selected commune, taken from the form field 'gemeinde'. </description>
 			</parameter>
 			<parameter name="street" type="text" required="true">
 				<label>Street</label>
-				<description>Street from selected street value. For example "02095@Oesterleystr.+/+Südstadt@Südstadt"</description>
+				<description>The selected street, taken from the form field 'strasse'. This value must look like
+					67269@Rosmarinweg+/+Kirchhorst@Kirchhorst</description>
 			</parameter>
 			<parameter name="houseNumber" type="text" required="true">
-				<label>House number</label>
-				<description>House number from form input</description>
+				<label>House Number</label>
+				<description>The selected house number, taken from the form field 'hausnr'.</description>
 			</parameter>
 			<parameter name="houseNumberAddon" type="text" required="false">
-				<label>House number addon from form input, may be empty</label>
-				<description></description>
+				<label>House Number Addon</label>
+				<description>The selected house number addon, taken from the form field 'hausnraddon'.</description>
 			</parameter>
 			<parameter name="collectionPlace" type="text" required="true">
-				<label>Collection place</label>
-				<description>Collection place from form input, for example: "02095-0010+"</description>
+				<label>Collection Place</label>
+				<description>Form value for the collection place, taken from the form field `ladeort`. This value must look like
+					67269-0010+</description>
 			</parameter>
 		</config-description>
 	</thing-type>

bundles/org.openhab.binding.ahawastecollection/src/test/java/org/openhab/binding/ahawastecollection/internal/AhaCollectionScheduleStubFactory.java

@@ -21,8 +21,8 @@
 public final class AhaCollectionScheduleStubFactory implements AhaCollectionScheduleFactory {
 
     @Override
-    public AhaCollectionSchedule create(final String gemeinde, final String strasse, final String hausnr,
-            final String hausnraddon, final String ladeort) {
+    public AhaCollectionSchedule create(final String commune, final String street, final String houseNumber,
+            final String houseNumberAddon, final String collectionPlace) {
         return new AhaCollectionScheduleStub();
     }
 }