morkai / h5.modbus

Implementation of the MODBUS IP/ASCII/RTU master and slave over TCP/UDP/Serial for Node.js.
https://miracle.systems/p/h5.modbus
MIT License
28 stars 21 forks source link

Interval Option - Write Values passed by reference #6

Closed JeremyBYU closed 9 years ago

JeremyBYU commented 9 years ago

I am doing a write coils to a modbus slave through serial rtu. I am specifying the data to write through the variable "states". The writeMultipleCoils function I am calling takes advantage of your "interval" option.

I am changing the variable "states" periodically (as seen in the below code). I have verified that the variable is indeed changing. However, the modbus write command is still writing the original states value. Its as if it passed the variable state by value and not by reference. Is this truly the case, or am I missing something here?

var COIL_ADDRESS = 0x0000;
            states = [false];

            t2 = master.writeMultipleCoils(COIL_ADDRESS, states, {
                unit: 1,
                maxRetries: 10,
                timeout: 1000,
                interval: 3000,
                onComplete: function(err, response) {
                    console.log(states);
                    if (err) {
                        console.err(err.message);
                        console.err('I make the error here!');
                    } else {

                        console.log(response);
                        console.log(response.exceptionCode);
                        if (states[0] === true)
                        {
                            states = [false];
                        }
                        else
                        {
                            states = [true];
                        }

                        //console.log(response.values.readUInt16BE(0));
                    }

                }
            });
morkai commented 9 years ago

That's because JavaScript is different :)

If you want to change the states array stored by the WriteMultipleCoilsRequest you have to change the first element of the array and not override the whole variable. Instead of:

if (states[0] === true)
{
    states = [false];
}
else
{
    states = [true];
}

this:

states[0] = !states[0];

...but a part of the request frame buffer (Application Data Unit) is cached by the Transport the first time it is created and stored in the request's Transaction, so changing the states array won't change the cached ADU. If you want to go that road, then you must reset the ADU when changing the states array:

states[0] = !states[0];
this.setAdu(null); // this inside callbacks is an instance of Transaction

...but the interval option is meant to be used with read requests only, because they don't change (and that's why the above solution might seem like a hack). The proper way to do it is to always create a new write request:

function toggleCoils(address, interval, states) {
    master.writeMultipleCoils(address, states, {
        unit: 1,
        maxRetries: 10,
        timeout: 1000,
        onComplete: function(err, response) {
            if (err) {
                console.error(err.message);
            } else {
                console.log(response.toString());
            }
            setTimeout(
              toggleState,
              interval,
              address,
              interval,
              states.map(function(state) { return !state; })
            );
        }
    });
}

toggleCoils(0x0000, 3000, [false]);
JeremyBYU commented 9 years ago

Oh thanks! Yeah the write interval was just for testing, but I see I really didn't do it correctly.

Thanks again.