RawRequest addition

[Miq1]

Reading the thread on data and type casting Hasenburg had raised, I found my original approach on raw requests that are agnostic to the function codes was shortsighted. As there simply is no common data element to describe the length of any response, I will have to stick to the MODBUS basics, I am afraid.

The receive() function will have to look for bus intervals to determine the end of any transmissions. This is standard for MODBUS slaves in the wild and I have implemented it already in another project, but the timing is critical. The calculation of interval better has to use micros() instead of millis(), and receive() will have to be a tight loop state machine.

This may have an impact on all currently implemented function codes, as the response length would not be needed anymore.

I will have a closer look at this and report back.

[bertmelis]

It's a different approach but perfectly doable:

• read the Serial and wait for the silent period and if there is something, advance a step in the FSM
• parse the received data and generate a valid message out of it
• if the message is something we expected (eg: has a callback attached to it etc): go on, otherwise discard
• start all over

[Miq1]

Well, yes, that is exactly how I did it before :)

Shall I implement it in the lib?

[bertmelis]

Yes, I was thinking of doing something like that because I want to combine RTU and TCP. This approach can then be shared. Only differences are the way data comes in and the validator offcourse.

[Miq1]

That is great! I was about to add in the TCP variant next, to enable my Bridge device to talk both ways. I was considering to add another constructor that takes an EthernetClient instead of a Serial as parameter. Requests will be done by giving IP address and port in the call, using another xQueue and xTask. The instance will open the TCP connection, do the request, wait for the response and close again.

But let me put in the token and raw request features first.

[Miq1]

Some more thoughts about the response handling:

• the lib in its current state is making the same assumption that I made: the third byte of a response is assumed to be the length byte. This is only correct for function codes intended for data reading (0x01, 0x02, 0x03 and 0x04), writing FCs (0x05, 0x06, 0x0F and 0x10) will echo the request instead. The contents of user-defined function codes are completely unknown.

• bus timing is less critical than I thought. I programmed a slave device before where this is much different, as the slave has to get the start of every single request or response on the bus to determine if it is affected. Since the lib is for a master device, the master may choose whatever bus timing (interval) it will require.

• most responses are short. As we do not know in advance the length of a response packet, we need to collect it in a fixed buffer first. I would reckon a buffer of - say - 128 bytes will be sufficient for >95% of all responses. Only if that fixed buffer is full, we may need to allocate another to catch the overflow, a third if that is full and so on. This will help keeping the FSM timing tight.

• Given a working universal receive function, we may drop the response_length mechanism completely. This again will allow to rewrite the known requests (FCs 0x01, 0x02 etc.) in a much simpler form using the RawRequest(slave, fc, *data, data_length, token) call.

As soon as you will have merged the Token PR I will start implementing the new receive and RawRequest functions. I am shying away from starting another branch on top of the Token branch, as I do not know if the merge will be complicated if you try to merge the RawRequest first etc.

Ciao, Michael

[Miq1]

Another one... Currently ModbusMessage.cpp has

uint8_t* ModbusResponse::getData() {
  return &_buffer[3];
}

uint8_t ModbusResponse::getByteCount() {
  return _buffer[2];
}

Both functions will only be correct for the data reading function codes. The others will not have the length byte, so getData() will return a wrong pointer (1 off) and getByteCount()will even return some arbitrary byte.

I would suggest replacing these functions by

uint8_t* ModbusResponse::getData() {
  uint8_t fc = _request->getFunctionCode();
  if(fc == 0x01 || fc == 0x02 || fc == 0x03 || fc == 0x04) {
    return &_buffer[3];
  }
  else {
    return &_buffer[2];
  }
}

uint8_t ModbusResponse::getByteCount() {
  uint8_t fc = _request->getFunctionCode();
  if(fc == 0x01 || fc == 0x02 || fc == 0x03 || fc == 0x04) {
    return _buffer[2];
  }
  else {
    return _index - 2;
  }
}

This will keep the interface intact for existing implementations, but allow for correctness with added function codes. I had to add getFunctionCode() to ModbusRequest, though.

I also changed the return type of ModbusResponse::getFunctionCode() to uint8_t, since we may encounter function codes that are not encoded in esp32Modbus::FunctionCode.

[Miq1]

Closing this as has been implemented in #41