[nibeuplink] Fix NPEs, minor refactoring (#14508)

* improved code quality: added some additional null checks, removed some obsolete null checks. simplified some code sections.

Signed-off-by: Alexander Friese <af944580@googlemail.com>
This commit is contained in:
alexf2015 2023-03-20 20:09:20 +01:00 committed by GitHub
parent dae33c405e
commit 99087f08c3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 113 additions and 132 deletions

View File

@ -87,6 +87,10 @@ public final class NibeUplinkBindingConstants {
public static final String CHANNEL_TYPE_HW_MODE_RW = "rwtype-hw-mode";
public static final String CHANNEL_TYPE_FAN_SPEED_RW = "rwtype-fan-speed";
// Status Keys
public static final String STATUS_INVALID_NIBE_ID = "@text/status.invalid.nibeId";
public static final String STATUS_INVALID_CREDENTIALS = "@text/status.invalid.credentials";
// URLs
public static final String LOGIN_URL = "https://www.nibeuplink.com/LogIn";
public static final String DATA_API_URL = "https://www.nibeuplink.com/PrivateAPI/QueueValues";

View File

@ -167,8 +167,11 @@ public abstract class AbstractUplinkCommandCallback extends BufferingResponseLis
protected abstract String getURL();
@Override
public final @Nullable StatusUpdateListener getListener() {
return listener;
public final void updateListenerStatus() {
StatusUpdateListener listener = this.listener;
if (listener != null) {
listener.update(communicationStatus);
}
}
@Override

View File

@ -26,7 +26,6 @@ import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.util.Fields;
import org.openhab.binding.nibeuplink.internal.callback.AbstractUplinkCommandCallback;
import org.openhab.binding.nibeuplink.internal.connector.StatusUpdateListener;
import org.openhab.binding.nibeuplink.internal.handler.NibeUplinkHandler;
import org.openhab.binding.nibeuplink.internal.model.DataResponse;
import org.openhab.binding.nibeuplink.internal.model.DataResponseTransformer;
@ -83,10 +82,7 @@ public class GenericStatusUpdate extends AbstractUplinkCommandCallback implement
logger.debug("onComplete()");
if (!HttpStatus.Code.OK.equals(getCommunicationStatus().getHttpCode()) && retries++ < MAX_RETRIES) {
StatusUpdateListener listener = getListener();
if (listener != null) {
listener.update(getCommunicationStatus());
}
updateListenerStatus();
handler.getWebInterface().enqueueCommand(this);
} else {
String json = getContentAsString(StandardCharsets.UTF_8);

View File

@ -59,9 +59,6 @@ public class Login extends AbstractUplinkCommandCallback implements NibeUplinkCo
@Override
public void onComplete(@Nullable Result result) {
StatusUpdateListener listener = getListener();
if (listener != null) {
listener.update(getCommunicationStatus());
}
updateListenerStatus();
}
}

View File

@ -13,7 +13,6 @@
package org.openhab.binding.nibeuplink.internal.command;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.Response.CompleteListener;
import org.eclipse.jetty.client.api.Response.ContentListener;
@ -39,12 +38,10 @@ public interface NibeUplinkCommand extends SuccessListener, FailureListener, Con
void performAction(HttpClient asyncclient);
/**
* get the current listener
* update the status of the registered listener instance
*
* @return instance of the listener, might be null.
*/
@Nullable
StatusUpdateListener getListener();
void updateListenerStatus();
/**
* register a listener

View File

@ -70,8 +70,7 @@ public class UpdateSetting extends AbstractUplinkCommandCallback implements Nibe
ChannelTypeUID typeUID = channel.getChannelTypeUID();
String channelId = channel.getUID().getIdWithoutGroup();
if (typeUID == null || typeUID.getId() == null
|| !typeUID.getId().startsWith(NibeUplinkBindingConstants.RW_CHANNEL_PREFIX)) {
if (typeUID == null || !typeUID.getId().startsWith(NibeUplinkBindingConstants.RW_CHANNEL_PREFIX)) {
logger.info("channel '{}' does not support write access - value to set '{}'", channelId, value);
throw new UnsupportedOperationException("channel (" + channelId + ") does not support write access");
}

View File

@ -47,7 +47,7 @@ public class CommunicationStatus {
public final String getMessage() {
Exception err = error;
String errMsg = err == null ? err.getMessage() : null;
String errMsg = err == null ? null : err.getMessage();
String msg = getHttpCode().getMessage();
if (errMsg != null && !errMsg.isEmpty()) {
return errMsg;

View File

@ -27,7 +27,6 @@ import org.eclipse.jetty.util.BlockingArrayQueue;
import org.openhab.binding.nibeuplink.internal.AtomicReferenceTrait;
import org.openhab.binding.nibeuplink.internal.command.Login;
import org.openhab.binding.nibeuplink.internal.command.NibeUplinkCommand;
import org.openhab.binding.nibeuplink.internal.config.NibeUplinkConfiguration;
import org.openhab.binding.nibeuplink.internal.handler.NibeUplinkHandler;
import org.openhab.core.thing.ThingStatus;
import org.openhab.core.thing.ThingStatusDetail;
@ -42,15 +41,8 @@ import org.slf4j.LoggerFactory;
@NonNullByDefault
public class UplinkWebInterface implements AtomicReferenceTrait {
private static final int NIBE_ID_THRESHOLD = 14;
private final Logger logger = LoggerFactory.getLogger(UplinkWebInterface.class);
/**
* Configuration
*/
private NibeUplinkConfiguration config;
/**
* handler for updating thing status
*/
@ -88,7 +80,6 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
*
* @author afriese - initial contribution
*/
@NonNullByDefault
private class WebRequestExecutor implements Runnable {
/**
@ -126,43 +117,93 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
*/
@Override
public void run() {
logger.debug("run queued commands, queue size is {}", commandQueue.size());
if (!isAuthenticated()) {
authenticate();
}
else if (isAuthenticated() && !commandQueue.isEmpty()) {
StatusUpdateListener statusUpdater = new StatusUpdateListener() {
@Override
public void update(CommunicationStatus status) {
switch (status.getHttpCode()) {
case SERVICE_UNAVAILABLE:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE,
status.getMessage());
setAuthenticated(false);
break;
case FOUND:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
"most likely your NibeId is wrong. please check your NibeId.");
setAuthenticated(false);
break;
case OK:
// no action needed as the thing is already online.
break;
default:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
status.getMessage());
setAuthenticated(false);
}
}
};
NibeUplinkCommand command = commandQueue.poll();
if (command != null) {
command.setListener(statusUpdater);
command.performAction(httpClient);
} else if (isAuthenticated() && !commandQueue.isEmpty()) {
try {
executeCommand();
} catch (Exception ex) {
logger.warn("command execution ended with exception:", ex);
}
}
}
/**
* executes the next command in the queue. requires authenticated session.
*
* @throws ValidationException
*/
private void executeCommand() {
NibeUplinkCommand command = commandQueue.poll();
if (command != null) {
command.setListener(this::processExecutionResult);
command.performAction(httpClient);
}
}
/**
* callback that handles result from command execution.
*
* @param status status information to be evaluated
*/
private void processExecutionResult(CommunicationStatus status) {
switch (status.getHttpCode()) {
case SERVICE_UNAVAILABLE:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE,
status.getMessage());
setAuthenticated(false);
break;
case FOUND:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
STATUS_INVALID_NIBE_ID);
setAuthenticated(false);
break;
case OK:
// no action needed as the thing is already online.
break;
default:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
status.getMessage());
setAuthenticated(false);
}
}
/**
* authenticates with the Nibe Uplink WEB interface
*/
private synchronized void authenticate() {
setAuthenticated(false);
new Login(uplinkHandler, this::processAuthenticationResult).performAction(httpClient);
}
/**
* callback that handles result from authentication.
*
* @param status status information to be evaluated
*/
private void processAuthenticationResult(CommunicationStatus status) {
switch (status.getHttpCode()) {
case FOUND:
uplinkHandler.setStatusInfo(ThingStatus.ONLINE, ThingStatusDetail.NONE, null);
setAuthenticated(true);
break;
case OK:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
STATUS_INVALID_CREDENTIALS);
setAuthenticated(false);
break;
case SERVICE_UNAVAILABLE:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE,
status.getMessage());
setAuthenticated(false);
break;
default:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
status.getMessage());
setAuthenticated(false);
}
}
}
/**
@ -171,7 +212,6 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
* @param config the Bridge configuration
*/
public UplinkWebInterface(ScheduledExecutorService scheduler, NibeUplinkHandler handler, HttpClient httpClient) {
this.config = handler.getConfiguration();
this.uplinkHandler = handler;
this.scheduler = scheduler;
this.requestExecutor = new WebRequestExecutor();
@ -182,7 +222,6 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
* starts the periodic request executor job which handles all web requests
*/
public void start() {
this.config = uplinkHandler.getConfiguration();
setAuthenticated(false);
updateJobReference(requestExecutorJobReference, scheduler.scheduleWithFixedDelay(requestExecutor,
WEB_REQUEST_INITIAL_DELAY, WEB_REQUEST_INTERVAL, TimeUnit.MILLISECONDS));
@ -197,72 +236,6 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
requestExecutor.enqueue(command);
}
/**
* authenticates with the Nibe Uplink WEB interface
*/
private synchronized void authenticate() {
setAuthenticated(false);
if (preCheck()) {
StatusUpdateListener statusUpdater = new StatusUpdateListener() {
@Override
public void update(CommunicationStatus status) {
switch (status.getHttpCode()) {
case FOUND:
uplinkHandler.setStatusInfo(ThingStatus.ONLINE, ThingStatusDetail.NONE, "logged in");
setAuthenticated(true);
break;
case OK:
uplinkHandler.setStatusInfo(ThingStatus.UNKNOWN, ThingStatusDetail.CONFIGURATION_ERROR,
"invalid username or password");
setAuthenticated(false);
break;
case SERVICE_UNAVAILABLE:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE,
status.getMessage());
setAuthenticated(false);
break;
default:
uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
status.getMessage());
setAuthenticated(false);
}
}
};
new Login(uplinkHandler, statusUpdater).performAction(httpClient);
}
}
/**
* performs some pre cheks on configuration before attempting to login
*
* @return true on success, false otherwise
*/
private boolean preCheck() {
String preCheckStatusMessage = "";
String localPassword = config.getPassword();
String localUser = config.getUser();
String localNibeId = config.getNibeId();
if (localPassword == null || localPassword.isEmpty()) {
preCheckStatusMessage = "please configure password first";
} else if (localUser == null || localUser.isEmpty()) {
preCheckStatusMessage = "please configure user first";
} else if (localNibeId == null || localNibeId.isEmpty()) {
preCheckStatusMessage = "please configure nibeId first";
} else if (localNibeId.length() > NIBE_ID_THRESHOLD) {
preCheckStatusMessage = "your NibeId is too long. Please refer to the documentation on how to set this value.";
} else {
return true;
}
this.uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
preCheckStatusMessage);
return false;
}
/**
* will be called by the ThingHandler to abort periodic jobs.
*/

