feat: add Exception type to public api (#246)

slowtec · Jan 21, 2024 · b481976 · b481976
1 parent 2a7ae27
commit b481976
Show file tree

Hide file tree

Showing 17 changed files with 405 additions and 126 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,10 +3,20 @@
 
 # Changelog
 
-## v0.10.1 (Unreleased)
+## v0.11.0 (Unreleased)
 
 - Server: Remove `Sync` and `Unpin` trait bounds from `Service::call()` future
   result.
+- Feature: Add `Exception` to the public API.
+- Example: Update examples with new `Service` trait for the Server.
+- Tests: Add `Exception` integration tests for `tcp-server`, `rtu-server` and
+  `rtu-over-tcp-server`.
+
+### Breaking Changes
+
+- Server: Update `Service::Future` generic associated type.
+- Server: Remove `Service::Response` and `Service::Error` generic associated
+  type.
 
 ## v0.10.0 (2024-01-03)
 

diff --git a/examples/rtu-over-tcp-server.rs b/examples/rtu-over-tcp-server.rs
@@ -28,9 +28,7 @@ struct ExampleService {
 
 impl tokio_modbus::server::Service for ExampleService {
     type Request = SlaveRequest<'static>;
-    type Response = Response;
-    type Error = std::io::Error;
-    type Future = future::Ready<Result<Self::Response, Self::Error>>;
+    type Future = future::Ready<Result<Response, Exception>>;
 
     fn call(&self, req: Self::Request) -> Self::Future {
         println!("{}", req.slave);
@@ -68,11 +66,7 @@ impl tokio_modbus::server::Service for ExampleService {
             }
             _ => {
                 println!("SERVER: Exception::IllegalFunction - Unimplemented function code in request: {req:?}");
-                // TODO: We want to return a Modbus Exception response `IllegalFunction`. https://github.com/slowtec/tokio-modbus/issues/165
-                future::ready(Err(std::io::Error::new(
-                    std::io::ErrorKind::AddrNotAvailable,
-                    "Unimplemented function code in request".to_string(),
-                )))
+                future::ready(Err(Exception::IllegalFunction))
             }
         }
     }
@@ -101,19 +95,15 @@ fn register_read(
     registers: &HashMap<u16, u16>,
     addr: u16,
     cnt: u16,
-) -> Result<Vec<u16>, std::io::Error> {
+) -> Result<Vec<u16>, Exception> {
     let mut response_values = vec![0; cnt.into()];
     for i in 0..cnt {
         let reg_addr = addr + i;
         if let Some(r) = registers.get(&reg_addr) {
             response_values[i as usize] = *r;
         } else {
-            // TODO: Return a Modbus Exception response `IllegalDataAddress` https://github.com/slowtec/tokio-modbus/issues/165
             println!("SERVER: Exception::IllegalDataAddress");
-            return Err(std::io::Error::new(
-                std::io::ErrorKind::AddrNotAvailable,
-                format!("no register at address {reg_addr}"),
-            ));
+            return Err(Exception::IllegalDataAddress);
         }
     }
 
@@ -126,18 +116,14 @@ fn register_write(
     registers: &mut HashMap<u16, u16>,
     addr: u16,
     values: &[u16],
-) -> Result<(), std::io::Error> {
+) -> Result<(), Exception> {
     for (i, value) in values.iter().enumerate() {
         let reg_addr = addr + i as u16;
         if let Some(r) = registers.get_mut(&reg_addr) {
             *r = *value;
         } else {
-            // TODO: Return a Modbus Exception response `IllegalDataAddress` https://github.com/slowtec/tokio-modbus/issues/165
             println!("SERVER: Exception::IllegalDataAddress");
-            return Err(std::io::Error::new(
-                std::io::ErrorKind::AddrNotAvailable,
-                format!("no register at address {reg_addr}"),
-            ));
+            return Err(Exception::IllegalDataAddress);
         }
     }
 

diff --git a/examples/rtu-server-address.rs b/examples/rtu-server-address.rs
@@ -13,21 +13,19 @@ struct Service {
 
 impl tokio_modbus::server::Service for Service {
     type Request = SlaveRequest<'static>;
-    type Response = Option<Response>;
-    type Error = std::io::Error;
-    type Future = future::Ready<Result<Self::Response, Self::Error>>;
+    type Future = future::Ready<Result<Response, Exception>>;
 
     fn call(&self, req: Self::Request) -> Self::Future {
         if req.slave != self.slave.into() {
-            return future::ready(Ok(None));
+            return future::ready(Err(Exception::IllegalFunction));
         }
         match req.request {
             Request::ReadInputRegisters(_addr, cnt) => {
                 let mut registers = vec![0; cnt.into()];
                 registers[2] = 0x77;
-                future::ready(Ok(Some(Response::ReadInputRegisters(registers))))
+                future::ready(Ok(Response::ReadInputRegisters(registers)))
             }
-            _ => unimplemented!(),
+            _ => future::ready(Err(Exception::IllegalFunction)),
         }
     }
 }

diff --git a/examples/rtu-server.rs b/examples/rtu-server.rs
@@ -11,9 +11,7 @@ struct Service;
 
 impl tokio_modbus::server::Service for Service {
     type Request = SlaveRequest<'static>;
-    type Response = Response;
-    type Error = std::io::Error;
-    type Future = future::Ready<Result<Self::Response, Self::Error>>;
+    type Future = future::Ready<Result<Response, Exception>>;
 
     fn call(&self, req: Self::Request) -> Self::Future {
         match req.request {

diff --git a/examples/tcp-server.rs b/examples/tcp-server.rs
@@ -28,9 +28,7 @@ struct ExampleService {
 
 impl tokio_modbus::server::Service for ExampleService {
     type Request = Request<'static>;
-    type Response = Response;
-    type Error = std::io::Error;
-    type Future = future::Ready<Result<Self::Response, Self::Error>>;
+    type Future = future::Ready<Result<Response, Exception>>;
 
     fn call(&self, req: Self::Request) -> Self::Future {
         match req {
@@ -67,11 +65,7 @@ impl tokio_modbus::server::Service for ExampleService {
             }
             _ => {
                 println!("SERVER: Exception::IllegalFunction - Unimplemented function code in request: {req:?}");
-                // TODO: We want to return a Modbus Exception response `IllegalFunction`. https://github.com/slowtec/tokio-modbus/issues/165
-                future::ready(Err(std::io::Error::new(
-                    std::io::ErrorKind::AddrNotAvailable,
-                    "Unimplemented function code in request".to_string(),
-                )))
+                future::ready(Err(Exception::IllegalFunction))
             }
         }
     }
@@ -100,7 +94,7 @@ fn register_read(
     registers: &HashMap<u16, u16>,
     addr: u16,
     cnt: u16,
-) -> Result<Vec<u16>, std::io::Error> {
+) -> Result<Vec<u16>, Exception> {
     let mut response_values = vec![0; cnt.into()];
     for i in 0..cnt {
         let reg_addr = addr + i;
@@ -109,10 +103,7 @@ fn register_read(
         } else {
             // TODO: Return a Modbus Exception response `IllegalDataAddress` https://github.com/slowtec/tokio-modbus/issues/165
             println!("SERVER: Exception::IllegalDataAddress");
-            return Err(std::io::Error::new(
-                std::io::ErrorKind::AddrNotAvailable,
-                format!("no register at address {reg_addr}"),
-            ));
+            return Err(Exception::IllegalDataAddress);
         }
     }
 
@@ -125,18 +116,15 @@ fn register_write(
     registers: &mut HashMap<u16, u16>,
     addr: u16,
     values: &[u16],
-) -> Result<(), std::io::Error> {
+) -> Result<(), Exception> {
     for (i, value) in values.iter().enumerate() {
         let reg_addr = addr + i as u16;
         if let Some(r) = registers.get_mut(&reg_addr) {
             *r = *value;
         } else {
             // TODO: Return a Modbus Exception response `IllegalDataAddress` https://github.com/slowtec/tokio-modbus/issues/165
             println!("SERVER: Exception::IllegalDataAddress");
-            return Err(std::io::Error::new(
-                std::io::ErrorKind::AddrNotAvailable,
-                format!("no register at address {reg_addr}"),
-            ));
+            return Err(Exception::IllegalDataAddress);
         }
     }
 

diff --git a/examples/tls-server.rs b/examples/tls-server.rs
@@ -78,9 +78,7 @@ struct ExampleService {
 
 impl tokio_modbus::server::Service for ExampleService {
     type Request = Request<'static>;
-    type Response = Response;
-    type Error = std::io::Error;
-    type Future = future::Ready<Result<Self::Response, Self::Error>>;
+    type Future = future::Ready<Result<Response, Exception>>;
 
     fn call(&self, req: Self::Request) -> Self::Future {
         match req {
@@ -117,11 +115,7 @@ impl tokio_modbus::server::Service for ExampleService {
             }
             _ => {
                 println!("SERVER: Exception::IllegalFunction - Unimplemented function code in request: {req:?}");
-                // TODO: We want to return a Modbus Exception response `IllegalFunction`. https://github.com/slowtec/tokio-modbus/issues/165
-                future::ready(Err(std::io::Error::new(
-                    std::io::ErrorKind::AddrNotAvailable,
-                    "Unimplemented function code in request".to_string(),
-                )))
+                future::ready(Err(Exception::IllegalFunction))
             }
         }
     }
@@ -150,19 +144,15 @@ fn register_read(
     registers: &HashMap<u16, u16>,
     addr: u16,
     cnt: u16,
-) -> Result<Vec<u16>, std::io::Error> {
+) -> Result<Vec<u16>, Exception> {
     let mut response_values = vec![0; cnt.into()];
     for i in 0..cnt {
         let reg_addr = addr + i;
         if let Some(r) = registers.get(&reg_addr) {
             response_values[i as usize] = *r;
         } else {
-            // TODO: Return a Modbus Exception response `IllegalDataAddress` https://github.com/slowtec/tokio-modbus/issues/165
             println!("SERVER: Exception::IllegalDataAddress");
-            return Err(std::io::Error::new(
-                std::io::ErrorKind::AddrNotAvailable,
-                format!("no register at address {reg_addr}"),
-            ));
+            return Err(Exception::IllegalDataAddress);
         }
     }
 
@@ -175,18 +165,14 @@ fn register_write(
     registers: &mut HashMap<u16, u16>,
     addr: u16,
     values: &[u16],
-) -> Result<(), std::io::Error> {
+) -> Result<(), Exception> {
     for (i, value) in values.iter().enumerate() {
         let reg_addr = addr + i as u16;
         if let Some(r) = registers.get_mut(&reg_addr) {
             *r = *value;
         } else {
-            // TODO: Return a Modbus Exception response `IllegalDataAddress` https://github.com/slowtec/tokio-modbus/issues/165
             println!("SERVER: Exception::IllegalDataAddress");
-            return Err(std::io::Error::new(
-                std::io::ErrorKind::AddrNotAvailable,
-                format!("no register at address {reg_addr}"),
-            ));
+            return Err(Exception::IllegalDataAddress);
         }
     }
 

diff --git a/src/frame/mod.rs b/src/frame/mod.rs
@@ -449,7 +449,7 @@ impl From<Result<Response, ExceptionResponse>> for ResponsePdu {
     feature = "tcp-server"
 ))]
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct OptionalResponsePdu(pub(crate) Option<ResponsePdu>);
+pub(crate) struct OptionalResponsePdu(pub(crate) Option<ResponsePdu>);
 
 #[cfg(any(
     feature = "rtu-over-tcp-server",

diff --git a/src/lib.rs b/src/lib.rs
@@ -42,7 +42,7 @@ pub use self::slave::{Slave, SlaveId};
 mod codec;
 
 mod frame;
-pub use self::frame::{Address, FunctionCode, Quantity, Request, Response};
+pub use self::frame::{Address, Exception, FunctionCode, Quantity, Request, Response};
 
 mod service;
 

diff --git a/src/prelude.rs b/src/prelude.rs
@@ -29,7 +29,7 @@ pub mod sync {
 ///////////////////////////////////////////////////////////////////
 /// Types
 ///////////////////////////////////////////////////////////////////
-pub use crate::{Request, Response};
+pub use crate::{Exception, Request, Response};
 pub use crate::{Slave, SlaveId};
 
 #[cfg(feature = "server")]

diff --git a/src/server/rtu.rs b/src/server/rtu.rs
@@ -13,7 +13,7 @@ use crate::{
     codec::rtu::ServerCodec,
     frame::{
         rtu::{RequestAdu, ResponseAdu},
-        OptionalResponsePdu,
+        ExceptionResponse, OptionalResponsePdu,
     },
     server::service::Service,
 };
@@ -44,8 +44,6 @@ impl Server {
     where
         S: Service + Send + Sync + 'static,
         S::Request: From<RequestAdu<'static>> + Send,
-        S::Response: Into<OptionalResponsePdu> + Send,
-        S::Error: Into<io::Error>,
     {
         let framed = Framed::new(self.serial, ServerCodec::default());
         process(framed, service).await
@@ -59,8 +57,6 @@ impl Server {
     where
         S: Service + Send + Sync + 'static,
         S::Request: From<RequestAdu<'static>> + Send,
-        S::Response: Into<OptionalResponsePdu> + Send,
-        S::Error: Into<io::Error>,
         X: Future<Output = ()> + Sync + Send + Unpin + 'static,
     {
         let framed = Framed::new(self.serial, ServerCodec::default());
@@ -77,27 +73,29 @@ impl Server {
 }
 
 /// frame wrapper around the underlying service's responses to forwarded requests
-async fn process<S, Req, Res>(
+async fn process<S, Req>(
     mut framed: Framed<SerialStream, ServerCodec>,
     service: S,
 ) -> io::Result<()>
 where
-    S: Service<Request = Req, Response = Res> + Send + Sync + 'static,
+    S: Service<Request = Req> + Send + Sync + 'static,
     S::Request: From<RequestAdu<'static>> + Send,
-    S::Response: Into<OptionalResponsePdu> + Send,
-    S::Error: Into<io::Error>,
 {
     loop {
         let Some(request) = framed.next().await.transpose()? else {
             log::debug!("Stream has finished");
             break;
         };
 
+        let fc = request.pdu.0.function_code();
         let hdr = request.hdr;
         let OptionalResponsePdu(Some(response_pdu)) = service
             .call(request.into())
             .await
-            .map_err(Into::into)?
+            .map_err(|e| ExceptionResponse {
+                function: fc,
+                exception: e,
+            })
             .into()
         else {
             log::debug!("Sending no response for request {hdr:?}");