From: jwveldhuis Date: Sat, 26 Dec 2020 19:21:21 +0000 (+0100) Subject: [denonmarantz] Run the Telnet socket in a dedicated thread (#9511) X-Git-Url: https://git.basschouten.com/?a=commitdiff_plain;h=d014ac6350ec97c71a80703955e8e5dcbf93ce33;p=openhab-addons.git [denonmarantz] Run the Telnet socket in a dedicated thread (#9511) * Run the Telnet socket in a dedicated thread, not from the shared thread pool. Fixes #9494 Signed-off-by: Jan-Willem Veldhuis --- diff --git a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/DenonMarantzConnectorFactory.java b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/DenonMarantzConnectorFactory.java index 50bab9745c..dac91cc8c6 100644 --- a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/DenonMarantzConnectorFactory.java +++ b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/DenonMarantzConnectorFactory.java @@ -29,9 +29,9 @@ import org.openhab.binding.denonmarantz.internal.connector.telnet.DenonMarantzTe 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); } diff --git a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetClient.java b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetClient.java deleted file mode 100644 index 425977ae28..0000000000 --- a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetClient.java +++ /dev/null @@ -1,175 +0,0 @@ -/** - * Copyright (c) 2010-2020 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0 - * - * SPDX-License-Identifier: EPL-2.0 - */ -package org.openhab.binding.denonmarantz.internal.connector.telnet; - -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.OutputStreamWriter; -import java.net.InetSocketAddress; -import java.net.Socket; -import java.net.SocketTimeoutException; - -import org.apache.commons.lang.StringUtils; -import org.openhab.binding.denonmarantz.internal.config.DenonMarantzConfiguration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Manage telnet connection to the Denon/Marantz Receiver - * - * @author Jeroen Idserda - Initial contribution (1.x Binding) - * @author Jan-Willem Veldhuis - Refactored for 2.x - */ -public class DenonMarantzTelnetClient implements Runnable { - - private Logger logger = LoggerFactory.getLogger(DenonMarantzTelnetClient.class); - - private static final Integer RECONNECT_DELAY = 60000; // 1 minute - - private static final Integer TIMEOUT = 60000; // 1 minute - - private DenonMarantzConfiguration config; - - private DenonMarantzTelnetListener listener; - - private boolean running = true; - - private boolean connected = false; - - private Socket socket; - - private OutputStreamWriter out; - - private BufferedReader in; - - public DenonMarantzTelnetClient(DenonMarantzConfiguration config, DenonMarantzTelnetListener listener) { - logger.debug("Denon listener created"); - this.config = config; - this.listener = listener; - } - - @Override - public void run() { - while (running) { - if (!connected) { - connectTelnetSocket(); - } - - do { - try { - String line = in.readLine(); - if (line == null) { - logger.debug("No more data read from client. Disconnecting.."); - listener.telnetClientConnected(false); - disconnect(); - break; - } - logger.trace("Received from {}: {}", config.getHost(), line); - if (!StringUtils.isBlank(line)) { - listener.receivedLine(line); - } - } catch (SocketTimeoutException e) { - logger.trace("Socket timeout"); - // Disconnects are not always detected unless you write to the socket. - try { - out.write('\r'); - out.flush(); - } catch (IOException e2) { - logger.debug("Error writing to socket"); - connected = false; - } - } catch (IOException e) { - if (!Thread.currentThread().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); - } - } - - public void sendCommand(String command) { - if (out != null) { - logger.debug("Sending command {}", command); - try { - out.write(command + '\r'); - out.flush(); - } catch (IOException e) { - logger.debug("Error sending command", e); - } - } else { - logger.debug("Cannot send command, no telnet connection"); - } - } - - public void shutdown() { - this.running = false; - disconnect(); - } - - private void connectTelnetSocket() { - disconnect(); - int delay = 0; - - while (this.running && (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. - socket = new Socket(); - socket.connect(new InetSocketAddress(config.getHost(), config.getTelnetPort()), TIMEOUT); - socket.setKeepAlive(true); - socket.setSoTimeout(TIMEOUT); - - in = new BufferedReader(new InputStreamReader(socket.getInputStream())); - out = new OutputStreamWriter(socket.getOutputStream(), "UTF-8"); - - connected = true; - listener.telnetClientConnected(true); - } catch (IOException e) { - logger.debug("Cannot connect to {}", config.getHost(), e); - listener.telnetClientConnected(false); - } catch (InterruptedException e) { - logger.debug("Interrupted while connecting to {}", config.getHost(), e); - } - delay = RECONNECT_DELAY; - } - - logger.debug("Denon telnet client connected to {}", config.getHost()); - } - - public boolean isConnected() { - return connected; - } - - private void disconnect() { - if (socket != null) { - logger.debug("Disconnecting socket"); - try { - socket.close(); - } catch (IOException e) { - logger.debug("Error while disconnecting telnet client", e); - } finally { - socket = null; - out = null; - in = null; - listener.telnetClientConnected(false); - } - } - } -} diff --git a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetClientThread.java b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetClientThread.java new file mode 100644 index 0000000000..809cecde70 --- /dev/null +++ b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetClientThread.java @@ -0,0 +1,173 @@ +/** + * Copyright (c) 2010-2020 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.binding.denonmarantz.internal.connector.telnet; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStreamWriter; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.SocketTimeoutException; + +import org.apache.commons.lang.StringUtils; +import org.openhab.binding.denonmarantz.internal.config.DenonMarantzConfiguration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Manage telnet connection to the Denon/Marantz Receiver + * + * @author Jeroen Idserda - Initial contribution (1.x Binding) + * @author Jan-Willem Veldhuis - Refactored for 2.x + */ +public class DenonMarantzTelnetClientThread extends Thread { + + private Logger logger = LoggerFactory.getLogger(DenonMarantzTelnetClientThread.class); + + private static final Integer RECONNECT_DELAY = 60000; // 1 minute + + private static final Integer TIMEOUT = 60000; // 1 minute + + private DenonMarantzConfiguration config; + + private DenonMarantzTelnetListener listener; + + private boolean connected = false; + + private Socket socket; + + private OutputStreamWriter out; + + private BufferedReader in; + + public DenonMarantzTelnetClientThread(DenonMarantzConfiguration config, DenonMarantzTelnetListener listener) { + logger.debug("Denon listener created"); + this.config = config; + this.listener = listener; + } + + @Override + public void run() { + while (!isInterrupted()) { + if (!connected) { + connectTelnetSocket(); + } + + do { + try { + String line = in.readLine(); + if (line == null) { + logger.debug("No more data read from client. Disconnecting.."); + listener.telnetClientConnected(false); + disconnect(); + break; + } + logger.trace("Received from {}: {}", config.getHost(), line); + if (!StringUtils.isBlank(line)) { + listener.receivedLine(line); + } + } catch (SocketTimeoutException e) { + logger.trace("Socket timeout"); + // Disconnects are not always detected unless you write to the socket. + try { + out.write('\r'); + out.flush(); + } catch (IOException e2) { + logger.debug("Error writing to socket"); + connected = false; + } + } catch (IOException e) { + 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 (!isInterrupted() && connected); + } + disconnect(); + logger.debug("Stopped client thread"); + } + + public void sendCommand(String command) { + if (out != null) { + try { + out.write(command + '\r'); + out.flush(); + } catch (IOException e) { + logger.debug("Error sending command", e); + } + } else { + logger.debug("Cannot send command, no telnet connection"); + } + } + + public void shutdown() { + disconnect(); + } + + private void connectTelnetSocket() { + disconnect(); + int delay = 0; + + 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. + socket = new Socket(); + socket.connect(new InetSocketAddress(config.getHost(), config.getTelnetPort()), TIMEOUT); + socket.setKeepAlive(true); + socket.setSoTimeout(TIMEOUT); + + in = new BufferedReader(new InputStreamReader(socket.getInputStream())); + out = new OutputStreamWriter(socket.getOutputStream(), "UTF-8"); + + 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); + } catch (InterruptedException e) { + logger.debug("Interrupted while connecting to {}", config.getHost(), e); + } + delay = RECONNECT_DELAY; + } + } + + public boolean isConnected() { + return connected; + } + + private void disconnect() { + if (socket != null) { + logger.debug("Disconnecting socket"); + try { + socket.close(); + } catch (IOException e) { + logger.debug("Error while disconnecting telnet client", e); + } finally { + socket = null; + out = null; + in = null; + listener.telnetClientConnected(false); + } + } + } +} diff --git a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetConnector.java b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetConnector.java index b5ae903f13..f3c200272d 100644 --- a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetConnector.java +++ b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/connector/telnet/DenonMarantzTelnetConnector.java @@ -46,7 +46,7 @@ public class DenonMarantzTelnetConnector extends DenonMarantzConnector implement private static final BigDecimal NINETYNINE = new BigDecimal("99"); - private DenonMarantzTelnetClient telnetClient; + private DenonMarantzTelnetClientThread telnetClientThread; private boolean displayNowplaying = false; @@ -54,13 +54,14 @@ public class DenonMarantzTelnetConnector extends DenonMarantzConnector implement 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; } /** @@ -68,8 +69,9 @@ public class DenonMarantzTelnetConnector extends DenonMarantzConnector implement */ @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 @@ -98,9 +100,12 @@ public class DenonMarantzTelnetConnector extends DenonMarantzConnector implement 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; } } @@ -262,7 +267,7 @@ public class DenonMarantzTelnetConnector extends DenonMarantzConnector implement logger.warn("Trying to send empty command"); return; } - telnetClient.sendCommand(command); + telnetClientThread.sendCommand(command); } /** diff --git a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/handler/DenonMarantzHandler.java b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/handler/DenonMarantzHandler.java index 645ce108b0..bec113b839 100644 --- a/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/handler/DenonMarantzHandler.java +++ b/bundles/org.openhab.binding.denonmarantz/src/main/java/org/openhab/binding/denonmarantz/internal/handler/DenonMarantzHandler.java @@ -314,7 +314,8 @@ public class DenonMarantzHandler extends BaseThingHandler implements DenonMarant 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(); }