Import Stream RTU

[Miq1]

Preparing next release

[bertmelis]

We can still keep the HardwareSerial constructors and cast the hwserial reference inside the constructor.

It's not really necessary for me but the less technical users might benefit from backwards compatability.

EDIT: This remark is completely up to you. I'm totally fine with "leaving it up to the user". I've got no other remarks.

[Miq1]

Thanks for the reading and comments. I considered offering the HardwareSerial constructors as well, but then we only can set the FIFO threshold and read out the baud rate, the buffer size remains out of reach. If we aloow that we should limit the baud rates available to 115200 and below, as I found that anything above will bust the buffer with larger messages.

I will think again...

[bertmelis]

what do you mean with the buffer size remains out of reach. If the user is able to set the buffer, we can do it too, right? Am I missing something here?

[bertmelis]

Forget my last remark. The user already calls .begin(...). This would only work if our lib calls hwserial.begin(...).

[Miq1]

Surprise, surprise.

I modified the RTU sources again to have different constructors for Stream with mandatory baud rate parameter and HardwareSerial without, as I expected to read that by HardwareSerial::baudRate() at run time, and did a setRxFIFOFull(1); there as well.

Did not work. I first got a division by zero reboot loop because the baud rate was zero at the time I called the interval calculation (huh?), but just safeguarded that with a default interval value without thinking further.

And now got lots of errors with RTU...

Reason is, the constructor for the RTU object gets called very early, if the objects are declared global. The HardwareSerial baud rates are determined at the begin() call in setup() - which happens much later. The setRxFIFOFull() fails for the same reason, it is only effective is issued after the serial's begin().

Too bad, it would have been very convenient to only have a different constructor. The begin() call of the ModbusRTU classes could be made to recognize the explicit class of the object (whether it is a HardwareSerial or some other Stream) if written as a function template, but that again will require it to be called like RTUserver.begin<HardwareSerial>(1);, which is no improvement.

The final idea was to give the Stream or HardwareSerial object with the begin() call only and omit it from the constructor. Then we would have something along

SoftwareSerial ss(...);
...
ModbusClientRTU mcr;
...
// Then
mcr.begin(Serial1);
// or
mcr.begin(ssr, 9600);

Would you like that better? It would open the opportunity to switch interfaces at run time, since every new begin() will be able to use a different.

[Miq1]

I now have a version with the proposed shift of the Stream object into the begin() call.

I do not know how to judge the result.

Positive:

• The setRxFIFOFull(1) is integrated into begin(), the user must not care about it
• switching interfaces or baud rates followed with another begin() is working

Negative:

• If you will try begin(Serial1, Serial1.baudRate()), i.e., call the Stream-type begin() with a HardwareSerial, you will get errors amass. I tried to catch and redirect such calls by checking the typeid in begin(), but that does require the -frtti build flag I am sure no one will be able to set in the Arduino IDE at least.
• I cannot prevent the user being responsible to do the setRxBufferSize(260) calls before the Serial.begin(). The problem remains the same: it has to be set before starting the interface due to the core, but stopping it, setting the buffer size and restarting it is not possible for us due to incomplete configuration information available.
• the buffer problem is symmetrical, by the way. You also will need to call setTxBufferSize(260) to prevent segmented messages the other way round.
• you will get errors with messages larger than 127 bytes at all baud rates above 9600, if the two buffer resizes have not been done before. Interestingly the number of errors seems not to be increasing significantly with the baud rate - whyever.

So we have two options:

• urgently ask the users to add the two buffer size calls before their serial begin()
• Require all serial parameters in the constructors for a HardwareSerial, stop it in case it is running, configure it to our needs and restart.

Neither of both is appealing me much.

[Miq1]

I now created a static utility function RTUutils::prepareHardwareSerial(HardwareSerial& s, uint16_t bufferSize = 260); that will set both buffers to 260 (or another value, if given).

But still we must require the users to call it correctly before the serial begin()...

[bertmelis]

I'd say that is the cleanest solution (for now?).

[Miq1]

Thanks for checking. I did not find a better solution and at least it does require less diligence from the users. I am going to merge the PR now, update the docs tomorrow and publish a release 1.7 with it.