steveohara / j2mod

Enhanced Modbus library implemented in the Java programming language
Apache License 2.0
267 stars 111 forks source link

Function code 0x17 first performs a read and then the write. #32

Closed elasticoder closed 7 years ago

elasticoder commented 7 years ago

According to the specification a Read/Write request in a single command (function code 23, or 0x17) should first perform the write and then the read command.

If you write to an address and read the same address back using the same command, you would expect the number you've just written to be returned. (used for is-alive tests)

Yet, the second call returns the value written so it always lacks a command behind.

From the spec: This function code performs a combination of one read operation and one write operation in a single MODBUS transaction. The write operation is performed before the read.

elasticoder commented 7 years ago

The solution (I think), for class ReadWriteMultipleRequest, look for the following function:

public ModbusResponse createResponse(AbstractModbusListener listener) {
    ReadWriteMultipleResponse response;
    InputRegister[] readRegs;
    Register[] writeRegs;

    // 1. get process image
    ProcessImage procimg = listener.getProcessImage(getUnitID());
    // 2. get input registers range
    try 
    {
        // First the write
        writeRegs = procimg.getRegisterRange(getWriteReference(), getWriteWordCount());

        for (int i = 0; i < writeRegs.length; i++) {
            writeRegs[i].setValue(getRegister(i).getValue());
        }

        // And then the read
        readRegs = procimg.getRegisterRange(getReadReference(), getReadWordCount());

        InputRegister[] dummy = new InputRegister[readRegs.length];
        for (int i = 0; i < readRegs.length; i++) {
            dummy[i] = new SimpleInputRegister(readRegs[i].getValue());
        }

        readRegs = dummy;

    }
    catch (IllegalAddressException e) {
        return createExceptionResponse(Modbus.ILLEGAL_ADDRESS_EXCEPTION);
    }
    response = (ReadWriteMultipleResponse)getResponse();
    response.setRegisters(readRegs);

    return response;
}
steveohara commented 7 years ago

I agree.
I've copied your changes and built them into 2.3.1-SNAPSHOT. Could you give it a try and confirm it's now doing it correctly.

steveohara commented 7 years ago

Merged into 2.3.1