Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
466 stars 174 forks source link

V3...TCP Multi-Unit...Communication hang #180

Closed kevingarman closed 6 years ago

kevingarman commented 6 years ago

I've spent hours trying to figure out what's happening but I have yet to get it...here's my setup. I'm using diagslave for a test server (I see the same thing with real modbus gateway) http://www.modbusdriver.com/diagslave.html. I'm trying to read two devices from the server (unit IDs 1 and 2) each with two reads...for testing, I'm doing the reads every 10 seconds. One device works fine and two devices with one read each works fine.

Here's what I'm seeing...

Both device 1 reads work fine every time, but only one of the device 2 reads gets sent per read cycle. The reply comes back immediately but it doesn't get handled until the next round of reads starts and so the second read can't get sent thus the device 2 reads start piling up in the request queue.

Hopefully some of that makes sense...

stefanpoeter commented 6 years ago

Can you present some code.

kevingarman commented 6 years ago

I've created a simple test and it seems to be working, so it's looking like it must be something in my code...

kevingarman commented 6 years ago

Ok, I've got my test code exhibiting the same behavior...here's the code...

'use strict';

const mb = require('jsmodbus');
const net = require('net');

const IP = '127.0.0.1';
const PORT = 502;
const POLL_INTERVAL = 2000;

const sock = new net.Socket();
sock.connect({
    host: IP,
    port: PORT
});

let dev1 = new mb.client.TCP(sock, 1, 5000);
let dev2 = new mb.client.TCP(sock, 2, 5000);

setInterval(() => {
    console.error('polling 1');
    dev1.readInputRegisters(5008, 8).then((result) => {
        console.info('success 1', result.response.unitId);
    }).catch(() => {
        console.info('fail 1');
    });
    dev1.readHoldingRegisters(6638, 12).then((result) => {
        console.info('success 1', result.response.unitId);
    }).catch(() => {
        console.info('fail 1');
    });
}, POLL_INTERVAL);

setInterval(() => {
    console.error('polling 2');
    dev2.readInputRegisters(5008, 8).then((result) => {
        console.info('success 2', result.response.unitId);
    }).catch(() => {
        console.info('fail 2');
    });
    dev2.readHoldingRegisters(6638, 12).then((result) => {
        console.info('success 2', result.response.unitId);
    }).catch(() => {
        console.info('fail 2');
    });
}, POLL_INTERVAL);

setInterval(() => {
    console.info('\n\n\n\n');
}, Math.floor(POLL_INTERVAL / 3));

If you let it run for a number of polling cycles you should see that device 2 only gets one result per cycle. I want the devices to be completely separate (have their own polling loops as above), but that seems to be what is causing the problem. If the reads are all done in a single interval it works as expected.

The diagslave command I'm using is: sudo ./diagslave -m tcp

kevingarman commented 6 years ago

I suppose I could queue my read/write requests in a shared queue and then have one interval that processes the queue...though it seems like the above should work.

kevingarman commented 6 years ago

...putting the two polling intervals on slightly different times allows the test code to work...somehow requests are stepping on each other.

kevingarman commented 6 years ago

I got it! My fix for filtering on unitId was dropping responses if multiple came back at once. See pull request #182.