Skip to content

Commit

Permalink
[smsmodem] Apply review
Browse files Browse the repository at this point in the history 
Signed-off-by: Gwendal Roulleau <[email protected]>
  • Loading branch information
@dalgwen
dalgwen committed Sep 26, 2022
1 parent 279dc21 commit 86d3991
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 58 deletions.
2 changes: 1 addition & 1 deletion ...odem/src/main/java/org/openhab/binding/smsmodem/internal/SMSModemBridgeConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
public class SMSModemBridgeConfiguration {

public String serialPort = "";
public Integer baud = 9800;
public Integer baud = 9600;
public String simPin = "";
public Integer pollingInterval = 15;
public Integer delayBetweenSend = 0;
Expand Down
2 changes: 1 addition & 1 deletion ...smsmodem/src/main/java/org/openhab/binding/smsmodem/internal/actions/SMSModemActions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void sendSMS(
if (recipient != null && !recipient.isEmpty() && message != null) {
handler.send(recipient, message, false, encoding);
} else {
logger.error("SMSModem cannot send a message with no recipient or text");
logger.warn("SMSModem cannot send a message with no recipient or text");
}
}

Expand Down
34 changes: 14 additions & 20 deletions ...m/src/main/java/org/openhab/binding/smsmodem/internal/handler/SMSConversationHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.smsmodem.internal.SMSConversationConfiguration;
import org.openhab.binding.smsmodem.internal.SMSModemBindingConstants;
import org.openhab.core.i18n.ConfigurationException;
import org.openhab.core.library.types.StringType;
import org.openhab.core.thing.Bridge;
import org.openhab.core.thing.ChannelUID;
Expand Down Expand Up @@ -56,24 +57,21 @@ public String getRecipient() {
return config.recipient.trim();
}

private synchronized @Nullable SMSModemBridgeHandler getBridgeHandler() {
private synchronized void checkBridgeHandler() {
if (this.bridgeHandler == null) {
Bridge bridge = getBridge();
if (bridge == null) {
logger.error("Required bridge not defined for SMSconversation {} with {}.", thing.getUID(),
getRecipient());
return null;
throw new ConfigurationException("Required bridge not defined for SMSconversation {} with {}.",
thing.getUID(), getRecipient());
}
ThingHandler handler = bridge.getHandler();
if (handler instanceof SMSModemBridgeHandler) {
this.bridgeHandler = (SMSModemBridgeHandler) handler;
} else {
logger.error("No available bridge handler found for SMSConversation {} bridge {} .", thing.getUID(),
bridge.getUID());
return null;
throw new ConfigurationException("No available bridge handler found for SMSConversation {} bridge {} .",
thing.getUID(), bridge.getUID());
}
}
return this.bridgeHandler;
}

protected void checkAndReceive(String sender, String text) {
Expand Down Expand Up @@ -115,29 +113,25 @@ public void send(String text) {
@Override
public void initialize() {
config = getConfigAs(SMSConversationConfiguration.class);
bridgeHandler = getBridgeHandler();
setStatusByBridgeStatus();
try {
checkBridgeHandler();
setStatusByBridgeStatus();
} catch (ConfigurationException confe) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, confe.getMessage());
}
}

private void setStatusByBridgeStatus() {
SMSModemBridgeHandler bridgeHandlerFinal = bridgeHandler;
if (bridgeHandlerFinal != null) {
switch (bridgeHandlerFinal.getThing().getStatus()) {
case INITIALIZING:
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE);
break;
case OFFLINE:
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE);
break;
case ONLINE:
updateStatus(ThingStatus.ONLINE, ThingStatusDetail.NONE);
break;
case INITIALIZING:
case OFFLINE:
case REMOVED:
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE);
break;
case REMOVING:
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE);
break;
case UNINITIALIZED:
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE);
break;
Expand Down
50 changes: 17 additions & 33 deletions ...em/src/main/java/org/openhab/binding/smsmodem/internal/handler/SMSModemBridgeHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
import java.nio.file.Paths;
import java.util.Collection;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand All @@ -42,7 +44,6 @@
import org.openhab.core.thing.ThingStatusDetail;
import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.binding.BaseBridgeHandler;
import org.openhab.core.thing.binding.ThingHandler;
import org.openhab.core.thing.binding.ThingHandlerService;
import org.openhab.core.types.Command;
import org.slf4j.Logger;
Expand Down Expand Up @@ -77,8 +78,6 @@ public class SMSModemBridgeHandler extends BaseBridgeHandler

private final Logger logger = LoggerFactory.getLogger(SMSModemBridgeHandler.class);

private Set<SMSConversationHandler> childHandlers = new HashSet<>();

private SerialPortManager serialPortManager;

