WriteMultipleCoilsRequest failure

[brianruss]

When using a WriteMultipleCoilsRequest, if the quantity parameter is not a multiple of 8, the request fails to send, and a response timeout ensues. The issue appears to be with the formula used to calculate the number of bytes that are needed to hold the quantity of bits to be written. Currently the formula (in ModbusRequestEncoder.encodeWriteMultipleCoils() ) is:

        int byteCount = (request.getQuantity() / 8) + (request.getQuantity() % 8);

This should be:

        int byteCount = 1 + (request.getQuantity() -1) / 8;

The ModbusRequestSerializationTest could be enhanced to check for these as below:

    @DataProvider
    private Object[][] getAddressAndNumCoilsAndCoilValues() {
        return new Object[][]{
                {0, 1, new byte[]{0x01}},
                {0, 2, new byte[]{0x02}},
                {0, 3, new byte[]{0x04}},
                {0, 4, new byte[]{0x08}},
                {0, 5, new byte[]{0x10}},
                {0, 6, new byte[]{0x20}},
                {0, 7, new byte[]{0x40}},
                {0, 8, new byte[]{(byte)0x80}},
                {0, 9, new byte[]{0x01, 0x01}},
                {0, 10, new byte[]{0x01, 0x02}},
                {0, 11, new byte[]{0x01, 0x04}},
                {0, 12, new byte[]{0x01, 0x08}},
                {0, 13, new byte[]{0x01, 0x10}},
                {0, 14, new byte[]{0x01, 0x20}},
                {0, 15, new byte[]{0x01, 0x40}},
                {0, 16, new byte[]{0x01, (byte)0x80}},
                {0, 17, new byte[]{0x02, 0x01, 0x01}}
        };
    }
.
.
.
    @Test(dataProvider = "getAddressAndNumCoilsAndCoilValues")
    public void testWriteMultipleCoilsRequest(int address, int numCoils, byte[] values) {
        WriteMultipleCoilsRequest request = new WriteMultipleCoilsRequest(address, numCoils, values);
        request.retain().content().markReaderIndex();

        ByteBuf encoded = encoder.encode(request, Unpooled.buffer());
        WriteMultipleCoilsRequest decoded = (WriteMultipleCoilsRequest) decoder.decode(encoded);

        request.content().resetReaderIndex();

        assertEquals(request.getAddress(), decoded.getAddress());
        assertEquals(request.getQuantity(), decoded.getQuantity());
        assertEquals(request.getValues(), decoded.getValues());
    }

The problem is detected as a IndexOutOfBoundsException, which does not get reported to the Modbus sendRequest() future.

[kevinherron]

Thank you! I'm not sure how this snuck by. The way it's currently calculating the byte count is just embarrassingly wrong.

I'll merge your changes in as well as open an issue for and implement reporting encoding and decoding errors to any outstanding CompletableFutures.

[kevinherron]

You see any issue with this formula instead?

int byteCount = (request.getQuantity() + 7) / 8;

I think this is what my brain must have been reaching for when I wrote the broken version.

[brianruss]

The alternative formula is good.