Skip to content

Commit 222ab56

Browse files
committed
various
- ignore null DTO fields to prevent NPE risk - fix inherited exception stack trace logging Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
1 parent 7435425 commit 222ab56

3 files changed

Lines changed: 38 additions & 9 deletions

File tree

bundles/org.openhab.binding.homekit/src/main/java/org/openhab/binding/homekit/internal/dto/Characteristic.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -982,4 +982,11 @@ public String toString() {
982982
public @Nullable StatusCode getStatusCode() {
983983
return status instanceof Integer code ? StatusCode.from(code) : null;
984984
}
985+
986+
/**
987+
* Return true if the characteristic is valid i.e. itself and its aid and iid fields are not null
988+
*/
989+
public static boolean isValid(@Nullable Characteristic characteristic) {
990+
return characteristic != null && characteristic.aid != null && characteristic.iid != null;
991+
}
985992
}

bundles/org.openhab.binding.homekit/src/main/java/org/openhab/binding/homekit/internal/handler/HomekitAccessoryHandler.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -790,27 +790,40 @@ private void eventingPollingFinalize(Accessory accessory, Map<String, Channel> c
790790
if (aid == null) {
791791
return;
792792
}
793+
List<Service> services = accessory.services;
794+
if (services == null) {
795+
return;
796+
}
793797

794798
for (Channel channel : channels.values()) {
795799
final ChannelUID channelUID = channel.getUID();
796800
if (CHANNEL_SNAPSHOT.equals(channelUID.getId())) {
797801
continue; // skip camera snapshot channel
798802
}
799803
if (isLinked(channelUID)) {
800-
Long iid = 0L;
804+
Long iid;
801805
boolean checkChannelLinkByIID = !channelUID.equals(lightModelClientHSBTypeChannel);
802806
if (checkChannelLinkByIID && channel.getProperties().get(PROPERTY_IID) instanceof String iidProperty) {
803807
try {
804808
iid = Long.parseLong(iidProperty);
805809
} catch (NumberFormatException e) {
806810
continue; // error will already have been logged elsewhere
807811
}
812+
} else {
813+
iid = null;
814+
}
815+
if (checkChannelLinkByIID && iid == null) {
816+
continue;
808817
}
809818

810819
nestedLoops: // break marker for nested loops below
811-
for (Service service : accessory.services) {
820+
for (Service service : services) {
821+
if (service == null || service.characteristics == null) {
822+
continue;
823+
}
812824
for (Characteristic characteristic : service.characteristics) {
813-
if ((checkChannelLinkByIID && iid.equals(characteristic.iid))
825+
if (Characteristic.isValid(characteristic)
826+
&& (checkChannelLinkByIID && characteristic.iid.equals(iid))
814827
|| LIGHT_MODEL_RELEVANT_TYPES.contains(characteristic.getCharacteristicType())) {
815828
Characteristic entry = new Characteristic();
816829
entry.aid = aid;

bundles/org.openhab.binding.homekit/src/main/java/org/openhab/binding/homekit/internal/handler/HomekitBaseAccessoryHandler.java

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import static org.openhab.binding.homekit.internal.HomekitBindingConstants.*;
1616

1717
import java.io.IOException;
18+
import java.io.PrintWriter;
19+
import java.io.StringWriter;
1820
import java.math.BigDecimal;
1921
import java.nio.charset.StandardCharsets;
2022
import java.security.NoSuchAlgorithmException;
@@ -682,10 +684,11 @@ private void enableEventsOrThrow(boolean enable) throws Exception {
682684
}
683685
Service service = new Service();
684686
service.characteristics = new ArrayList<>();
685-
service.characteristics.addAll(getEventedCharacteristics().values().stream().map(cxx -> {
686-
cxx.ev = enable;
687-
return cxx;
688-
}).toList());
687+
service.characteristics
688+
.addAll(getEventedCharacteristics().values().stream().filter(Characteristic::isValid).map(cxx -> {
689+
cxx.ev = enable;
690+
return cxx;
691+
}).toList());
689692
if (service.characteristics.isEmpty()) {
690693
return;
691694
}
@@ -703,7 +706,7 @@ private void enableEventsOrThrow(boolean enable) throws Exception {
703706
* Called periodically by the refresh task and on-demand when RefreshType.REFRESH is called.
704707
*/
705708
private synchronized void refresh() {
706-
List<String> queries = getPolledCharacteristics().values().stream().filter(c -> c.iid != null && c.aid != null)
709+
List<String> queries = getPolledCharacteristics().values().stream().filter(Characteristic::isValid)
707710
.map(c -> "%s.%s".formatted(c.aid, c.iid)).toList();
708711
if (queries.isEmpty()) {
709712
return;
@@ -966,7 +969,13 @@ protected void scheduleSnapshotRefresh() {
966969
protected void onCommunicationError(Exception exception, String i18nSuffix) {
967970
String message = exception.getMessage();
968971
logger.debug("{} {} {}, reconnecting..", thing.getUID(), i18nSuffix, message);
969-
logger.trace("{} stack trace", thing.getUID(), exception);
972+
if (logger.isTraceEnabled()) {
973+
try (StringWriter sw = new StringWriter(); PrintWriter pw = new PrintWriter(sw)) {
974+
exception.printStackTrace(pw);
975+
logger.trace("{}", sw.toString().trim());
976+
} catch (IOException e) {
977+
}
978+
}
970979
String description = THING_STATUS_FMT.formatted("error." + i18nSuffix.replace(' ', '-'), message);
971980
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, description);
972981
scheduleConnectionAttempt();

0 commit comments

Comments
 (0)