View File

@ -15,6 +15,7 @@ package org.openhab.binding.nibeuplink.internal.handler;
import java.util.Map;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.nibeuplink.internal.config.NibeUplinkConfiguration;
import org.openhab.binding.nibeuplink.internal.connector.UplinkWebInterface;
import org.openhab.core.thing.Channel;
@ -38,7 +39,7 @@ public interface NibeUplinkHandler extends ThingHandler, ChannelProvider {
* @param statusDetail Bridge status detail
* @param description Bridge status description
*/
void setStatusInfo(ThingStatus status, ThingStatusDetail statusDetail, String description);
void setStatusInfo(ThingStatus status, ThingStatusDetail statusDetail, @Nullable String description);
/**
* Provides the web interface object.

View File

@ -87,8 +87,7 @@ public abstract class UplinkBaseHandler extends BaseThingHandler implements Nibe
Channel channel = getSpecificChannel(channelUID.getIdWithoutGroup());
if (channel != null) {
ChannelTypeUID typeUID = channel.getChannelTypeUID();
if (typeUID != null && typeUID.getId() != null
&& typeUID.getId().startsWith(NibeUplinkBindingConstants.RW_CHANNEL_PREFIX)) {
if (typeUID != null && typeUID.getId().startsWith(NibeUplinkBindingConstants.RW_CHANNEL_PREFIX)) {
webInterface.enqueueCommand(new UpdateSetting(this, channel, command));
}
}
@ -192,7 +191,7 @@ public abstract class UplinkBaseHandler extends BaseThingHandler implements Nibe
}
@Override
public void setStatusInfo(ThingStatus status, ThingStatusDetail statusDetail, String description) {
public void setStatusInfo(ThingStatus status, ThingStatusDetail statusDetail, @Nullable String description) {
super.updateStatus(status, statusDetail, description);
}

View File

@ -12,11 +12,14 @@
*/
package org.openhab.binding.nibeuplink.internal.model;
import org.eclipse.jdt.annotation.NonNullByDefault;
/**
* used to determine the group a channel belongs to
*
* @author Alexander Friese - initial contribution
*/
@NonNullByDefault
public enum ChannelGroup {
BASE,
GENERAL,

View File

@ -30,6 +30,7 @@ import org.openhab.core.library.unit.Units;
import org.openhab.core.thing.Channel;
import org.openhab.core.thing.type.ChannelTypeUID;
import org.openhab.core.types.State;
import org.openhab.core.types.UnDefType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -63,6 +64,9 @@ public class DataResponseTransformer {
if (channel == null) {
// This should not happen but we want to get informed about it
logger.warn("Channel not found: {}", channelId);
} else if (value == null) {
logger.debug("null value for channel: {}", channelId);
result.put(channel, UnDefType.UNDEF);
} else {
ChannelTypeUID typeUID = channel.getChannelTypeUID();
String type = typeUID == null ? "null" : typeUID.getId();

View File

@ -467,3 +467,8 @@ channel-type.nibeuplink.type-switch.label = Unnamed Switch
channel-type.nibeuplink.type-temperature.label = Unnamed Temperature
channel-type.nibeuplink.type-time-scale10.label = Unnamed Time
channel-type.nibeuplink.type-time-unscaled.label = Unnamed Time
# status translations
status.invalid.nibeId = "Most likely your NibeId is wrong. Please check your NibeId"
status.invalid.credentials = "Invalid username or password"