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.

Sign up for GitHub

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

Use ts strict checking for binding modbus #1050

Merged
merged 3 commits into from
Aug 11, 2023
Merged

Use ts strict checking for binding modbus #1050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fea3cf7
chore(binding-modbus): use ts strict checking
relu91 Aug 3, 2023
792c429
chore(binding-modbus/modbus-client-factory): remove leftovers when te…
relu91 Aug 9, 2023
dbf9979
refactor(binding-modbus/modbus-connection): improve error messages in…
relu91 Aug 11, 2023
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
5 changes: 3 additions & 2 deletions packages/binding-modbus/src/modbus-client-factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
43 changes: 26 additions & 17 deletions packages/binding-modbus/src/modbus-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -148,32 +153,32 @@ 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
let connection = this._connections.get(hostAndPort);

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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
Expand All @@ -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"]);
}
}
Expand Down Expand Up @@ -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;
}
}
76 changes: 55 additions & 21 deletions packages/binding-modbus/src/modbus-connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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
*/
Expand All @@ -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<ModbusTransaction>; // queue of further transactions
config: {
connectionTimeout?: number;
operationTimeout?: number;
connectionRetryTime?: number;
maxRetries?: number;
connectionTimeout: number;
operationTimeout: number;
connectionRetryTime: number;
maxRetries: number;
};

constructor(
Expand All @@ -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;
Expand Down Expand Up @@ -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]);
}

Expand All @@ -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]);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -399,7 +421,7 @@ export class ModbusConnection {
error(`Cannot close session. ${err}`);
}
});
clearInterval(this.timer);
this.timer && clearInterval(this.timer);
this.timer = null;
}
}
Expand All @@ -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<Content>) => void;
reject: (reason?: Error) => void;
resolve?: (value?: Content | PromiseLike<Content>) => 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;
}
Expand All @@ -436,7 +458,7 @@ export class PropertyOperation {
* Trigger execution of this operation.
*
*/
async execute(): Promise<Content | PromiseLike<Content>> {
async execute(): Promise<(Content | PromiseLike<Content>) | undefined> {
return new Promise(
(resolve: (value?: Content | PromiseLike<Content>) => void, reject: (reason?: Error) => void) => {
this.resolve = resolve;
Expand All @@ -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;
Expand All @@ -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);
}
Expand Down
14 changes: 10 additions & 4 deletions packages/binding-modbus/test/modbus-connection-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/binding-modbus/test/test-modbus-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading