diff --git a/packages/core/src/error/ZWaveError.ts b/packages/core/src/error/ZWaveError.ts index 81261ad61d91..11bd3c87dec7 100644 --- a/packages/core/src/error/ZWaveError.ts +++ b/packages/core/src/error/ZWaveError.ts @@ -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, @@ -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, @@ -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 & { diff --git a/packages/zwave-js/src/lib/controller/Controller.ts b/packages/zwave-js/src/lib/controller/Controller.ts index 274f3c17ba60..342fa80652b2 100644 --- a/packages/zwave-js/src/lib/controller/Controller.ts +++ b/packages/zwave-js/src/lib/controller/Controller.ts @@ -130,10 +130,7 @@ import { ApplicationUpdateRequestSmartStartHomeIDReceived, ApplicationUpdateRequestSmartStartLongRangeHomeIDReceived, } from "../serialapi/application/ApplicationUpdateRequest"; -import { - type SerialAPIStartedRequest, - SerialAPIWakeUpReason, -} from "../serialapi/application/SerialAPIStartedRequest"; + import { ShutdownRequest, type ShutdownResponse, @@ -453,10 +450,6 @@ export class ZWaveController FunctionType.ReplaceFailedNode, this.handleReplaceNodeStatusReport.bind(this), ); - driver.registerRequestHandler( - FunctionType.SerialAPIStarted, - this.handleSerialAPIStartedUnexpectedly.bind(this), - ); } private _type: MaybeNotKnown; @@ -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 { @@ -4392,48 +4389,6 @@ supported CCs: ${ return false; } - /** - * Is called when the Serial API restart unexpectedly. - */ - private async handleSerialAPIStartedUnexpectedly( - msg: SerialAPIStartedRequest, - ): Promise { - // 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(); /** * If routes are currently being rebuilt for the entire network, this returns the current progress. diff --git a/packages/zwave-js/src/lib/driver/Driver.ts b/packages/zwave-js/src/lib/driver/Driver.ts index 09ac9ab441a3..8bb35ed26f47 100644 --- a/packages/zwave-js/src/lib/driver/Driver.ts +++ b/packages/zwave-js/src/lib/driver/Driver.ts @@ -100,6 +100,7 @@ import { serializeCacheValue, stripUndefined, timespan, + wasControllerReset, } from "@zwave-js/core"; import type { NodeSchedulePollOptions, @@ -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 { @@ -4202,6 +4206,69 @@ export class Driver extends TypedEventEmitter } } + /** + * Is called when the Serial API restart unexpectedly. + */ + private async handleSerialAPIStartedUnexpectedly( + msg: SerialAPIStartedRequest, + ): Promise { + // 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. @@ -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( @@ -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 ) { @@ -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 + | undefined; + /** Handles sequencing of queued Serial API commands */ private async drainSerialAPIQueue(): Promise { for await (const item of this.serialAPIQueue) { @@ -5188,6 +5269,8 @@ ${handlers.length} left`, result.resolve(ret); } catch (e) { result.reject(e); + } finally { + this._currentSerialAPICommandPromise = undefined; } } } @@ -5362,6 +5445,7 @@ ${handlers.length} left`, this.serialAPIInterpreter.start(); + this._currentSerialAPICommandPromise = result; return result; } @@ -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); diff --git a/packages/zwave-js/src/lib/test/driver/sendDataMissingCallbackAbort.test.ts b/packages/zwave-js/src/lib/test/driver/sendDataMissingCallbackAbort.test.ts index a4b42cab2cae..66fe537c9511 100644 --- a/packages/zwave-js/src/lib/test/driver/sendDataMissingCallbackAbort.test.ts +++ b/packages/zwave-js/src/lib/test/driver/sendDataMissingCallbackAbort.test.ts @@ -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, @@ -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, + ) + ); + }, + }, +);