Skip to content

Commit

Permalink
Merge pull request openhab#4 from thewiep/code-review-fixes
Browse files Browse the repository at this point in the history 
Code review fixes
  • Loading branch information
@thewiep
thewiep authored Oct 10, 2019
2 parents 77589a7 + ec79bc8 commit 289bd3a
Show file tree
Hide file tree
Showing 21 changed files with 92 additions and 92 deletions.
2 changes: 1 addition & 1 deletion CODEOWNERS
View file
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
/bundles/org.openhab.binding.mail/ @J-N-K
/bundles/org.openhab.binding.max/ @marcelrv
/bundles/org.openhab.binding.mcp23017/ @aogorek
/bundles/org.openhab.binding.melcloud/ @lucacalcaterra @paulianttila
/bundles/org.openhab.binding.melcloud/ @lucacalcaterra @paulianttila @thewiep
/bundles/org.openhab.binding.meteoblue/ @9037568
/bundles/org.openhab.binding.meteostick/ @cdjackson
/bundles/org.openhab.binding.miele/ @kgoderis
Expand Down
2 changes: 0 additions & 2 deletions bundles/org.openhab.binding.melcloud/NOTICE
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,3 @@ https://www.eclipse.org/legal/epl-2.0/.
== Source Code

https://github.com/openhab/openhab2-addons

== Third-party Content
34 changes: 17 additions & 17 deletions bundles/org.openhab.binding.melcloud/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,19 +82,19 @@ A.C. device configuration:

A.C. device channels

| Channel | Type | Description | Read Only |
|---------------------|--------------------|----------------------------------------------------------------------------|-----------|
| power | Switch | Power Status of Device. | False |
| operationMode | Number | Operation mode: 1 = Heat, 2 = Dry, 3 = Cool, 7 = Fan, 8 = Auto. | False |
| setTemperature | Number:Temperature | Set Temperature: Min = 10, Max = 40. | False |
| fanSpeed | Number | Fan speed: 0 = Auto, 1 = 1, 2 = 2, 3 = 3, 4 = 4, 5 = 5. | False |
| vaneHorizontal | Number | Vane Horizontal: 0 = Auto, 1 = 1, 2 = 2, 3 = 3, 4 = 4, 5 = 5, 12 = Swing. | False |
| vaneVertical | Number | Vane Vertical: 0 = Auto, 1 = 1, 2 = 2, 3 = 3, 4 = 4, 5 = 5, 7 = Swing. | False |
| roomTemperature | Number:Temperature | Room temperature. | True |
| lastCommunication | DateTime | Last Communication time when MELCloud communicated to the device. | True |
| nextCommunication | DateTime | Next communication time when MELCloud will communicate to the device. | True |
| offline | Switch | Is device in offline state. | True |
| hasPendingCommand | Switch | Device has a pending command(s). | True |
| Channel | Type | Description | Read Only |
|---------------------|--------------------|------------------------------------------------------------------------------------------|-----------|
| power | Switch | Power Status of Device. | False |
| operationMode | String | Operation mode: "1" = Heat, "2" = Dry, "3" = Cool, "7" = Fan, "8" = Auto. | False |
| setTemperature | Number:Temperature | Set Temperature: Min = 10, Max = 40. | False |
| fanSpeed | String | Fan speed: "0" = Auto, "1" = 1, "2" = 2, "3" = 3, "4" = 4, "5" = 5. | False |
| vaneHorizontal | String | Vane Horizontal: "0" = Auto, "1" = 1, "2" = 2, "3" = 3, "4" = 4, "5" = 5, "12" = Swing. | False |
| vaneVertical | String | Vane Vertical: "0" = Auto, "1" = 1, "2" = 2, "3" = 3, "4" = 4, "5" = 5, "7" = Swing. | False |
| roomTemperature | Number:Temperature | Room temperature. | True |
| lastCommunication | DateTime | Last Communication time when MELCloud communicated to the device. | True |
| nextCommunication | DateTime | Next communication time when MELCloud will communicate to the device. | True |
| offline | Switch | Is device in offline state. | True |
| hasPendingCommand | Switch | Device has a pending command(s). | True |


## Full Example for items configuration
Expand All @@ -111,11 +111,11 @@ Bridge melcloud:melcloudaccount:myaccount "My MELCloud account" [ username="user

```
Switch power { channel="melcloud:acdevice:myaccount:livingroom:power" }
Number operationMode { channel="melcloud:acdevice:myaccount:livingroom:operationMode" }
String operationMode { channel="melcloud:acdevice:myaccount:livingroom:operationMode" }
Number setTemperature { channel="melcloud:acdevice:myaccount:livingroom:setTemperature" }
Number fanSpeed { channel="melcloud:acdevice:myaccount:livingroom:fanSpeed" }
Number vaneHorizontal { channel="melcloud:acdevice:myaccount:livingroom:vaneHorizontal" }
Number vaneVertical { channel="melcloud:acdevice:myaccount:livingroom:vaneVertical" }
String fanSpeed { channel="melcloud:acdevice:myaccount:livingroom:fanSpeed" }
String vaneHorizontal { channel="melcloud:acdevice:myaccount:livingroom:vaneHorizontal" }
String vaneVertical { channel="melcloud:acdevice:myaccount:livingroom:vaneVertical" }
Number roomTemperature { channel="melcloud:acdevice:myaccount:livingroom:roomTemperature" }
DateTime lastCommunication { channel="melcloud:acdevice:myaccount:livingroom:lastCommunication" }
DateTime nextCommunication { channel="melcloud:acdevice:myaccount:livingroom:nextCommunication" }
Expand Down
2 changes: 1 addition & 1 deletion ...elcloud/src/main/java/org/openhab/binding/melcloud/internal/MelCloudBindingConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
* The {@link MelCloudBindingConstants} class defines common constants, which are
* used across the whole binding.
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
public class MelCloudBindingConstants {

Expand Down
30 changes: 1 addition & 29 deletions ....melcloud/src/main/java/org/openhab/binding/melcloud/internal/MelCloudHandlerFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,34 +14,25 @@

import static org.openhab.binding.melcloud.internal.MelCloudBindingConstants.*;

import java.util.HashMap;
import java.util.Map;

import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.config.discovery.DiscoveryService;
import org.eclipse.smarthome.core.thing.Bridge;
import org.eclipse.smarthome.core.thing.Thing;
import org.eclipse.smarthome.core.thing.ThingTypeUID;
import org.eclipse.smarthome.core.thing.ThingUID;
import org.eclipse.smarthome.core.thing.binding.BaseThingHandlerFactory;
import org.eclipse.smarthome.core.thing.binding.ThingHandler;
import org.eclipse.smarthome.core.thing.binding.ThingHandlerFactory;
import org.openhab.binding.melcloud.internal.discovery.MelCloudDiscoveryService;
import org.openhab.binding.melcloud.internal.handler.MelCloudAccountHandler;
import org.openhab.binding.melcloud.internal.handler.MelCloudDeviceHandler;
import org.osgi.framework.ServiceRegistration;
import org.osgi.service.component.annotations.Component;

/**
* The {@link MelCloudHandlerFactory} is responsible for creating things and thing
* handlers.
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
@Component(configurationPid = "binding.melcloud", service = ThingHandlerFactory.class)
public class MelCloudHandlerFactory extends BaseThingHandlerFactory {
private Map<ThingUID, ServiceRegistration<DiscoveryService>> discoveryServiceRegistrations = new HashMap<>();

@Override
public boolean supportsThingType(ThingTypeUID thingTypeUID) {
Expand All @@ -54,7 +45,6 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {

if (THING_TYPE_MELCLOUD_ACCOUNT.equals(thingTypeUID)) {
MelCloudAccountHandler handler = new MelCloudAccountHandler((Bridge) thing);
registerDiscoveryService(handler);
return handler;
} else if (THING_TYPE_ACDEVICE.equals(thingTypeUID)) {
MelCloudDeviceHandler handler = new MelCloudDeviceHandler(thing);
Expand All @@ -64,22 +54,4 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
return null;
}

@Override
protected void removeHandler(@NonNull ThingHandler thingHandler) {
ServiceRegistration<DiscoveryService> serviceRegistration = discoveryServiceRegistrations
.get(thingHandler.getThing().getUID());

if (serviceRegistration != null) {
serviceRegistration.unregister();
}
}

private void registerDiscoveryService(MelCloudAccountHandler handler) {
MelCloudDiscoveryService discoveryService = new MelCloudDiscoveryService(handler);

ServiceRegistration<DiscoveryService> serviceRegistration = this.bundleContext
.registerService(DiscoveryService.class, discoveryService, null);

discoveryServiceRegistrations.put(handler.getThing().getUID(), serviceRegistration);
}
}
14 changes: 8 additions & 6 deletions ....melcloud/src/main/java/org/openhab/binding/melcloud/internal/api/MelCloudConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class MelCloudConnection {
private static final String DEVICE_LIST_URL = "https://app.melcloud.com/Mitsubishi.Wifi.Client/User/ListDevices";
private static final String DEVICE_URL = "https://app.melcloud.com/Mitsubishi.Wifi.Client/Device";

private static final int TIMEOUT = 10000;
private static final int TIMEOUT_MILLISECONDS = 10000;

// Gson objects are safe to share across threads and are somewhat expensive to construct. Use a single instance.
private static final Gson gson = new GsonBuilder().excludeFieldsWithoutExposeAnnotation().create();
Expand All @@ -73,7 +73,8 @@ public void login(String username, String password, int languageId)
InputStream data = new ByteArrayInputStream(jsonReq.toString().getBytes(StandardCharsets.UTF_8));

try {
String loginResponse = HttpUtil.executeUrl("POST", LOGIN_URL, null, data, "application/json", TIMEOUT);
String loginResponse = HttpUtil.executeUrl("POST", LOGIN_URL, null, data, "application/json",
TIMEOUT_MILLISECONDS);
logger.debug("Login response: {}", loginResponse);
LoginClientResponse resp = gson.fromJson(loginResponse, LoginClientResponse.class);
if (resp.getErrorId() != null) {
Expand All @@ -86,15 +87,15 @@ public void login(String username, String password, int languageId)
sessionKey = resp.getLoginData().getContextKey();
setConnected(true);
} catch (IOException | JsonSyntaxException e) {
throw new MelCloudCommException(String.format("Login error, reason: %s", e.getMessage(), e));
throw new MelCloudCommException(String.format("Login error, reason: %s", e.getMessage()), e);
}
}

public List<Device> fetchDeviceList() throws MelCloudCommException {
if (isConnected()) {
try {
String response = HttpUtil.executeUrl("GET", DEVICE_LIST_URL, getHeaderProperties(), null, null,
TIMEOUT);
TIMEOUT_MILLISECONDS);
logger.debug("Device list response: {}", response);
List<Device> devices = new ArrayList<Device>();
ListDevicesResponse[] buildings = gson.fromJson(response, ListDevicesResponse[].class);
Expand Down Expand Up @@ -128,7 +129,8 @@ public DeviceStatus fetchDeviceStatus(int deviceId, int buildingId) throws MelCl
if (isConnected()) {
String url = DEVICE_URL + String.format("/Get?id=%d&buildingID=%d", deviceId, buildingId);
try {
String response = HttpUtil.executeUrl("GET", url, getHeaderProperties(), null, null, TIMEOUT);
String response = HttpUtil.executeUrl("GET", url, getHeaderProperties(), null, null,
TIMEOUT_MILLISECONDS);
logger.debug("Device status response: {}", response);
DeviceStatus deviceStatus = gson.fromJson(response, DeviceStatus.class);
return deviceStatus;
Expand All @@ -147,7 +149,7 @@ public DeviceStatus sendDeviceStatus(DeviceStatus deviceStatus) throws MelCloudC
InputStream data = new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8));
try {
String response = HttpUtil.executeUrl("POST", DEVICE_URL + "/SetAta", getHeaderProperties(), data,
"application/json", TIMEOUT);
"application/json", TIMEOUT_MILLISECONDS);
logger.debug("Device status sending response: {}", response);
return gson.fromJson(response, DeviceStatus.class);
} catch (IOException | JsonSyntaxException e) {
Expand Down
2 changes: 1 addition & 1 deletion ...binding.melcloud/src/main/java/org/openhab/binding/melcloud/internal/api/json/Device.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
* Device Structure.
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/

public class Device {
Expand Down
2 changes: 1 addition & 1 deletion ...ng.melcloud/src/main/java/org/openhab/binding/melcloud/internal/api/json/DeviceProps.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* Device Properties.
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
public class DeviceProps {

Expand Down
2 changes: 1 addition & 1 deletion ...g.melcloud/src/main/java/org/openhab/binding/melcloud/internal/api/json/DeviceStatus.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* Device Status data
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
* @author Pauli Anttila - Fine tuned expose annotations
*/
public class DeviceStatus {
Expand Down
2 changes: 1 addition & 1 deletion ...oud/src/main/java/org/openhab/binding/melcloud/internal/api/json/ListDevicesResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* Response of Devices List.
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
public class ListDevicesResponse {

Expand Down
2 changes: 1 addition & 1 deletion ...oud/src/main/java/org/openhab/binding/melcloud/internal/api/json/LoginClientResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* Response Data of Login.
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
public class LoginClientResponse {

Expand Down
2 changes: 1 addition & 1 deletion ...ding.melcloud/src/main/java/org/openhab/binding/melcloud/internal/api/json/LoginData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* LoginData for Login Request.
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
public class LoginData {

Expand Down
2 changes: 1 addition & 1 deletion ...binding.melcloud/src/main/java/org/openhab/binding/melcloud/internal/api/json/Preset.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* Preset data
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
public class Preset {

Expand Down
2 changes: 1 addition & 1 deletion ...ud/src/main/java/org/openhab/binding/melcloud/internal/api/json/QuantizedCoordinates.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* QuantizedCoordinates data
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
public class QuantizedCoordinates {

Expand Down
2 changes: 1 addition & 1 deletion ...ding.melcloud/src/main/java/org/openhab/binding/melcloud/internal/api/json/Structure.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* Structure Data
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
* @author Wietse van Buitenen - Add Floor and Area
*/
public class Structure {
Expand Down
2 changes: 1 addition & 1 deletion ...loud/src/main/java/org/openhab/binding/melcloud/internal/api/json/WeatherObservation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* WeatherObservation Data
* Generated with jsonschema2pojo
*
* @author LucaCalcaterra - Initial contribution
* @author Luca Calcaterra - Initial contribution
*/
public class WeatherObservation {

Expand Down
29 changes: 25 additions & 4 deletions ...c/main/java/org/openhab/binding/melcloud/internal/discovery/MelCloudDiscoveryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@
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;
import org.eclipse.smarthome.core.thing.ThingUID;
import org.eclipse.smarthome.core.thing.binding.ThingHandler;
import org.eclipse.smarthome.core.thing.binding.ThingHandlerService;
import org.openhab.binding.melcloud.internal.MelCloudBindingConstants;
import org.openhab.binding.melcloud.internal.api.json.Device;
import org.openhab.binding.melcloud.internal.exceptions.MelCloudCommException;
Expand All @@ -38,27 +42,32 @@
* @author Luca Calcaterra - Initial Contribution
* @author Pauli Anttila - Refactoring
*/
public class MelCloudDiscoveryService extends AbstractDiscoveryService {
public class MelCloudDiscoveryService extends AbstractDiscoveryService
implements DiscoveryService, ThingHandlerService {
private final Logger logger = LoggerFactory.getLogger(MelCloudDiscoveryService.class);

private static final int DISCOVER_TIMEOUT_SECONDS = 10;

private final MelCloudAccountHandler melCloudHandler;
private @NonNullByDefault({}) MelCloudAccountHandler melCloudHandler;
private ScheduledFuture<?> scanTask;

/**
* Creates a MelCloudDiscoveryService with enabled autostart.
*/
public MelCloudDiscoveryService(MelCloudAccountHandler melCloudHandler) {
public MelCloudDiscoveryService() {
super(MelCloudBindingConstants.DISCOVERABLE_THING_TYPE_UIDS, DISCOVER_TIMEOUT_SECONDS, true);
this.melCloudHandler = melCloudHandler;
}

@Override
protected void activate(Map<String, @Nullable Object> configProperties) {
super.activate(configProperties);
}

@Override
public void deactivate() {
super.deactivate();
}

@Override
@Modified
protected void modified(Map<String, @Nullable Object> configProperties) {
Expand Down Expand Up @@ -135,4 +144,16 @@ private String createLabel(Device device) {
sb.append(device.getDeviceName());
return sb.toString();
}

@Override
public void setThingHandler(@Nullable ThingHandler handler) {
if (handler instanceof MelCloudAccountHandler) {
melCloudHandler = (MelCloudAccountHandler) handler;
}
}

@Override
public @Nullable ThingHandler getThingHandler() {
return melCloudHandler;
}
}
8 changes: 8 additions & 0 deletions ...d/src/main/java/org/openhab/binding/melcloud/internal/handler/MelCloudAccountHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*/
package org.openhab.binding.melcloud.internal.handler;

import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
Expand All @@ -24,11 +25,13 @@
import org.eclipse.smarthome.core.thing.ThingStatusDetail;
import org.eclipse.smarthome.core.thing.ThingUID;
import org.eclipse.smarthome.core.thing.binding.BaseBridgeHandler;
import org.eclipse.smarthome.core.thing.binding.ThingHandlerService;
import org.eclipse.smarthome.core.types.Command;
import org.openhab.binding.melcloud.internal.api.MelCloudConnection;
import org.openhab.binding.melcloud.internal.api.json.Device;
import org.openhab.binding.melcloud.internal.api.json.DeviceStatus;
import org.openhab.binding.melcloud.internal.config.AccountConfig;
import org.openhab.binding.melcloud.internal.discovery.MelCloudDiscoveryService;
import org.openhab.binding.melcloud.internal.exceptions.MelCloudCommException;
import org.openhab.binding.melcloud.internal.exceptions.MelCloudLoginException;
import org.slf4j.Logger;
Expand All @@ -55,6 +58,11 @@ public MelCloudAccountHandler(Bridge bridge) {
super(bridge);
}

@Override
public Collection<Class<? extends ThingHandlerService>> getServices() {
return Collections.singleton(MelCloudDiscoveryService.class);
}

@Override
public void initialize() {
logger.debug("Initializing MELCloud account handler.");
Expand Down
Loading

0 comments on commit 289bd3a

Please sign in to comment.