Support for custom functions in RTU

[vdominguez1993]

I have notices that custom functions are supported and there is even an example for TCP.

But I was testing in RTU and I found that the custom command is sent, but the response is never processed.
This is in part functions are not supported in:
fn get_response_pdu_len(adu_buf: &BytesMut) -> Result<Option<usize>>

Additionally in this funcion some quick fix can be done for supporting exception code responses for all functions, changing the match arm from 0x81..=0xAB to 0x81..=0xFF. As all the exceptions share the same format regardless of the function code.

I guess the initial problem of not supporting this is how to determine the start and end of a frame.
I have tested some changes in the decode functions and I kind of found a way to deal with it. It's not the best option, but it's something.

diff --git a/src/codec/rtu.rs b/src/codec/rtu.rs
index 60a4cf8..5e39da0 100644
--- a/src/codec/rtu.rs
+++ b/src/codec/rtu.rs
@@ -11,6 +11,7 @@ use log::{debug, error, warn};
 use smallvec::SmallVec;
 use std::io::{Cursor, Error, ErrorKind, Result};
 use tokio_util::codec::{Decoder, Encoder};
+use std::time::{Instant, Duration};
 
 // [MODBUS over Serial Line Specification and Implementation Guide V1.02](http://modbus.org/docs/Modbus_over_serial_l
ine_V1_02.pdf), page 13
 // "The maximum size of a MODBUS RTU frame is 256 bytes."
@@ -105,6 +106,8 @@ pub(crate) struct RequestDecoder {
 #[derive(Debug, Default, Eq, PartialEq)]
 pub(crate) struct ResponseDecoder {
     frame_decoder: FrameDecoder,
+    last_time: Option<Instant>,
+    last_size: usize,
 }
 
 #[derive(Debug, Default, Eq, PartialEq)]
@@ -165,8 +168,15 @@ fn get_response_pdu_len(adu_buf: &BytesMut) -> Result<Option<usize>> {
                     // incomplete frame
                     return Ok(None);
                 }
-            }
-            0x81..=0xAB => 2,
+            },
+            // For custom functions, use all the buffer
+            65..=75 | 100..=110 => if adu_buf.len() > 3 {
+                adu_buf.len() - 3
+            } else {
+                return Ok(None);
+            },
+            // Above this value it means an exception code as the first bit is 1
+            0x81..=0xFF => 2,
             _ => {
                 return Err(Error::new(
                     ErrorKind::InvalidData,
@@ -223,12 +233,37 @@ impl Decoder for ResponseDecoder {
     type Error = Error;
 
     fn decode(&mut self, buf: &mut BytesMut) -> Result<Option<(SlaveId, Bytes)>> {
-        decode(
+        if let Some(last_time) = self.last_time {
+            // Check if 20 ms passed since the last burst of data
+            if last_time.elapsed() > Duration::from_millis(20) {
+                // If it was, clear the previous bytes received
+                buf.advance(self.last_size);
+                self.last_size = 0;
+            }
+        }
+
+        let resp = decode(
             "response",
             &mut self.frame_decoder,
             get_response_pdu_len,
             buf,
-        )
+        );
+        
+        if let Ok(Some(_)) = resp {
+            // Restart if the frame was parsed
+            self.last_time = None;
+            // Ensure last size is cleared
+            self.last_size = 0;
+        } else {
+            if let Ok(_) = resp {
+                // Only update the last_time if the frame was not parsed
+                self.last_time = Some(Instant::now());
+            }
+            // In any case, update the last size
+            self.last_size = buf.len();
+        }
+
+        resp
     }
 }
 
@@ -242,28 +277,20 @@ where
     F: Fn(&BytesMut) -> Result<Option<usize>>,
 {
     // TODO: Transform this loop into idiomatic code
-    loop {
-        let mut retry = false;
-        let res = get_pdu_len(buf)
-            .and_then(|pdu_len| {
-                debug_assert!(!retry);
-                if let Some(pdu_len) = pdu_len {
-                    frame_decoder.decode(buf, pdu_len)
-                } else {
-                    // Incomplete frame
-                    Ok(None)
-                }
-            })
-            .or_else(|err| {
-                warn!("Failed to decode {} frame: {}", pdu_type, err);
-                frame_decoder.recover_on_error(buf);
-                retry = true;
+    get_pdu_len(buf)
+        .and_then(|pdu_len| {
+            if let Some(pdu_len) = pdu_len {
+                frame_decoder.decode(buf, pdu_len)
+            } else {
+                // Incomplete frame
                 Ok(None)
-            });
-        if !retry {
-            return res;
-        }
-    }
+            }
+        })
+        .or_else(|err| {
+            warn!("Failed to decode {} frame: {}", pdu_type, err);
+            //frame_decoder.recover_on_error(buf);
+            Ok(None)
+        })
 }
 
 impl Decoder for ClientCodec {

Do you think it can be a good solution?

[llamicron]

I would also appreciate this. I have some devices that don't provide register/coil addresses, they only specify a series of bytes to send (according to the Modbus RTU spec). Currently I'm using TTYPort::write() from the serialport crate, but it is no longer maintained and I'd like to move away from that if possible. Also, this crate is async while serialport is not.

Here's the spec for a device that doesn't provide registers/coils, as an example.

Maybe I'm misunderstanding the issue and this is actually possible with tokio-modbus...

[ju6ge]

Hey just leaving a note that support for custom messages would be awesome. I also need to communicate with a device that just implements custom functions 🙈

[ju6ge]

Okay so I played around with this just to come to the conclusion that some vendors are just breaking the modbus spec shamelessly and are still calling it modbus. Anyway I do not see a way how this can be resolved nicely. So I will just have to stick with a vendor specific fork for my current project. Sucks but what ever 🤷‍♂️ …

[snejugal]

While I don't need actually custom functions, I wanted to use a function that is in the spec but tokio-modbus did not implement, which could be treated as a custom one from tokio-modbus standpoint. However, I faced the same situation with the response never being processed, and had to add support for the function in tokio-modbus (see #274). So here's my take on how tokio-modbus could handle custom functions.

As already mentioned, the initial problem is that the crate needs to know the length of the message to decode it, but this highly depends on the function being called. So, what if the user could provide us with this information? Enums don't really fit this use-case well, but this can be done with traits.

We start by defining a trait for functions:

trait Function {
    const CODE: u8;
    type Request;
    type Response;
}

This trait itself will be implemented for a dummy struct, but it will tell us what the function looks like. Here's an example:

struct WriteSingleRegister;
struct WriteSingleRegisterRequest(Address, Word);
struct WriteSingleRegisterResponse(Address, Word);

impl Function for WriteSingleRegister {
    const CODE: u8 = 0x06;
    type Request = WriteSingleRegisterRequest;
    type Response = WriteSingleRegisterResponse;
}

Next, we use this definition in Client:

#[async_trait]
trait Client {
    async fn call<F>(&mut self, request: F::Request) -> Result<F::Response>
        where
            F: Function,
            F::Request: Encode,
            F::Response: Decode;
}

Here we additionally require the request type implement Encode and the response type implement Decode. If using this function server-side, the requirements would be opposite, but as long as the user cares about one side only, they don't need to implement unnecessary traits.

Also note that with this definition, we rule out the possibility of calling one function but receiving a response for another one. Although call already checks this at runtime (and this check must be preserved), it still returns an enum in the response, so the user still has to assert that the response is of the type they want.

Encoding should be straightforward, but decoding is more interesting:

trait Decode {
    fn len(buf: Bytes) -> Result<Option<usize>>;
    fn decode(buf: Bytes) -> Result<Option<Self>>;
}

Some request/response types can return a constant length, but they can inspect the message if a dynamically-sized message is expected. A type need only concert with decoding itself, while the slave ID and the checksum should be possible to handle generically (once the message length is obtained).

With such a setup, any function can be implemented outside of tokio-modbus, though the crate may provide the most common functions from the spec. Should the user need a custom function or just an uncommon one, they can do it themselves via the Function trait fully in their own code.

Another advantage of this approach is even further type-safety. For example, the function ReadHoldingRegisters can have a const generic for the number of registers to read for cases when this quantity is statically known:

struct ReadHoldingRegisters<const N: usize>;
struct ReadHoldingRegistersRequest<const N: usize>(Address);
struct ReadHoldingRegistersResponse<const N: usize>([Word; N]);

impl<const N: usize> Function for ReadHoldingRegisters<N> {
    const CODE: u8 = 0x03;
    type Request = ReadHoldingRegistersRequest<N>;
    type Response = ReadHoldingRegistersResponse<N>;
}

A downside is that implementing this approach would require a massive overhaul of the crate. I might also miss some quirks that will arise during implementation (though I expect that this trait-based approach can be adjusted without losing the general idea). Moreover, since I'm mostly interested in the client-side of this crate, I didn't think about the server-side much, but I'm pretty sure the Function trait can be applied there as well. I'm open to feedback and might also try implementing this approach in the future.