/**
Expand Down Expand Up @@ -167,7 +166,6 @@ private void checkAndStartModemIfNeeded() {
}
} catch (ModemConfigurationException e) {
String message = e.getMessage();
logger.error(message, e);
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, message);
}
}
Expand All @@ -190,7 +188,6 @@ private void checkRemoteParam(SMSModemRemoteBridgeConfiguration config) throws M
// test reachable address :
try (Socket s = new Socket(ip, config.networkPort)) {
}

} catch (IOException | NumberFormatException ex) {
// no ip
throw new ModemConfigurationException(
Expand All @@ -216,20 +213,6 @@ public boolean isRunning() {
return modem != null && (modem.getStatus() == Status.Started || modem.getStatus() == Status.Starting);
}

@Override
public void childHandlerInitialized(ThingHandler childHandler, Thing childThing) {
if (childHandler instanceof SMSConversationHandler) {
childHandlers.add((SMSConversationHandler) childHandler);
} else {
logger.error("The SMSModemBridgeHandler can only handle SMSConversation as child");
}
}

@Override
public void childHandlerDisposed(ThingHandler childHandler, Thing childThing) {
childHandlers.remove(childHandler);
}

@Override
public void handleCommand(ChannelUID channelUID, Command command) {
}
Expand All @@ -244,7 +227,7 @@ public void messageReceived(InboundMessage message) {
if (text != null) {
messageText = text;
} else {
logger.error("Message has no payload !");
logger.warn("Message has no payload !");
return;
}
} else {
Expand All @@ -253,14 +236,14 @@ public void messageReceived(InboundMessage message) {
logger.warn("Message payload in binary format. Don't know how to handle it. Please report it.");
messageText = bytes.toString();
} else {
logger.error("Message has no payload !");
logger.warn("Message has no payload !");
return;
}
}
logger.debug("Receiving new message from {} : {}", sender, messageText);

// dispatch to conversation :
for (SMSConversationHandler child : childHandlers) {
for (SMSConversationHandler child : getChildHandlers()) {
child.checkAndReceive(sender, messageText);
}

Expand All @@ -277,7 +260,7 @@ public void messageReceived(InboundMessage message) {
try { // delete message on the sim
modem.delete(message);
} catch (CommunicationException e) {
logger.error("Cannot delete message after receiving it !", e);
logger.warn("Cannot delete message after receiving it !", e);
}
}

Expand All @@ -296,7 +279,7 @@ public void send(String recipient, String text, boolean deliveryReport, @Nullabl
out.setEncoding(encoding2);
}
} catch (IllegalArgumentException e) {
logger.error("Encoding {} is not supported. Use Enc7, Enc8, EncUcs2, or EncCustom", encoding);
logger.warn("Encoding {} is not supported. Use Enc7, Enc8, EncUcs2, or EncCustom", encoding);
}
out.setRequestDeliveryReport(deliveryReport);
logger.debug("Sending message to {}", recipient);
Expand All @@ -321,25 +304,21 @@ public Collection<Class<? extends ThingHandlerService>> getServices() {
public boolean processStatusCallback(Modem.Status oldStatus, Modem.Status newStatus) {
switch (newStatus) {
case Error:
logger.error("SMSLib reported an error on the underlying modem {}", modem.getDescription());
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR);
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
"SMSLib reported an error on the underlying modem " + modem.getDescription());
break;
case Started:
logger.debug("SMSLib reported the modem {} is started", modem.getDescription());
updateStatus(ThingStatus.ONLINE);
break;
case Starting:
logger.debug("SMSLib reported the modem {} is starting", modem.getDescription());
updateStatus(ThingStatus.UNKNOWN);
break;
case Stopped:
logger.debug("SMSLib reported the modem {} is stopped", modem.getDescription());
if (thing.getStatus() != ThingStatus.OFFLINE) {
updateStatus(ThingStatus.OFFLINE);
}
break;
case Stopping:
logger.debug("SMSLib reported the modem {} is stopping", modem.getDescription());
if (thing.getStatus() != ThingStatus.OFFLINE) {
updateStatus(ThingStatus.OFFLINE);
}
Expand Down Expand Up @@ -374,7 +353,7 @@ public void messageSent(OutboundMessage message) {
MsIsdn recipientAddress = message.getRecipientAddress();
if (recipientAddress != null) {
String recipient = recipientAddress.getAddress();
for (SMSConversationHandler child : childHandlers) {
for (SMSConversationHandler child : getChildHandlers()) {
child.checkAndUpdateDeliveryStatus(recipient, sentStatus);
}
}
Expand Down Expand Up @@ -405,17 +384,22 @@ public void messageDelivered(DeliveryReportMessage message) {
MsIsdn recipientAddress = message.getRecipientAddress();
if (recipientAddress != null) {
String recipient = recipientAddress.getAddress();
for (SMSConversationHandler child : childHandlers) {
for (SMSConversationHandler child : getChildHandlers()) {
child.checkAndUpdateDeliveryStatus(recipient, sentStatus);
}
}
try {
modem.delete(message);
} catch (CommunicationException e) {
logger.error("Cannot delete delivery report after receiving it !", e);
logger.warn("Cannot delete delivery report after receiving it !", e);
}
}

private Set<SMSConversationHandler> getChildHandlers() {
return getThing().getThings().stream().map(Thing::getHandler).filter(Objects::nonNull)
.map(handler -> (SMSConversationHandler) handler).collect(Collectors.toSet());
}

@Override
public void setManufacturer(String manufacturer) {
thing.setProperty(SMSModemBindingConstants.PROPERTY_MANUFACTURER, manufacturer);
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.openhab.binding.smsmodem/src/main/resources/OH-INF/binding/binding.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
xsi:schemaLocation="https://openhab.org/schemas/binding/v1.0.0 https://openhab.org/schemas/binding-1.0.0.xsd">

<name>SMSModem Binding</name>
<description>This binding handle a GSM modem connected to the openHAB server (Serial), or exposed on the network. It
<description>This binding handles a GSM modem connected to the openHAB server (Serial), or exposed on the network. It
can send and receive SMS.</description>

</binding:binding>
3 changes: 1 addition & 2 deletions bundles/org.openhab.binding.smsmodem/src/main/resources/OH-INF/thing/smsmodem.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@
<kind>trigger</kind>
<label>Message Received</label>
<description>Triggered when a message is received, in the form "&lt;msisdn_sender&gt;|&lt;text&gt;"</description>
<event>
</event>
<event/>
</channel-type>

</thing:thing-descriptions>

0 comments on commit 86d3991

Please sign in to comment.