Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
471 stars 175 forks source link

Adding ability to specify unit identifier when creating client #9

Closed jayharper closed 8 years ago

jayharper commented 8 years ago

Added the ability to specify a unit identifier when creating a modbus tcp client. Should also still be backward compatible. I also created the necessary tests.

Cloud-Automation commented 8 years ago

Hey Ray,

that's great. I will review and merge it shortly.

Thanks a lot

timelesshaze commented 8 years ago

Hello all,

this change breaks the package in case no unit_id is supplied!

1) The connect and error events in case of tcpClient without a unit_id (jsModbus.js) will never be emitted. 2) TCPClient will never use the unit_id, as it's from the wrong type (function) src/tcpClient.js.

Try the following simple test case in the module directory:

var modbus = require('./src/jsModbus.js');

modbus.createTCPServer(8888, '127.0.0.1', function (err, server) {});

// Delayed connection, just to make sure
setImmediate(function() {
    modbus.createTCPClient(8888, '127.0.0.1', function (err) {
        console.log('Connected without unit_id');
    });

    modbus.createTCPClient(8888, '127.0.0.1', 1, function (err) {
        console.log('Connected with unit_id');
    });
});

Also: please always bump semver MINOR instead of PATCH when adding new functionality.

Cloud-Automation commented 8 years ago

Timelessshaze is right. This breaks the API.

I would prefer to set the unit id after one created the tcp client or use a standard unit id with a setter function.

This way there is an additional interface but it remains compatible to lower versions. Since there is a request for another function code, I will create a new minor version 0.5.0.

jayharper commented 8 years ago

@Cloud-Automation @timelesshaze:

You were correct my initial pull request broke the API when a unit_id was not supplied. I took another stab at it. You can utilize the following code to test:

var modbus = require('./src/jsModbus.js');

// create readInputRegister handler
var rirHandler = function (start, quantity) {
  var resp = [];
  for (var i = start; i < start + quantity; i += 1) {
    resp.push(i);
  }

  return [resp];
};

var coil = false;
var writeCoilHandler = function (addr, value) {

   if (addr === 0) {
    coil = value;
  }

  return [addr, value];

};

modbus.createTCPServer(8888, '127.0.0.1', function (err, server) {
  // addHandler
  server.addHandler(4, rirHandler);
  server.addHandler(5, writeCoilHandler);
});

// Delayed connection, just to make sure
setImmediate(function() {
    var client = modbus.createTCPClient(8888, '127.0.0.1', function (err) {

        if (err){
        console.log(err);
        }
        console.log('Connected without unit_id');
        // make some calls
        client.readInputRegister(0, 10, function (resp, err) {
        // resp will look like { fc: 4, byteCount: 20, register: [ values 0 - 10 ] }
        console.log(resp);
        if (err){
        console.log(err);
        }
        });

        client.writeSingleCoil(5, true, function (resp, err) {
        // resp will look like { fc: 5, byteCount: 4, outputAddress: 5, outputValue: true }
        console.log(resp);
        if (err){
        console.log(err);
        }
        });

    });     

    var client = modbus.createTCPClient(8888, '127.0.0.1', 100, function (err) {

        if (err){
        console.log(err);
        }       
        console.log('Connected with unit_id');
        client.readInputRegister(0, 10, function (resp, err) {
        // resp will look like { fc: 4, byteCount: 20, register: [ values 0 - 9 ] }
        console.log(resp);
        if (err){
        console.log(err);
        }
        });     

        client.writeSingleCoil(5, true, function (resp, err) {
        // resp will look like { fc: 5, byteCount: 4, outputAddress: 5, outputValue: true }
        console.log(resp);
        if (err){
        console.log(err);
        }
        });

    });
});
Cloud-Automation commented 8 years ago

Merged it. Will tag it later for Version 0.5.0 when I added the write multiple coils fc.