Skip to content

Commit a389495

Browse files
apella12apella12
committed
Improve firmware update handling and logging
Clarify and harden firmware update flow: enhance debug logging for ACK advancement and duplicate UPDATE_MD_GET handling to better describe sequential cases, and increase NOP/version refresh scheduling delays (delayedExecutor from 2s to 10s and added extra waitTimeSeconds offset) to reduce race conditions. Update unit test timeout to match the longer delay. Refactor ZWaveThingHandler: reorder firmware-related fields, replace direct callback identity check with Objects.equals, and add advanceFirmwareProgressTo helper to advance progress steps safely. Minor cleanup: move FirmwareDownloadStatus enum within ZWaveFirmwareUpdateCommandClass, tweak author tags in ZWaveNode, and trim incidental whitespace. Signed-off-by: Bob Eckhoff <katmandodo@yahoo.com>
1 parent b132da1 commit a389495

6 files changed

Lines changed: 85 additions & 76 deletions

File tree

‎src/main/java/org/openhab/binding/zwave/actions/ZWaveThingActions.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,5 +163,4 @@ public void setThingHandler(ThingHandler thingHandler) {
163163
}
164164
return "Handler is null, cannot poll linked channels";
165165
}
166-
167166
}

‎src/main/java/org/openhab/binding/zwave/firmwareupdate/ZWaveFirmwareUpdateSession.java‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public void abort(String reason) {
130130
failFirmwareUpdate("Firmware update session aborted: " + reason);
131131
}
132132

133-
// Sends the initial FIRMWARE_MD_GET to start the process
133+
// Sends or queues the initial FIRMWARE_MD_GET to start the process
134134
private void requestMetadata() {
135135
ZWaveFirmwareUpdateCommandClass fw = (ZWaveFirmwareUpdateCommandClass) node
136136
.getCommandClass(CommandClass.COMMAND_CLASS_FIRMWARE_UPDATE_MD);
@@ -728,8 +728,16 @@ private boolean handleUpdateMdGet(FirmwareUpdateEvent event) {
728728
// confirmed receipt of those fragments by previously requesting something higher.
729729
int impliedAckReport = Math.min(highestTransmittedReportNumber, requestedStartReport - 1);
730730
if (impliedAckReport > highestAckedReportNumber) {
731-
logger.debug("NODE {}: Advancing ACK anchor from {} to {} based on UPDATE_MD_GET start {}",
732-
node.getNodeId(), highestAckedReportNumber, impliedAckReport, requestedStartReport);
731+
if (requestedStartReport == highestTransmittedReportNumber + 1
732+
&& highestTransmittedReportNumber == startReportNumber) {
733+
logger.debug(
734+
"NODE {}: Advancing ACK anchor from {} to {} based on sequential UPDATE_MD_GET start {} after completed fragment {} transmission",
735+
node.getNodeId(), highestAckedReportNumber, impliedAckReport, requestedStartReport,
736+
highestTransmittedReportNumber);
737+
} else {
738+
logger.debug("NODE {}: Advancing ACK anchor from {} to {} based on UPDATE_MD_GET start {}",
739+
node.getNodeId(), highestAckedReportNumber, impliedAckReport, requestedStartReport);
740+
}
733741
highestAckedReportNumber = impliedAckReport;
734742
}
735743

@@ -745,12 +753,18 @@ private boolean handleUpdateMdGet(FirmwareUpdateEvent event) {
745753
// Ignore these near-term, but allow a late
746754
// retry so the device can recover if the fragment was truly missed.
747755
if (requestedStartReport > startReportNumber && startReportNumber > 0) {
748-
logger.debug(
749-
"NODE {}: Received UPDATE_MD_GET for fragment {} while trying fragment {}; treating this as implicit ACK of fragment {} and continuing with the higherfragment",
750-
node.getNodeId(), requestedStartReport, startReportNumber, startReportNumber);
756+
if (requestedStartReport == highestTransmittedReportNumber + 1
757+
&& highestTransmittedReportNumber == startReportNumber) {
758+
logger.debug(
759+
"NODE {}: Received sequential UPDATE_MD_GET for fragment {} after fragment {} transmission completion; continuing with requested fragment",
760+
node.getNodeId(), requestedStartReport, startReportNumber);
761+
} else {
762+
logger.debug(
763+
"NODE {}: Received UPDATE_MD_GET for fragment {} while trying fragment {}; treating this as implicit ACK of fragment {} and continuing with the higher fragment",
764+
node.getNodeId(), requestedStartReport, startReportNumber, startReportNumber);
765+
}
751766
duplicateGetsForSentReport = 0;
752767
} else if (requestedStartReport <= highestTransmittedReportNumber) {
753-
754768
Long lastSentTime = reportLastSentTimes.get(requestedStartReport);
755769
long elapsedMillis = lastSentTime != null ? currentTimeMillis() - lastSentTime.longValue() : Long.MAX_VALUE;
756770

@@ -1059,7 +1073,7 @@ private void scheduleNopAfterWaitTime(int waitTimeSeconds) {
10591073
waitTimeSeconds = 5;
10601074
}
10611075

1062-
final int delay = waitTimeSeconds;
1076+
final int delay = waitTimeSeconds + 5;
10631077
logger.debug("NODE {}: Scheduling NOP ping after {} seconds", node.getNodeId(), delay);
10641078

10651079
CompletableFuture.runAsync(() -> {
@@ -1081,7 +1095,7 @@ private void scheduleVersionRefresh() {
10811095
} else {
10821096
logger.warn("NODE {}: Version command class not available for version refresh", node.getNodeId());
10831097
}
1084-
}, CompletableFuture.delayedExecutor(2, TimeUnit.SECONDS));
1098+
}, CompletableFuture.delayedExecutor(10, TimeUnit.SECONDS));
10851099
}
10861100

10871101
private void completeSuccess() {

‎src/main/java/org/openhab/binding/zwave/handler/ZWaveThingHandler.java‎

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -120,17 +120,6 @@ public class ZWaveThingHandler extends ConfigStatusThingHandler implements ZWave
120120

121121
private ZWaveControllerHandler controllerHandler;
122122

123-
private byte[] pendingFirmwareBytes;
124-
private Integer pendingFirmwareTarget = 0;
125-
private @Nullable ZWaveFirmwareUpdateSession firmwareSession;
126-
private @Nullable ProgressCallback firmwareProgressCallback;
127-
private int firmwareProgressStepIndex = -1;
128-
private @Nullable Integer lastFirmwareUpdateProgressPercent;
129-
private @Nullable String lastFirmwareFailureDescription;
130-
private boolean finalTypeSet = false;
131-
132-
private static final List<Integer> FIRMWARE_PROGRESS_UI_MILESTONES = List.of(5, 25, 50, 75);
133-
134123
private int nodeId;
135124
private List<ZWaveThingChannel> thingChannelsCmd = Collections.emptyList();
136125
private List<ZWaveThingChannel> thingChannelsState = Collections.emptyList();
@@ -139,15 +128,22 @@ public class ZWaveThingHandler extends ConfigStatusThingHandler implements ZWave
139128

140129
private final Map<Integer, ZWaveConfigSubParameter> subParameters = new HashMap<Integer, ZWaveConfigSubParameter>();
141130
private final Map<String, Object> pendingCfg = new HashMap<String, Object>();
131+
private boolean finalTypeSet = false;
142132

133+
private byte[] pendingFirmwareBytes;
134+
private Integer pendingFirmwareTarget = 0;
135+
private @Nullable ZWaveFirmwareUpdateSession firmwareSession;
136+
private @Nullable ProgressCallback firmwareProgressCallback;
137+
private int firmwareProgressStepIndex = -1;
138+
private @Nullable Integer lastFirmwareUpdateProgressPercent;
139+
private @Nullable String lastFirmwareFailureDescription;
140+
private static final List<Integer> FIRMWARE_PROGRESS_UI_MILESTONES = List.of(5, 25, 50, 75);
143141
private static final Set<String> SUPPORTED_FIRMWARE_EXTENSIONS = Set.of(".bin", ".hex", ".ota", ".otz", ".gbl",
144142
".zip", ".exe", ".ex_");
145-
146143
private static boolean isSupportedFirmwareFile(Path file) {
147144
String name = file.getFileName().toString().toLowerCase(Locale.ROOT);
148145
return SUPPORTED_FIRMWARE_EXTENSIONS.stream().anyMatch(name::endsWith);
149146
}
150-
151147
private Path getNodeFirmwareFolder() {
152148
return Paths.get(OpenHAB.getUserDataFolder(), "zwave", "firmware", "node-" + nodeId);
153149
}
@@ -2228,6 +2224,24 @@ public void updateFirmware(Firmware firmware, ProgressCallback progressCallback)
22282224
}
22292225
}
22302226

