diff --git a/packages/binding-modbus/src/modbus-client-factory.ts b/packages/binding-modbus/src/modbus-client-factory.ts index 059b9d406..29173f710 100644 --- a/packages/binding-modbus/src/modbus-client-factory.ts +++ b/packages/binding-modbus/src/modbus-client-factory.ts @@ -20,12 +20,13 @@ const warn = createWarnLogger("binding-modbus", "modbus-client-factory"); export default class ModbusClientFactory implements ProtocolClientFactory { public readonly scheme: string = "modbus+tcp"; - private singleton: ModbusClient; + private singleton?: ModbusClient; public getClient(): ProtocolClient { debug(`Get client for '${this.scheme}'`); this.init(); - return this.singleton; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- singleton is initialized in init() + return this.singleton!; } public init(): boolean { diff --git a/packages/binding-modbus/src/modbus-client.ts b/packages/binding-modbus/src/modbus-client.ts index a2a66df3a..c191ad661 100644 --- a/packages/binding-modbus/src/modbus-client.ts +++ b/packages/binding-modbus/src/modbus-client.ts @@ -20,7 +20,7 @@ import { ModbusForm, ModbusFunction } from "./modbus"; import { ProtocolClient, Content, DefaultContent, createDebugLogger, Endianness } from "@node-wot/core"; import { SecurityScheme } from "@node-wot/td-tools"; import { modbusFunctionToEntity } from "./utils"; -import { ModbusConnection, PropertyOperation } from "./modbus-connection"; +import { ModbusConnection, ModbusFormWithDefaults, PropertyOperation } from "./modbus-connection"; import { Readable } from "stream"; import { Subscription } from "rxjs/Subscription"; @@ -51,7 +51,7 @@ class ModbusSubscription { next(result); } catch (e) { if (error) { - error(e); + error(e instanceof Error ? e : new Error(JSON.stringify(e))); } clearInterval(this.interval); } @@ -94,7 +94,12 @@ export default class ModbusClient implements ProtocolClient { form = this.validateAndFillDefaultForm(form, 0); const id = `${form.href}/${form["modbus:unitID"]}#${form["modbus:function"]}?${form["modbus:address"]}&${form["modbus:quantity"]}`; - this._subscriptions.get(id).unsubscribe(); + const subscription = this._subscriptions.get(id); + if (!subscription) { + throw new Error("No subscription for " + id + " found"); + } + subscription.unsubscribe(); + this._subscriptions.delete(id); return Promise.resolve(); @@ -148,15 +153,15 @@ export default class ModbusClient implements ProtocolClient { if (content) { body = await content.toBuffer(); } - form = this.validateAndFillDefaultForm(form, body?.byteLength); + const formValidated = this.validateAndFillDefaultForm(form, body?.byteLength); - const endianness = this.validateEndianness(form); + const endianness = this.validateEndianness(formValidated); const host = parsed.hostname; const hostAndPort = host + ":" + port; if (body) { - this.validateBufferLength(form, body); + this.validateBufferLength(formValidated, body); } // find or create connection @@ -164,16 +169,16 @@ export default class ModbusClient implements ProtocolClient { if (!connection) { debug(`Creating new ModbusConnection for ${hostAndPort}`); - this._connections.set( - hostAndPort, - new ModbusConnection(host, port, { connectionTimeout: form["modbus:timeout"] || DEFAULT_TIMEOUT }) - ); - connection = this._connections.get(hostAndPort); + + connection = new ModbusConnection(host, port, { + connectionTimeout: form["modbus:timeout"] || DEFAULT_TIMEOUT, + }); + this._connections.set(hostAndPort, connection); } else { debug(`Reusing ModbusConnection for ${hostAndPort}`); } // create operation - const operation = new PropertyOperation(form, endianness, body); + const operation = new PropertyOperation(formValidated, endianness, body); // enqueue the operation at the connection connection.enqueue(operation); @@ -206,10 +211,14 @@ export default class ModbusClient implements ProtocolClient { input["modbus:unitID"] = parseInt(pathComp[1], 10) || input["modbus:unitID"]; input["modbus:address"] = parseInt(pathComp[2], 10) || input["modbus:address"]; - input["modbus:quantity"] = parseInt(query.get("quantity"), 10) || input["modbus:quantity"]; + + const queryQuantity = query.get("quantity"); + if (queryQuantity) { + input["modbus:quantity"] = parseInt(queryQuantity, 10); + } } - private validateBufferLength(form: ModbusForm, buffer: Buffer) { + private validateBufferLength(form: ModbusFormWithDefaults, buffer: Buffer) { const mpy = form["modbus:entity"] === "InputRegister" || form["modbus:entity"] === "HoldingRegister" ? 2 : 1; const quantity = form["modbus:quantity"]; if (buffer && buffer.length !== mpy * quantity) { @@ -223,7 +232,7 @@ export default class ModbusClient implements ProtocolClient { } } - private validateAndFillDefaultForm(form: ModbusForm, contentLength = 0): ModbusForm { + private validateAndFillDefaultForm(form: ModbusForm, contentLength = 0): ModbusFormWithDefaults { const mode = contentLength > 0 ? "w" : "r"; // Use form values if provided, otherwise use form values (we are more merciful then the spec for retro-compatibility) @@ -243,7 +252,7 @@ export default class ModbusClient implements ProtocolClient { } // Check if the function is a valid modbus function code - if (!Object.keys(ModbusFunction).includes(result["modbus:function"].toString())) { + if (!Object.keys(ModbusFunction).includes(form["modbus:function"].toString())) { throw new Error("Undefined function number or name: " + form["modbus:function"]); } } @@ -296,6 +305,6 @@ export default class ModbusClient implements ProtocolClient { result["modbus:pollingTime"] = form["modbus:pollingTime"] ? form["modbus:pollingTime"] : DEFAULT_POLLING; result["modbus:timeout"] = form["modbus:timeout"] ? form["modbus:timeout"] : DEFAULT_TIMEOUT; - return result; + return result as ModbusFormWithDefaults; } } diff --git a/packages/binding-modbus/src/modbus-connection.ts b/packages/binding-modbus/src/modbus-connection.ts index bc79da3e8..9598ae611 100644 --- a/packages/binding-modbus/src/modbus-connection.ts +++ b/packages/binding-modbus/src/modbus-connection.ts @@ -15,13 +15,14 @@ import ModbusRTU from "modbus-serial"; import { ReadCoilResult, ReadRegisterResult } from "modbus-serial/ModbusRTU"; import { ModbusEntity, ModbusFunction, ModbusForm } from "./modbus"; -import { Content, createLoggers, Endianness } from "@node-wot/core"; +import { Content, ContentSerdes, createLoggers, Endianness } from "@node-wot/core"; import { Readable } from "stream"; import { inspect } from "util"; const { debug, warn, error } = createLoggers("binding-modbus", "modbus-connection"); const configDefaults = { + connectionTimeout: 1000, operationTimeout: 2000, connectionRetryTime: 10000, maxRetries: 5, @@ -100,7 +101,7 @@ class ModbusTransaction { } catch (err) { warn(`Read operation failed on ${this.base}, len: ${this.quantity}, ${err}`); // inform all operations and the invoker - this.operations.forEach((op) => op.failed(err)); + this.operations.forEach((op) => op.failed(err instanceof Error ? err : new Error(JSON.stringify(err)))); throw err; } } else { @@ -111,13 +112,27 @@ class ModbusTransaction { } catch (err) { warn(`Write operation failed on ${this.base}, len: ${this.quantity}, ${err}`); // inform all operations and the invoker - this.operations.forEach((op) => op.failed(err)); + this.operations.forEach((op) => op.failed(err instanceof Error ? err : new Error(JSON.stringify(err)))); throw err; } } } } +export type ModbusFormWithDefaults = ModbusForm & + Required< + Pick< + ModbusForm, + | "modbus:function" + | "modbus:entity" + | "modbus:unitID" + | "modbus:address" + | "modbus:quantity" + | "modbus:timeout" + | "modbus:pollingTime" + > + >; + /** * ModbusConnection represents a client connected to a specific host and port */ @@ -127,14 +142,14 @@ export class ModbusConnection { client: ModbusRTU; connecting: boolean; connected: boolean; - timer: NodeJS.Timer; // connection idle timer - currentTransaction: ModbusTransaction; // transaction currently in progress or null + timer: NodeJS.Timer | null; // connection idle timer + currentTransaction: ModbusTransaction | null; // transaction currently in progress or null queue: Array; // queue of further transactions config: { - connectionTimeout?: number; - operationTimeout?: number; - connectionRetryTime?: number; - maxRetries?: number; + connectionTimeout: number; + operationTimeout: number; + connectionRetryTime: number; + maxRetries: number; }; constructor( @@ -150,6 +165,7 @@ export class ModbusConnection { this.host = host; this.port = port; this.client = new ModbusRTU(); // new ModbusClient(); + this.connected = false; this.connecting = false; this.timer = null; this.currentTransaction = null; @@ -182,8 +198,8 @@ export class ModbusConnection { if (op.base === t.base + t.quantity) { // append t.quantity += op.quantity; - - if (t.content) { + // write operation + if (t.content && op.content) { t.content = Buffer.concat([t.content, op.content]); } @@ -196,7 +212,8 @@ export class ModbusConnection { t.base -= op.quantity; t.quantity += op.quantity; - if (t.content) { + // write operation + if (t.content && op.content) { t.content = Buffer.concat([op.content, t.content]); } @@ -268,13 +285,14 @@ export class ModbusConnection { // inform all the operations that the connection cannot be recovered this.queue.forEach((transaction) => { transaction.operations.forEach((op) => { - op.failed(error); + op.failed(error instanceof Error ? error : new Error(JSON.stringify(error))); }); }); } } else if (this.client.isOpen && this.currentTransaction == null && this.queue.length > 0) { // take next transaction from queue and execute - this.currentTransaction = this.queue.shift(); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- queue.length > 0 + this.currentTransaction = this.queue.shift()!; try { await this.currentTransaction.execute(); this.currentTransaction = null; @@ -324,6 +342,10 @@ export class ModbusConnection { clearTimeout(this.timer); } + if (!transaction.content) { + throw new Error("Invoked write transaction without content"); + } + this.timer = global.setTimeout(() => this.modbusstop(), this.config.operationTimeout); const modFunc: ModbusFunction = transaction.function; @@ -399,7 +421,7 @@ export class ModbusConnection { error(`Cannot close session. ${err}`); } }); - clearInterval(this.timer); + this.timer && clearInterval(this.timer); this.timer = null; } } @@ -415,19 +437,19 @@ export class PropertyOperation { function: ModbusFunction; content?: Buffer; endianness: Endianness; - transaction: ModbusTransaction; // transaction used to execute this operation + transaction: ModbusTransaction | null; // transaction used to execute this operation contentType: string; - resolve: (value?: Content | PromiseLike) => void; - reject: (reason?: Error) => void; + resolve?: (value?: Content | PromiseLike) => void; + reject?: (reason?: Error) => void; - constructor(form: ModbusForm, endianness: Endianness, content?: Buffer) { + constructor(form: ModbusFormWithDefaults, endianness: Endianness, content?: Buffer) { this.unitId = form["modbus:unitID"]; this.registerType = form["modbus:entity"]; this.base = form["modbus:address"]; this.quantity = form["modbus:quantity"]; this.function = form["modbus:function"] as ModbusFunction; this.endianness = endianness; - this.contentType = form.contentType; + this.contentType = form.contentType ?? ContentSerdes.DEFAULT; this.content = content; this.transaction = null; } @@ -436,7 +458,7 @@ export class PropertyOperation { * Trigger execution of this operation. * */ - async execute(): Promise> { + async execute(): Promise<(Content | PromiseLike) | undefined> { return new Promise( (resolve: (value?: Content | PromiseLike) => void, reject: (reason?: Error) => void) => { this.resolve = resolve; @@ -461,12 +483,21 @@ export class PropertyOperation { done(base?: number, buffer?: Buffer): void { debug("Operation done"); + if (!this.resolve || !this.reject) { + throw new Error("Function 'done' was invoked before executing the Modbus operation"); + } + if (base === null || base === undefined) { // resolve write operation this.resolve(); return; } + if (buffer === null || buffer === undefined) { + this.reject(new Error("Write operation finished without buffer")); + return; + } + // extract the proper part from the result and resolve promise const address = this.base - base; let resp: Content; @@ -490,6 +521,9 @@ export class PropertyOperation { */ failed(reason: Error): void { warn(`Operation failed: ${reason}`); + if (!this.reject) { + throw new Error("Function 'failed' was invoked before executing the Modbus operation"); + } // reject the Promise given to the invoking script this.reject(reason); } diff --git a/packages/binding-modbus/test/modbus-connection-test.ts b/packages/binding-modbus/test/modbus-connection-test.ts index 2e5126328..4b1cc2522 100644 --- a/packages/binding-modbus/test/modbus-connection-test.ts +++ b/packages/binding-modbus/test/modbus-connection-test.ts @@ -14,10 +14,10 @@ ********************************************************************************/ import { should } from "chai"; import * as chai from "chai"; -import { ModbusForm } from "../src/modbus"; +import { ModbusFunction } from "../src/modbus"; import ModbusServer from "./test-modbus-server"; import chaiAsPromised from "chai-as-promised"; -import { ModbusConnection, PropertyOperation } from "../src/modbus-connection"; +import { ModbusConnection, ModbusFormWithDefaults, PropertyOperation } from "../src/modbus-connection"; import { Endianness } from "@node-wot/core"; // should must be called to augment all variables @@ -63,12 +63,15 @@ describe("Modbus connection", () => { describe("Operation", () => { it("should fail for unknown host", async () => { - const form: ModbusForm = { + const form: ModbusFormWithDefaults = { href: "modbus://127.0.0.2:8502", "modbus:function": 15, "modbus:address": 0, "modbus:quantity": 1, "modbus:unitID": 1, + "modbus:entity": "HoldingRegister", + "modbus:timeout": 1000, + "modbus:pollingTime": 1000, }; const connection = new ModbusConnection("127.0.0.2", 8503, { connectionTimeout: 200, @@ -83,12 +86,15 @@ describe("Modbus connection", () => { }).timeout(5000); it("should throw with timeout", async () => { - const form: ModbusForm = { + const form: ModbusFormWithDefaults = { href: "modbus://127.0.0.1:8502", + "modbus:function": ModbusFunction.readCoil, "modbus:entity": "Coil", "modbus:address": 4444, "modbus:quantity": 1, "modbus:unitID": 1, + "modbus:timeout": 1000, + "modbus:pollingTime": 1000, }; const connection = new ModbusConnection("127.0.0.1", 8502, { connectionTimeout: 100, diff --git a/packages/binding-modbus/test/test-modbus-server.ts b/packages/binding-modbus/test/test-modbus-server.ts index 6ac7e3b4e..f0eeba4dd 100644 --- a/packages/binding-modbus/test/test-modbus-server.ts +++ b/packages/binding-modbus/test/test-modbus-server.ts @@ -83,7 +83,7 @@ export default class ModbusServer { error(err.toString()); }); this.serverTCP.on("error", (err) => { - debug(err.toString()); + debug(err?.toString()); }); this.serverTCP.on("initialized", resolve); diff --git a/packages/binding-modbus/tsconfig.json b/packages/binding-modbus/tsconfig.json index e9fa7c062..df9cd1d90 100644 --- a/packages/binding-modbus/tsconfig.json +++ b/packages/binding-modbus/tsconfig.json @@ -2,7 +2,8 @@ "extends": "../../tsconfig.json", "compilerOptions": { "outDir": "dist", - "rootDir": "src" + "rootDir": "src", + "strict": true }, "include": ["src/**/*"], "references": [{ "path": "../td-tools" }, { "path": "../core" }]