Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[avmfritz] Added initial refresh of Call Monitor channels and improved thread handling #9734

Merged
merged 7 commits into from
Jan 17, 2021
Merged

[avmfritz] Added initial refresh of Call Monitor channels and improved thread handling #9734

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
239b0f1
Added initial refresh of Call Monitor channels
cweitkamp Jan 7, 2021
d03174b
Improved CallMonitorThread handling
cweitkamp Jan 8, 2021
f3a311a
Incorporated commentd from review
cweitkamp Jan 12, 2021
cedb203
Incorporated commentd from review
cweitkamp Jan 12, 2021
1533478
Added UncaughtExceptionHandler
cweitkamp Jan 14, 2021
915f8d4
Incorporated comments from review
cweitkamp Jan 14, 2021
b845dc6
Incorporated comments from review
cweitkamp Jan 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 30 additions & 20 deletions ...g.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/callmonitor/CallEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@
@NonNullByDefault
public class CallEvent {

public static final String CALL_TYPE_CALL = "CALL";
public static final String CALL_TYPE_CONNECT = "CONNECT";
public static final String CALL_TYPE_RING = "RING";
public static final String CALL_TYPE_DISCONNECT = "DISCONNECT";

private final String rawEvent;
private final String timestamp;
private final String callType;
Expand All @@ -47,26 +52,31 @@ public CallEvent(String rawEvent) {
callType = fields[1];
id = fields[2];

if (callType.equals("RING")) {
externalNo = fields[3];
internalNo = fields[4];
connectionType = fields[5];
} else if (callType.equals("CONNECT")) {
line = fields[3];
if (fields.length > 4) {
externalNo = fields[4];
} else {
externalNo = "Unknown";
}
} else if (callType.equals("CALL")) {
line = fields[3];
internalNo = fields[4];
externalNo = fields[5];
connectionType = fields[6];
} else if (callType.equals("DISCONNECT")) {
// no fields to set
} else {
throw new IllegalArgumentException("Invalid call type: " + callType);
switch (callType) {
case CALL_TYPE_RING:
externalNo = fields[3];
internalNo = fields[4];
connectionType = fields[5];
break;
case CALL_TYPE_CONNECT:
line = fields[3];
if (fields.length > 4) {
externalNo = fields[4];
} else {
externalNo = "Unknown";
}
break;
case CALL_TYPE_CALL:
line = fields[3];
internalNo = fields[4];
externalNo = fields[5];
connectionType = fields[6];
break;
case CALL_TYPE_DISCONNECT:
// no fields to set
break;
default:
throw new IllegalArgumentException("Invalid call type: " + callType);
}
}

Expand Down
100 changes: 47 additions & 53 deletions ...avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/callmonitor/CallMonitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
*/
package org.openhab.binding.avmfritz.internal.callmonitor;

import static org.openhab.binding.avmfritz.internal.AVMFritzBindingConstants.*;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
Expand All @@ -22,7 +24,6 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.avmfritz.internal.AVMFritzBindingConstants;
import org.openhab.binding.avmfritz.internal.handler.BoxHandler;
import org.openhab.core.library.types.StringListType;
import org.openhab.core.thing.ThingStatus;
Expand All @@ -32,7 +33,7 @@
import org.slf4j.LoggerFactory;

/**
* This class handles all communication with the call monitor port of the fritzbox.
* This class handles all communication with the Call Monitor port of the FRITZ!Box.
*
* @author Kai Kreuzer - Initial contribution
*/
Expand All @@ -41,8 +42,8 @@ public class CallMonitor {

protected final Logger logger = LoggerFactory.getLogger(CallMonitor.class);

// port number to connect to fritzbox
private final int MONITOR_PORT = 1012;
// port number to connect to FRITZ!Box
private static final int MONITOR_PORT = 1012;

private @Nullable CallMonitorThread monitorThread;
private final ScheduledFuture<?> reconnectJob;
Expand All @@ -68,17 +69,26 @@ public CallMonitor(String ip, BoxHandler handler, ScheduledExecutorService sched
thread.start();
this.monitorThread = thread;
}, 0, 2, TimeUnit.HOURS);
// initialize states of call monitor channels
resetChannels();
}

/**
* Reset channels.
*/
public void resetChannels() {
handler.updateState(CHANNEL_CALL_INCOMING, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_OUTGOING, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_ACTIVE, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_STATE, CALL_STATE_IDLE);
}

/**
* Cancel the reconnect job.
*/
public void dispose() {
reconnectJob.cancel(true);
CallMonitorThread monitorThread = this.monitorThread;
if (monitorThread != null) {
monitorThread.interrupt();
}
stopThread();
}

public class CallMonitorThread extends Thread {
Expand All @@ -92,9 +102,6 @@ public class CallMonitorThread extends Thread {
// time to wait before reconnecting
private long reconnectTime = 60000L;

public CallMonitorThread() {
}

@Override
public void run() {
while (!interrupted) {
Expand All @@ -106,7 +113,7 @@ public void run() {
reader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
// reset the retry interval
reconnectTime = 60000L;
} catch (Exception e) {
} catch (IOException e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a custom thread uncaught runtime exceptions will not be logged if they occur. To make troubleshooting easier I think that you should add an UncaughtExceptionHandler to the thread so that unexpected thread deaths can be logged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like 1533478?

handler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
"Cannot connect to Fritz!Box call monitor - make sure to enable it by dialing '#96*5'!");
logger.debug("Error attempting to connect to FritzBox. Retrying in {} seconds",
Expand Down Expand Up @@ -176,54 +183,41 @@ public void interrupt() {
* @param ce call event to process
*/
private void handleCallEvent(CallEvent ce) {
if (ce.getCallType().equals("DISCONNECT")) {
// reset states of call monitor channels
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_INCOMING, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_OUTGOING, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_ACTIVE, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_STATE,
AVMFritzBindingConstants.CALL_STATE_IDLE);
} else if (ce.getCallType().equals("RING")) { // first event when call is incoming
StringListType state = new StringListType(ce.getInternalNo(), ce.getExternalNo());
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_INCOMING, state);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_OUTGOING, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_ACTIVE, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_STATE,
AVMFritzBindingConstants.CALL_STATE_RINGING);
} else if (ce.getCallType().equals("CONNECT")) { // when call is answered/running
StringListType state = new StringListType(ce.getExternalNo(), "");
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_ACTIVE, state);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_INCOMING, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_OUTGOING, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_STATE,
AVMFritzBindingConstants.CALL_STATE_ACTIVE);
} else if (ce.getCallType().equals("CALL")) { // outgoing call
StringListType state = new StringListType(ce.getExternalNo(), ce.getInternalNo());
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_INCOMING, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_OUTGOING, state);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_ACTIVE, UnDefType.UNDEF);
handler.updateState(AVMFritzBindingConstants.CHANNEL_CALL_STATE,
AVMFritzBindingConstants.CALL_STATE_DIALING);
switch (ce.getCallType()) {
case CallEvent.CALL_TYPE_DISCONNECT:
// reset states of call monitor channels
resetChannels();
break;
case CallEvent.CALL_TYPE_RING: // first event when call is incoming
handler.updateState(CHANNEL_CALL_INCOMING,
new StringListType(ce.getInternalNo(), ce.getExternalNo()));
handler.updateState(CHANNEL_CALL_OUTGOING, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_ACTIVE, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_STATE, CALL_STATE_RINGING);
break;
case CallEvent.CALL_TYPE_CONNECT: // when call is answered/running
handler.updateState(CHANNEL_CALL_ACTIVE, new StringListType(ce.getExternalNo(), ""));
handler.updateState(CHANNEL_CALL_INCOMING, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_OUTGOING, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_STATE, CALL_STATE_ACTIVE);
break;
case CallEvent.CALL_TYPE_CALL: // outgoing call
handler.updateState(CHANNEL_CALL_INCOMING, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_OUTGOING,
new StringListType(ce.getExternalNo(), ce.getInternalNo()));
handler.updateState(CHANNEL_CALL_ACTIVE, UnDefType.UNDEF);
handler.updateState(CHANNEL_CALL_STATE, CALL_STATE_DIALING);
break;
}
}
}

public void stopThread() {
logger.debug("Stopping call monitor thread...");
if (monitorThread != null) {
monitorThread.interrupt();
monitorThread = null;
}
}

public void startThread() {
logger.debug("Starting call monitor thread...");
if (monitorThread != null) {
monitorThread.interrupt();
CallMonitorThread thread = this.monitorThread;
if (thread != null) {
thread.interrupt();
monitorThread = null;
}
// create a new thread for listening to the FritzBox
monitorThread = new CallMonitorThread();
monitorThread.start();
}
}
2 changes: 1 addition & 1 deletion ...rc/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public abstract class AVMFritzBaseBridgeHandler extends BaseBridgeHandler {
/**
* Refresh interval which is used to poll values from the FRITZ!Box web interface (optional, defaults to 15 s)
*/
private long refreshInterval = 15;
protected long refreshInterval = 15;
cweitkamp marked this conversation as resolved.
Show resolved Hide resolved

/**
* Interface object for querying the FRITZ!Box web interface
Expand Down
20 changes: 17 additions & 3 deletions ...ding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/BoxHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ public BoxHandler(Bridge bridge, HttpClient httpClient,
@Override
protected void manageConnections() {
AVMFritzBoxConfiguration config = getConfigAs(AVMFritzBoxConfiguration.class);
if (this.callMonitor == null && callChannelsLinked()) {
CallMonitor cm = this.callMonitor;
if (cm == null && callChannelsLinked()) {
this.callMonitor = new CallMonitor(config.ipAddress, this, scheduler);
} else if (this.callMonitor != null && !callChannelsLinked()) {
CallMonitor cm = this.callMonitor;
} else if (cm != null && !callChannelsLinked()) {
cm.dispose();
this.callMonitor = null;
}
Expand Down Expand Up @@ -95,4 +95,18 @@ public void dispose() {
public void updateState(String channelID, State state) {
super.updateState(channelID, state);
}

@Override
public void handleRefreshCommand() {
refreshCallMonitorChannels();
super.handleRefreshCommand();
}

private void refreshCallMonitorChannels() {
CallMonitor cm = this.callMonitor;
if (cm != null) {
// initialize states of call monitor channels
cm.resetChannels();
}
}
}