Skip to content

Commit

Permalink
fix: handle unexpected controller reset during command execution
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlCalzone
AlCalzone committed Jun 23, 2024
1 parent 454d9f1 commit e23715a
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 52 deletions.
14 changes: 13 additions & 1 deletion packages/core/src/error/ZWaveError.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export enum ZWaveErrorCodes {

/** The driver failed to start */
Driver_Failed = 100,
Driver_Reset,
Driver_Reset, // FIXME: This is not used
Driver_Destroyed,
Driver_NotReady,
Driver_InvalidDataReceived,
Expand All @@ -35,6 +35,9 @@ export enum ZWaveErrorCodes {
Controller_ResponseNOK,
Controller_CallbackNOK,
Controller_Jammed,
/** The controller was reset in the middle of a Serial API command */
Controller_Reset,

Controller_InclusionFailed,
Controller_ExclusionFailed,

Expand Down Expand Up @@ -294,6 +297,15 @@ export function isMissingControllerACK(
&& e.context === "ACK";
}

export function wasControllerReset(
e: unknown,
): e is ZWaveError & {
code: ZWaveErrorCodes.Controller_Reset;
} {
return isZWaveError(e)
&& e.code === ZWaveErrorCodes.Controller_Reset;
}

export function isMissingControllerResponse(
e: unknown,
): e is ZWaveError & {
Expand Down
55 changes: 5 additions & 50 deletions packages/zwave-js/src/lib/controller/Controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,7 @@ import {
ApplicationUpdateRequestSmartStartHomeIDReceived,
ApplicationUpdateRequestSmartStartLongRangeHomeIDReceived,
} from "../serialapi/application/ApplicationUpdateRequest";
import {
type SerialAPIStartedRequest,
SerialAPIWakeUpReason,
} from "../serialapi/application/SerialAPIStartedRequest";

import {
ShutdownRequest,
type ShutdownResponse,
Expand Down Expand Up @@ -453,10 +450,6 @@ export class ZWaveController
FunctionType.ReplaceFailedNode,
this.handleReplaceNodeStatusReport.bind(this),
);
driver.registerRequestHandler(
FunctionType.SerialAPIStarted,
this.handleSerialAPIStartedUnexpectedly.bind(this),
);
}

private _type: MaybeNotKnown<ZWaveLibraryTypes>;
Expand Down Expand Up @@ -743,6 +736,10 @@ export class ZWaveController
public get nodeIdType(): NodeIDType {
return this._nodeIdType;
}
/** @internal */
public set nodeIdType(value: NodeIDType) {
this._nodeIdType = value;
}

/** Returns the node with the given DSK */
public getNodeByDSK(dsk: Buffer | string): ZWaveNode | undefined {
Expand Down Expand Up @@ -4392,48 +4389,6 @@ supported CCs: ${
return false;
}

/**
* Is called when the Serial API restart unexpectedly.
*/
private async handleSerialAPIStartedUnexpectedly(
msg: SerialAPIStartedRequest,
): Promise<boolean> {
// Normally, the soft reset command includes waiting for this message.
// If we end up here, it is unexpected.

switch (msg.wakeUpReason) {
// All wakeup reasons that indicate a reset of the Serial API
// need to be handled here, so we interpret node IDs correctly.
case SerialAPIWakeUpReason.Reset:
case SerialAPIWakeUpReason.WatchdogReset:
case SerialAPIWakeUpReason.SoftwareReset:
case SerialAPIWakeUpReason.EmergencyWatchdogReset:
case SerialAPIWakeUpReason.BrownoutCircuit: {
// The Serial API restarted unexpectedly
if (this._nodeIdType === NodeIDType.Long) {
this.driver.controllerLog.print(
`Serial API restarted unexpectedly.`,
"warn",
);

// Restart the watchdog unless disabled
if (this.driver.options.features.watchdog) {
await this.startWatchdog();
}

// We previously used 16 bit node IDs, but the controller was reset.
// Remember this and try to go back to 16 bit.
this._nodeIdType = NodeIDType.Short;
await this.trySetNodeIDType(NodeIDType.Long);
}

return true; // Don't invoke any more handlers
}
}

return false; // Not handled
}

private _rebuildRoutesProgress = new Map<number, RebuildRoutesStatus>();
/**
* If routes are currently being rebuilt for the entire network, this returns the current progress.
Expand Down
95 changes: 94 additions & 1 deletion packages/zwave-js/src/lib/driver/Driver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ import {
serializeCacheValue,
stripUndefined,
timespan,
wasControllerReset,
} from "@zwave-js/core";
import type {
NodeSchedulePollOptions,
Expand Down Expand Up @@ -171,7 +172,10 @@ import {
import { ApplicationCommandRequest } from "../serialapi/application/ApplicationCommandRequest";
import { ApplicationUpdateRequest } from "../serialapi/application/ApplicationUpdateRequest";
import { BridgeApplicationCommandRequest } from "../serialapi/application/BridgeApplicationCommandRequest";
import type { SerialAPIStartedRequest } from "../serialapi/application/SerialAPIStartedRequest";
import {
SerialAPIStartedRequest,
SerialAPIWakeUpReason,
} from "../serialapi/application/SerialAPIStartedRequest";
import { GetControllerVersionRequest } from "../serialapi/capability/GetControllerVersionMessages";
import { SoftResetRequest } from "../serialapi/misc/SoftResetRequest";
import {
Expand Down Expand Up @@ -4202,6 +4206,69 @@ export class Driver extends TypedEventEmitter<DriverEventCallbacks>
}
}

/**
* Is called when the Serial API restart unexpectedly.
*/
private async handleSerialAPIStartedUnexpectedly(
msg: SerialAPIStartedRequest,
): Promise<boolean> {
// Normally, the soft reset command includes waiting for this message.
// If we end up here, it is unexpected.

switch (msg.wakeUpReason) {
// All wakeup reasons that indicate a reset of the Serial API
// need to be handled here, so we interpret node IDs correctly.
case SerialAPIWakeUpReason.Reset:
case SerialAPIWakeUpReason.WatchdogReset:
case SerialAPIWakeUpReason.SoftwareReset:
case SerialAPIWakeUpReason.EmergencyWatchdogReset:
case SerialAPIWakeUpReason.BrownoutCircuit: {
// The Serial API restarted unexpectedly
this.controllerLog.print(
`Serial API restarted unexpectedly.`,
"warn",
);

// In this situation, we may be executing a Serial API command, which will never complete.
// Bail out of it, without rejecting the actual transaction
if (
this.serialAPIInterpreter?.status
=== InterpreterStatus.Running
) {
this.serialAPIInterpreter.stop();
}
if (this._currentSerialAPICommandPromise) {
this.controllerLog.print(
`Currently active command will be retried...`,
"warn",
);
this._currentSerialAPICommandPromise.reject(
new ZWaveError(
"The Serial API restarted unexpectedly",
ZWaveErrorCodes.Controller_Reset,
),
);
}

// Restart the watchdog unless disabled
if (this.options.features.watchdog) {
await this._controller?.startWatchdog();
}

if (this._controller?.nodeIdType === NodeIDType.Long) {
// We previously used 16 bit node IDs, but the controller was reset.
// Remember this and try to go back to 16 bit.
this._controller.nodeIdType = NodeIDType.Short;
await this._controller.trySetNodeIDType(NodeIDType.Long);
}

return true; // Don't invoke any more handlers
}
}

return false; // Not handled
}

/**
* Registers a handler for messages that are not handled by the driver as part of a message exchange.
* The handler function needs to return a boolean indicating if the message has been handled.
Expand Down Expand Up @@ -4653,6 +4720,10 @@ ${handlers.length} left`,
// Make sure we're ready to handle this command
this.ensureReady(true);
return this.controller.handleApplicationUpdateRequest(msg);
} else if (msg instanceof SerialAPIStartedRequest) {
if (await this.handleSerialAPIStartedUnexpectedly(msg)) {
return;
}
} else {
// TODO: This deserves a nicer formatting
this.driverLog.print(
Expand Down Expand Up @@ -5144,6 +5215,9 @@ ${handlers.length} left`,
} else if (isMissingControllerACK(e)) {
// The controller is unresponsive. Reject the transaction, so we can attempt to recover
throw e;
} else if (wasControllerReset(e)) {
// The controller was reset unexpectedly. Reject the transaction, so we can attempt to recover
throw e;
} else if (
e.code === ZWaveErrorCodes.Controller_MessageDropped
) {
Expand Down Expand Up @@ -5176,6 +5250,13 @@ ${handlers.length} left`,
transaction.setProgress({ state: TransactionState.Completed });
}

/**
* Provides access to the result Promise for the currently executed serial API command
*/
private _currentSerialAPICommandPromise:
| DeferredPromise<Message | undefined>
| undefined;

/** Handles sequencing of queued Serial API commands */
private async drainSerialAPIQueue(): Promise<void> {
for await (const item of this.serialAPIQueue) {
Expand All @@ -5188,6 +5269,8 @@ ${handlers.length} left`,
result.resolve(ret);
} catch (e) {
result.reject(e);
} finally {
this._currentSerialAPICommandPromise = undefined;
}
}
}
Expand Down Expand Up @@ -5362,6 +5445,7 @@ ${handlers.length} left`,

this.serialAPIInterpreter.start();

this._currentSerialAPICommandPromise = result;
return result;
}

Expand Down Expand Up @@ -6007,6 +6091,15 @@ ${handlers.length} left`,
if (
this.handleMissingSendDataResponseOrCallback(transaction, error)
) return;
} else if (wasControllerReset(error)) {
// The controller was reset in the middle of a transaction.
// Re-queue the transaction, so it can get handled again
// Its message generator may have finished, so reset that too.
transaction.reset();
this.getQueueForTransaction(transaction).add(
transaction.clone(),
);
return;
}

this.rejectTransaction(transaction, error);
Expand Down
111 changes: 111 additions & 0 deletions packages/zwave-js/src/lib/test/driver/sendDataMissingCallbackAbort.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ import {
} from "@zwave-js/core";
import path from "node:path";
import Sinon from "sinon";
import { determineNIF } from "../../controller/NodeInformationFrame";
import {
SerialAPIStartedRequest,
SerialAPIWakeUpReason,
} from "../../serialapi/application/SerialAPIStartedRequest";
import { SoftResetRequest } from "../../serialapi/misc/SoftResetRequest";
import {
RequestNodeInfoRequest,
Expand Down Expand Up @@ -825,3 +830,109 @@ integrationTestMulti(
},
},
);

integrationTest(
"Retry transmissions if the controller is reset by the watchdog while waiting for the callback",
{
// debug: true,

// provisioningDirectory: path.join(
// __dirname,
// "__fixtures/supervision_binary_switch",
// ),

additionalDriverOptions: {
testingHooks: {
skipNodeInterview: true,
},
},

customSetup: async (driver, mockController, mockNode) => {
// This is almost a 1:1 copy of the default behavior, except that the callback never gets sent
const handleBrokenSendData: MockControllerBehavior = {
async onHostMessage(host, controller, msg) {
// If the controller is operating normally, defer to the default behavior
if (!shouldTimeOut) return false;

if (msg instanceof SendDataRequest) {
// Check if this command is legal right now
const state = controller.state.get(
MockControllerStateKeys.CommunicationState,
) as MockControllerCommunicationState | undefined;
if (
state != undefined
&& state !== MockControllerCommunicationState.Idle
) {
throw new Error(
"Received SendDataRequest while not idle",
);
}

// Put the controller into sending state
controller.state.set(
MockControllerStateKeys.CommunicationState,
MockControllerCommunicationState.Sending,
);

// Notify the host that the message was sent
const res = new SendDataResponse(host, {
wasSent: true,
});
await controller.sendToHost(res.serialize());

return true;
} else if (msg instanceof SendDataAbort) {
// Put the controller into idle state
controller.state.set(
MockControllerStateKeys.CommunicationState,
MockControllerCommunicationState.Idle,
);

return true;
}
},
};
mockController.defineBehavior(handleBrokenSendData);
},
testBody: async (t, driver, node, mockController, mockNode) => {
// Circumvent the options validation so the test doesn't take forever
driver.options.timeouts.sendDataAbort = 1500;
driver.options.timeouts.sendDataCallback = 2000;

shouldTimeOut = true;

const pingPromise = node.ping();

await wait(1000);

// After 1 second, the watchdog restarts the controller
shouldTimeOut = false;

mockController.state.set(
MockControllerStateKeys.CommunicationState,
MockControllerCommunicationState.Idle,
);

const ret = new SerialAPIStartedRequest(mockController.host, {
wakeUpReason: SerialAPIWakeUpReason.WatchdogReset,
watchdogEnabled: true,
isListening: true,
...determineNIF(),
supportsLongRange: true,
});
setImmediate(async () => {
await mockController.sendToHost(ret.serialize());
});

// And the ping should eventually succeed
t.true(await pingPromise);

// But the transmission should not have been aborted
t.throws(() =>
mockController.assertReceivedHostMessage(
(msg) => msg.functionType === FunctionType.SendDataAbort,
)
);
},
},
);

0 comments on commit e23715a

Please sign in to comment.