Skip to content

[Bug] Duplicate byte order swap #3

@JanB1

Description

@JanB1

I found that the behaviour for readInputRegisters8i and readHoldingRegisters8i is different and readHoldingRegisters8i potentially has a bug.

In the readHoldingRegisters8i function you have a for-Loop that swaps the order of the bytes in the received answer from the Holding Register read, regardless of the byte order setting.

master-modbus/mmodbus.c

Lines 456 to 466 in f5f8ebd

if(data != NULL)
{
for(uint8_t i=0 ; i<mmodbus.rxBuf[2] ; i+=2)
{
uint8_t H = mmodbus.rxBuf[i+3];
mmodbus.rxBuf[i+3] = mmodbus.rxBuf[i+3+1];
mmodbus.rxBuf[i+3+1] = H;
}
memcpy(data, &mmodbus.rxBuf[3], mmodbus.rxBuf[2]);
}
return true;

Then, in for example readHoldingRegisters16i for example depending on the byte order setting, the bytes are swapped again.

master-modbus/mmodbus.c

Lines 540 to 559 in f5f8ebd

uint8_t tmp1[2],tmp2[2];
for(uint16_t i=0 ; i<length ; i++)
{
switch(mmodbus.byteOrder16)
{
case MModBus_16bitOrder_AB:
memcpy(tmp1, &data[i], 2);
tmp2[0] = tmp1[0];
tmp2[1] = tmp1[1];
memcpy(&data[i], tmp2, 2);
break;
default:
memcpy(tmp1, &data[i], 2);
tmp2[0] = tmp1[1];
tmp2[1] = tmp1[0];
memcpy(&data[i], tmp2, 2);
break;
}
}
return true;

This is inconsistent with the way the answer for the input register read is processed, where in readInputRegisters8i the data is just copied directly and then in readInputRegisters16i the byte order is swapped depending on the byte order setting.

master-modbus/mmodbus.c

Lines 330 to 332 in f5f8ebd

if(data != NULL)
memcpy(data, &mmodbus.rxBuf[3], mmodbus.rxBuf[2]);
return true;

master-modbus/mmodbus.c

Lines 406 to 425 in f5f8ebd

uint8_t tmp1[2],tmp2[2];
for(uint16_t i=0 ; i<length ; i++)
{
switch(mmodbus.byteOrder16)
{
case MModBus_16bitOrder_AB:
memcpy(tmp1, &data[i], 2);
tmp2[0] = tmp1[0];
tmp2[1] = tmp1[1];
memcpy(&data[i], tmp2, 2);
break;
default:
memcpy(tmp1, &data[i], 2);
tmp2[0] = tmp1[1];
tmp2[1] = tmp1[0];
memcpy(&data[i], tmp2, 2);
break;
}
}
return true;

This leads to inconsistent behaviour between reading an input or holding register.
Can you confirm this is a bug, or is this intended behaviour and I'm missing something here?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions