Response Generation Quantity Issue

[MatthewBoeding]

When parsing a request to generate a read coils response, the number of inputs is not parsed correctly. The bytes value should be a uint16_t instead of uint8_t. The standard requires 0x0000-0xFFFF. Currently, if you're value is 0x0000-0x00FF, you won't generate any value bytes.

Modbus/src/modbusResponse.cpp

Lines 46 to 51 in 9bd2ad4

	uint8_t bytes;
	switch (_functionCode) {
	case utils::ReadDiscreteOutputCoils:
	case utils::ReadDiscreteInputContacts:
	bytes = inputData[2];

You'll also need to change the value to be read:

uint16_t bytes; 
  
switch (_functionCode) { 
    case utils::ReadDiscreteOutputCoils: 
    case utils::ReadDiscreteInputContacts: 
    bytes = inputData[2] << 8 + inputData[3]; 

[Mazurel]

Interesting, I don't have access to the Modbus standard right now, but I will look for it. I was basing this on descriptions from simply modbus: https://www.simplymodbus.ca/FC02.htm

There you can see, that in response, there is only one byte to describe the amount of bytes:

[MatthewBoeding]

First, thank you for your response and all the work on this library!

You are correct, the response packet is correctly generated for the size of each field. I should clarify the issue is solely with parsing a request in the response constructor. I'm using this as a reference for packet testing: https://modbus.org/docs/Modbus_Application_Protocol_V1_1b3.pdf

In testing with a packet similar to this. If the request packet is generated with anything more than 255 in the third argument, it gets casted to 0.

MB::ModbusRequest request(1, MB::utils::ReadDiscreteOutputCoils, 100, 10);  
std::vector<uint8_t> pack = request.toRaw();
 
MB::ModbusResponse response(pack, bool(false)); 

Actually, looking at the Modbus Request Constructor, the address may be being interpreted as the quantity:

Modbus/src/modbusRequest.cpp

Lines 13 to 17 in 7d5352a

	ModbusRequest::ModbusRequest(uint8_t slaveId, utils::MBFunctionCode functionCode,
	uint16_t address, uint16_t registersNumber,
	std::vector<ModbusCell> values) noexcept
	: _slaveID(slaveId), _functionCode(functionCode), _address(address),
	_registersNumber(registersNumber), _values(std::move(values)) {

So the read packets are interpreting the address as the quantity:

Modbus/src/modbusResponse.cpp

Lines 48 to 51 in 9bd2ad4

	switch (_functionCode) {
	case utils::ReadDiscreteOutputCoils:
	case utils::ReadDiscreteInputContacts:
	bytes = inputData[2];

Modbus/src/modbusResponse.cpp

Lines 59 to 61 in 9bd2ad4

	case utils::ReadAnalogOutputHoldingRegisters:
	case utils::ReadAnalogInputRegisters:
	bytes = inputData[2];

I may look at tinkering with this and put in a pull request if you are interested.

[Mazurel]

I will quickly look at it and try to prepare some test case for it. It seems not that complicated.

I will try to make a PR (and mention you there), if I will have any problems with that, I will ask you for some help

[Mazurel]

Ok, it just clicked for me - I understand what is the issue now. I will mark PR as ready when I will fix it

[Mazurel]

@MatthewBoeding please take a look at PR #12 - it should resolve this issue, I have also added some more tests around that area.