Skip to content

Conversation

@gonzabrusco
Copy link

@gonzabrusco gonzabrusco commented Dec 22, 2021

Hello friend. I have made a small modification in modbus RTU. Now it uses micros() instead of millis() for better precision. Also, I added a function to be able to set another interframe delay when you need your device to be a bit more permissive. Because the interframe delay you calculate automatically is the MINIMUM set by the standard but it can be more than that in case of communication problems.

Also I corrected the formula for calculating the integrame delay because the modbus caracter is 11 bits not 10 bits.

Please test it on you side and let me know what you think!

@gonzabrusco
Copy link
Author

I noticed that setBaudrate() didn't do anything related to "setting a baudrate". So I decided to change the name of the function to give more clarity. Now it's calculateMinimumInterFrameTime() and just returns that, the minimum inter frame time calculated based on the baudrate. If you want to really set it you can call setInterFrameTime().

Please test it and let me know. Thanks.

Gonzalo

@emelianov
Copy link
Owner

emelianov commented Dec 23, 2021

Gonzalo,
Thanks for the PR.
My 2 cents:

  1. In my opinion switching to uS timings is a bit overkilling but see no good reason do to not switch ;).
  2. 11 bit char is not a constant. Arduino's default SERIAL_8N1 that frequently used in DIY is 10 bits. To be exactly precise it should be (default char_bits value is subject to discuss):
uint32_t ModbusRTUTemplate::calculateMinimumInterFrameTime(uint32_t baud, uint8_t char_bits = 11);
  1. In addition to new methods setBaudrate() must be kept for compatibility reasons.
  2. Probably it's better to
// Left definition as configurable values
#define MODBUSRTU_TIMEOUT 1000
#define MODBUSRTU_MAX_READMS 100

// Define for internal use
#define MODBUSRTU_TIMEOUT_US 1000UL * MODBUSRTU_TIMEOUT
#define MODBUSRTU_MAX_READ_US 1000UL * MODBUSRTU_MAX_READMS
  1. 1750UL is also must be set in
bool ModbusRTUTemplate::begin(Stream* port) {
     _port = port;
     _t = 2; // <----
  1. Slight fix in comment
uint32_t _t; // inter-frame delay in uS

@gonzabrusco
Copy link
Author

Sorry for the delay.

  1. Agreed.
  2. If you want to make it configurable, fine by me. But we should respect what the standard says about the modbus frame length. Modbus specifies 11 bit lenght so I think that should be the default value.
  3. Ok. No problem. But I doubt anyone is actually using setBaudrate() outside the method begin() because it has no use.
  4. Ok
  5. Ok
  6. Ok.

I have updated the branch. Take a look and let me know.

(I hope you have a happy new year 2022)

@emelianov
Copy link
Owner

Thanks Gonzalo,

  1. Ok. No problem. But I doubt anyone is actually using setBaudrate() outside the method begin() because it has no use.

setBaudrate/setInterFrameTime is required for generic Arduino (as Uno etc) as their implementation of HardwareSerial has no method to get current baud rate.

(Happy New Year!)

@emelianov emelianov merged commit 0cef4f5 into emelianov:master Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants