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

[denonmarantz] Run the Telnet socket in a dedicated thread #9511

Merged
merged 2 commits into from
Dec 26, 2020
Merged

[denonmarantz] Run the Telnet socket in a dedicated thread #9511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96d5110
Run the Telnet socket in a dedicated thread, not from the shared thre…
jwveldhuis Dec 24, 2020
fa4bbb5
Processed review comments.
jwveldhuis Dec 26, 2020
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
4 changes: 2 additions & 2 deletions ...ava/org/openhab/binding/denonmarantz/internal/connector/DenonMarantzConnectorFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
public class DenonMarantzConnectorFactory {

public DenonMarantzConnector getConnector(DenonMarantzConfiguration config, DenonMarantzState state,
ScheduledExecutorService scheduler, HttpClient httpClient) {
ScheduledExecutorService scheduler, HttpClient httpClient, String thingUID) {
if (config.isTelnet()) {
return new DenonMarantzTelnetConnector(config, state, scheduler);
return new DenonMarantzTelnetConnector(config, state, scheduler, thingUID);
} else {
return new DenonMarantzHttpConnector(config, state, scheduler, httpClient);
}
Expand Down
28 changes: 13 additions & 15 deletions ...ctor/telnet/DenonMarantzTelnetClient.java → ...elnet/DenonMarantzTelnetClientThread.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
* @author Jeroen Idserda - Initial contribution (1.x Binding)
* @author Jan-Willem Veldhuis - Refactored for 2.x
*/
public class DenonMarantzTelnetClient implements Runnable {
public class DenonMarantzTelnetClientThread extends Thread {

private Logger logger = LoggerFactory.getLogger(DenonMarantzTelnetClient.class);
private Logger logger = LoggerFactory.getLogger(DenonMarantzTelnetClientThread.class);

private static final Integer RECONNECT_DELAY = 60000; // 1 minute

Expand All @@ -43,8 +43,6 @@ public class DenonMarantzTelnetClient implements Runnable {

private DenonMarantzTelnetListener listener;

private boolean running = true;

private boolean connected = false;

private Socket socket;
Expand All @@ -53,15 +51,15 @@ public class DenonMarantzTelnetClient implements Runnable {

private BufferedReader in;

public DenonMarantzTelnetClient(DenonMarantzConfiguration config, DenonMarantzTelnetListener listener) {
public DenonMarantzTelnetClientThread(DenonMarantzConfiguration config, DenonMarantzTelnetListener listener) {
logger.debug("Denon listener created");
this.config = config;
this.listener = listener;
}

@Override
public void run() {
while (running) {
while (!isInterrupted()) {
if (!connected) {
connectTelnetSocket();
}
Expand Down Expand Up @@ -90,20 +88,21 @@ public void run() {
connected = false;
}
} catch (IOException e) {
if (!Thread.currentThread().isInterrupted()) {
if (!isInterrupted()) {
// only log if we don't stop this on purpose causing a SocketClosed
logger.debug("Error in telnet connection ", e);
}
connected = false;
listener.telnetClientConnected(false);
}
} while (running && connected);
} while (!isInterrupted() && connected);
}
disconnect();
logger.debug("Stopped client thread");
}

public void sendCommand(String command) {
if (out != null) {
logger.debug("Sending command {}", command);
try {
out.write(command + '\r');
out.flush();
Expand All @@ -116,21 +115,21 @@ public void sendCommand(String command) {
}

public void shutdown() {
this.running = false;
disconnect();
}

private void connectTelnetSocket() {
disconnect();
int delay = 0;

while (this.running && (socket == null || !socket.isConnected())) {
while (!isInterrupted() && (socket == null || !socket.isConnected())) {
try {
Thread.sleep(delay);
logger.debug("Connecting to {}", config.getHost());

// Use raw socket instead of TelnetClient here because TelnetClient sends an extra newline char
// after each write which causes the connection to become unresponsive.
// Use raw socket instead of TelnetClient here because TelnetClient sends an
// extra newline char after each write which causes the connection to become
// unresponsive.
socket = new Socket();
socket.connect(new InetSocketAddress(config.getHost(), config.getTelnetPort()), TIMEOUT);
socket.setKeepAlive(true);
Expand All @@ -141,6 +140,7 @@ private void connectTelnetSocket() {

connected = true;
listener.telnetClientConnected(true);
logger.debug("Denon telnet client connected to {}", config.getHost());
} catch (IOException e) {
logger.debug("Cannot connect to {}", config.getHost(), e);
listener.telnetClientConnected(false);
Expand All @@ -149,8 +149,6 @@ private void connectTelnetSocket() {
}
delay = RECONNECT_DELAY;
}

logger.debug("Denon telnet client connected to {}", config.getHost());
}

public boolean isConnected() {
Expand Down
23 changes: 14 additions & 9 deletions ...g/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,32 @@ public class DenonMarantzTelnetConnector extends DenonMarantzConnector implement

private static final BigDecimal NINETYNINE = new BigDecimal("99");

private DenonMarantzTelnetClient telnetClient;
private DenonMarantzTelnetClientThread telnetClientThread;

private boolean displayNowplaying = false;

protected boolean disposing = false;

private Future<?> telnetStateRequest;

private Future<?> telnetRunnable;
private String thingUID;

public DenonMarantzTelnetConnector(DenonMarantzConfiguration config, DenonMarantzState state,
ScheduledExecutorService scheduler) {
ScheduledExecutorService scheduler, String thingUID) {
this.config = config;
this.scheduler = scheduler;
this.state = state;
this.thingUID = thingUID;
}

/**
* Set up the connection to the receiver. Either using Telnet or by polling the HTTP API.
*/
@Override
public void connect() {
telnetClient = new DenonMarantzTelnetClient(config, this);
telnetRunnable = scheduler.submit(telnetClient);
telnetClientThread = new DenonMarantzTelnetClientThread(config, this);
telnetClientThread.setName("OH-binding-" + thingUID);
telnetClientThread.start();
}

@Override
Expand Down Expand Up @@ -98,9 +100,12 @@ public void dispose() {
telnetStateRequest = null;
}

if (telnetClient != null && !telnetRunnable.isDone()) {
telnetRunnable.cancel(true);
telnetClient.shutdown();
if (telnetClientThread != null) {
telnetClientThread.interrupt();
// Invoke a shutdown after interrupting the thread to close the socket immediately,
// otherwise the client keeps running until a line was received from the telnet connection
telnetClientThread.shutdown();
telnetClientThread = null;
}
}

Expand Down Expand Up @@ -262,7 +267,7 @@ protected void internalSendCommand(String command) {
logger.warn("Trying to send empty command");
return;
}
telnetClient.sendCommand(command);
telnetClientThread.sendCommand(command);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion .../src/main/java/org/openhab/binding/denonmarantz/internal/handler/DenonMarantzHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ private void createConnection() {
if (connector != null) {
connector.dispose();
}
connector = connectorFactory.getConnector(config, denonMarantzState, scheduler, httpClient);
connector = connectorFactory.getConnector(config, denonMarantzState, scheduler, httpClient,
this.getThing().getUID().getAsString());
connector.connect();
}

Expand Down