Add modbus-tcp support

[cpq]

No description provided.

[scaprile]

This is a lot of stuff, I will continue checking after we agree on the fundamentals

[scaprile]

How does this work ?
Modbus has two register spaces and two bit spaces.

• Inputs are read-only bit addressable, and can be read as several bits starting on any one bit
• Coils are read/write, same addressing
• Input registers are read-only
• Holding registers are read/write

A read coils request is a request to read from the coil space
A read inputs request is a request to read from the inputs space, a different read-only space
A read input registers request is a request to read from the input registers space, a read-only 16-bit register space
A read holding registers request is a request to read from the holding registers space, a different read/write 16-bit register space

[robertc2000]

The formats for both the response and request of each of these 2 commands is identical, so the handling logic done by us is the same. It should be up to the user to handle the distinction between inputs and coils at the user handler level.

[cpq]

There are 8 modbus commands. We pass the command to the user handler as-is
We also pass uint16_t *values array. For reads, user must fill those. For writes, user must use those.
Whatever coils and inputs are. Should we care at all ?

[scaprile]

OK, yes, I get it, so we have a nomenclature problem. I was confused by the naming and so might be any Modbus-savvy user. I even saw struct fields with that names.
IMHO, we should use generic names avoiding the words inputs, coils, and holding. Clearly indicate that is pointer to bits or registers, and that will avoid my confusion.
I'll look at the code again

[scaprile]

OK, yeah, the problem is with names...
@cpq No, we don't need to care, though those who know what they're doing may get confused if we use these names.
I would remove "coil" from these function names, as said: we should use generic names avoiding the words inputs, coils, and holding. Clearly indicate that is pointer to bit units or 16-bit registers, and that will avoid my confusion (and any other Modbus-savvy user).
Perhaps with a bit of API documentation... also, there's always a confusion on whether the data is already in host format or in Modbus native format (big endian), I understand it is host format, that should be clearly stated.

[scaprile]

READ --> set ?

[robertc2000]

This is responding to a READ request in the user handler. First, the user should set the data pointer (to coil_arr in our case). Then, we fill that array with the actual response (bool coil_1[] or bool coil_7[] depending on the test case).

My intention was to provide an API function (mg_modbus_set_coil) that should make it much easier for user to fill the coil/inputs buffer that we include in the response.

[scaprile]

This seems to be TCP stuff, it does not belong here, rejection must be done before attempting to switch on the command, otherwise the server code is dependent on TCP and can't be used for serial

[robertc2000]

I think it's useful to include the Transaction Identifier field (txid) in the structure. It's part of Modbus after all, even though it's just a field in the header. Main use of it is that the txid in the response must match the one in the request.

The only reason I use this field here in this test is to uniquely identify each test case. For example, the test with txid = 0x1c that you mentioned, is supposed to return a SERVER DEVICE FAILURE error. This can happen on the user side, the user handler logic can choose to trigger such an error, depending on their logic, so it's not a protocol-related error, such as an illegal function name or illegal size (which we handle in the server protocol handler BTW). Basically, this test is just a simulation of an user deciding to reply with SERVER DEVICE FAILURE to this request.

[scaprile]

No, txid is not part of Modbus... it is Modbus-TCP, it is another layer. Serial Modbus transactions do not have that field nor anything else in the TCP stuff. Yeah, this can be done later if necessary... just trying to avoid weeks of work as what decoupling from Ethernet was...

[robertc2000]

My understanding is @cpq wants only Modbus-TCP so far, we already assume the header is 7 bytes and the serial version does not have such header.

[scaprile]

The serial version has a different framing, but the application protocol is the same.
This is what I've done years ago, so details are clear:

struct mbmessage_s {
	uint8_t address;
	uint8_t function;
	union funcdata_u funcdata;
};

#define MBTCP_ADU_MAXLEN	260	// (MODBUS_MAXPDU_LEN+1+sizeof(struct mbtcpmbap_s))

struct mbtcpmbap_s {			// values are in network order, Modbus order, big endian
	uint16_t transaction;
	uint16_t protocol;
	uint16_t length;
	uint8_t unit;
};

struct mbtcpadu_s { 
	struct mbtcpmbap_s mbap;
	struct mbmessage_s message;		// with some bytes
};

#define MBSERIAL_ADU_MAXLEN	256	// (MODBUS_MAXPDU_LEN+1+sizeof(CRC))

union mbserialadu_u {
	struct mbmessage_s message;		// with some bytes, plus CRC
	uint8_t bytes[MBSERIAL_ADU_MAXLEN];
};