2227+
// Advances the firmware progress sequence to the given step index.
2228+
private @Nullable ProgressCallback advanceFirmwareProgressTo(int targetStepIndex,
2229+
@Nullable ProgressCallback callback) {
2230+
if (callback == null) {
2231+
return null;
2232+
}
2233+
2234+
while (firmwareProgressStepIndex < targetStepIndex) {
2235+
ProgressCallback usableCallback = keepCallbackIfUsable(callback, "next()", callback::next);
2236+
if (usableCallback == null) {
2237+
return null;
2238+
}
2239+
firmwareProgressStepIndex++;
2240+
}
2241+
2242+
return callback;
2243+
}
2244+
22312245
/**
22322246
* Loads firmware bytes from userdata/zwave/firmware/node-<nodeId>.
22332247
* Policy: exactly one supported file must exist.
@@ -2420,7 +2434,7 @@ private void resetFirmwareProgressSequence() {
24202434
logger.warn("NODE {}: Firmware progress callback {} failed ({}); disabling callback for this run", nodeId,
24212435
operation, e.getMessage());
24222436
logger.debug("NODE {}: Firmware progress callback {} failure detail", nodeId, operation, e);
2423-
if (this.firmwareProgressCallback == callback) {
2437+
if (Objects.equals(this.firmwareProgressCallback, callback)) {
24242438
this.firmwareProgressCallback = null;
24252439
}
24262440
resetFirmwareProgressSequence();
@@ -2462,27 +2476,9 @@ private void updateFirmwareProgressStatusForUiMilestone(int progressPercent) {
24622476
"Firmware update in progress (" + milestone + "%)");
24632477
}
24642478

2465-
// Advances the firmware progress sequence to the given step index.
2466-
private @Nullable ProgressCallback advanceFirmwareProgressTo(int targetStepIndex,
2467-
@Nullable ProgressCallback callback) {
2468-
if (callback == null) {
2469-
return null;
2470-
}
2471-
2472-
while (firmwareProgressStepIndex < targetStepIndex) {
2473-
ProgressCallback usableCallback = keepCallbackIfUsable(callback, "next()", callback::next);
2474-
if (usableCallback == null) {
2475-
return null;
2476-
}
2477-
firmwareProgressStepIndex++;
2478-
}
2479-
2480-
return callback;
2481-
}
2482-
24832479
/**
2484-
* This overcomes a communication failure where the node is marked DEAD, but
2485-
* comes back online before the firmware update completes.
2480+
* Restores the last known firmware update progress status in the UI
2481+
* if a firmware update is in progress and the node comes back online.
24862482
*/
24872483
private void restoreFirmwareUpdateProgressStatusIfNeeded() {
24882484
ZWaveFirmwareUpdateSession session = firmwareSession;

‎src/main/java/org/openhab/binding/zwave/internal/protocol/ZWaveNode.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@
5454
/**
5555
* Z-Wave node class. Represents a node in the Z-Wave network.
5656
*
57-
* @author Chris Jackson
58-
* @author Brian Crosby
57+
* @author Chris Jackson - Initial contribution
58+
* @author Brian Crosby - Contribution
5959
*/
6060
@XStreamAlias("node")
6161
public class ZWaveNode {

‎src/main/java/org/openhab/binding/zwave/internal/protocol/commandclass/ZWaveFirmwareUpdateCommandClass.java‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -382,35 +382,6 @@ public void handleFirmwarePrepareReport(ZWaveCommandClassPayload payload, int en
382382
// FirmwareUpdateEvent.forUpdatePrepareReport(getNode().getNodeId(), endpoint, status.getId(), checksum));
383383
}
384384

385-
public enum FirmwareDownloadStatus {
386-
INVALID_PAYLOAD(0x00),
387-
EXPECTED_AUTHORIZATION_EVENT(0x01),
388-
FRAGMENT_SIZE_EXCEEDED(0x02),
389-
FIRMWARE_TARGET_NOT_DOWNLOADABLE(0x03),
390-
INVALID_HARDWARE_VERSION(0x04),
391-
SUCCESS(0xFF),
392-
UNKNOWN(-1);
393-
394-
private final int id;
395-
396-
FirmwareDownloadStatus(int id) {
397-
this.id = id;
398-
}
399-
400-
public int getId() {
401-
return id;
402-
}
403-
404-
public static FirmwareDownloadStatus from(int v) {
405-
for (FirmwareDownloadStatus status : values()) {
406-
if (status.id == v) {
407-
return status;
408-
}
409-
}
410-
return UNKNOWN;
411-
}
412-
}
413-
414385
public enum FirmwareUpdateMdRequestStatus {
415386
ERROR_INVALID_MANUFACTURER_OR_FIRMWARE_ID(0x00),
416387
ERROR_AUTHENTICATION_EXPECTED(0x01),
@@ -502,6 +473,35 @@ public static FirmwareUpdateActivationStatus from(int v) {
502473
return UNKNOWN;
503474
}
504475
}
476+
477+
public enum FirmwareDownloadStatus {
478+
INVALID_PAYLOAD(0x00),
479+
EXPECTED_AUTHORIZATION_EVENT(0x01),
480+
FRAGMENT_SIZE_EXCEEDED(0x02),
481+
FIRMWARE_TARGET_NOT_DOWNLOADABLE(0x03),
482+
INVALID_HARDWARE_VERSION(0x04),
483+
SUCCESS(0xFF),
484+
UNKNOWN(-1);
485+
486+
private final int id;
487+
488+
FirmwareDownloadStatus(int id) {
489+
this.id = id;
490+
}
491+
492+
public int getId() {
493+
return id;
494+
}
495+
496+
public static FirmwareDownloadStatus from(int v) {
497+
for (FirmwareDownloadStatus status : values()) {
498+
if (status.id == v) {
499+
return status;
500+
}
501+
}
502+
return UNKNOWN;
503+
}
504+
}
505505

506506
/* CRC16-CCITT (poly 0x1021) implementation */
507507
public static int crc16Ccitt(byte[] data, int initial) {

‎src/test/java/org/openhab/binding/zwave/firmwareupdate/ZWaveFirmwareUpdateSessionTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ public void testUpdateMdStatusReportRestartPendingSchedulesNopPing() throws Exce
451451
assertTrue(handled);
452452
assertFalse(session.isActive());
453453
assertEquals(ZWaveFirmwareUpdateSession.State.SUCCESS, getState(session));
454-
Mockito.verify(node, Mockito.timeout((int) TimeUnit.SECONDS.toMillis(7))).pingNode();
454+
Mockito.verify(node, Mockito.timeout((int) TimeUnit.SECONDS.toMillis(12))).pingNode();
455455
}
456456

457457
@Test

0 commit comments

Comments
 (0)