Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid blocking the send queue when recovering controller from missed Send Data callback fails #6473

Merged
merged 1 commit into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 38 additions & 22 deletions packages/zwave-js/src/lib/driver/Driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3647,34 +3647,50 @@ export class Driver extends TypedEventEmitter<DriverEventCallbacks>
}
} else if (this._controller.status !== ControllerStatus.Unresponsive) {
// The controller was responsive before this transaction failed.
// Mark it as unresponsive and try to soft-reset it.
this.controller.setStatus(
ControllerStatus.Unresponsive,
);

this._recoveryPhase = ControllerRecoveryPhase.CallbackTimeout;
if (this.maySoftReset()) {
// Mark it as unresponsive and try to soft-reset it.
this.controller.setStatus(
ControllerStatus.Unresponsive,
);

this.driverLog.print(
"Controller missed Send Data callback. Attempting to recover...",
"warn",
);
this._recoveryPhase = ControllerRecoveryPhase.CallbackTimeout;

// Re-queue the transaction.
// Its message generator may have finished, so reset that too.
transaction.reset();
this.queue.add(transaction.clone());
this.driverLog.print(
"Controller missed Send Data callback. Attempting to recover...",
"warn",
);

// Execute the soft-reset asynchronously
void this.softReset().then(() => {
// The controller responded. It is no longer unresponsive
this._controller?.setStatus(ControllerStatus.Ready);
// Re-queue the transaction.
// Its message generator may have finished, so reset that too.
transaction.reset();
this.queue.add(transaction.clone());

this._recoveryPhase =
ControllerRecoveryPhase.CallbackTimeoutAfterReset;
}).catch(() => {
// Soft-reset failed. Just reject the original transaction.
// Execute the soft-reset asynchronously
void this.softReset().then(() => {
// The controller responded. It is no longer unresponsive
this._controller?.setStatus(ControllerStatus.Ready);

this._recoveryPhase =
ControllerRecoveryPhase.CallbackTimeoutAfterReset;
}).catch(() => {
// Soft-reset failed. Just reject the original transaction.
this.rejectTransaction(transaction, error);

this.driverLog.print(
"Automatic controller recovery failed. Returning to normal operation and hoping for the best.",
"warn",
);
this._recoveryPhase = ControllerRecoveryPhase.None;
this._controller?.setStatus(ControllerStatus.Ready);
});
} else {
this.driverLog.print(
"Controller missed Send Data callback. Cannot recover automatically because the soft reset feature is unsupported or disabled.",
"warn",
);
this.rejectTransaction(transaction, error);
});
}

return true;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,3 +367,112 @@ integrationTest(
},
},
);

integrationTest(
"With soft-reset disabled, transmissions do not get stuck after a missing Send Data callback",
{
// debug: true,

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

controllerCapabilities: {
// Soft-reset cannot be disabled on 700+ series
libraryVersion: "Z-Wave 6.84.0",
},

additionalDriverOptions: {
enableSoftReset: false,
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,
);

// We only timeout once in this test
shouldTimeOut = false;

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 = 1000;
driver.options.timeouts.sendDataCallback = 1500;

shouldTimeOut = true;

const firstCommand = node.commandClasses.Basic.set(99).catch((e) =>
e.code
);
const followupCommand = node.commandClasses.Basic.set(0);

await wait(2500);

// Transmission should have been aborted
mockController.assertReceivedHostMessage(
(msg) => msg.functionType === FunctionType.SendDataAbort,
);
// but the stick should NOT have been soft-reset
t.throws(() =>
mockController.assertReceivedHostMessage(
(msg) => msg.functionType === FunctionType.SoftReset,
)
);
mockController.clearReceivedHostMessages();

// The first command should be failed
t.is(await firstCommand, ZWaveErrorCodes.Controller_Timeout);

// The followup command should eventually succeed
await followupCommand;

t.pass();
},
},
);