Skip to content

Commit

Permalink
Fix panic on disconnect() (#261)
Browse files Browse the repository at this point in the history
* Add debug assertions

* Fix panic on disconnect()

* Update changelog
  • Loading branch information
uklotzde authored Jun 24, 2024
1 parent 13fcb83 commit 8b04b2f
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 43 deletions.
12 changes: 10 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@

# Changelog

## v0.13.0 (2024-06-23)
## v0.13.1 (Unreleased)

- Fix: Do not panic on disconnect.

### Breaking Changes

- Add `FunctionCode::Disconnect`.

## v0.13.0 (2024-06-23, yanked)

- Fix: Do not panic on mismatching request/response function codes.

Expand All @@ -12,7 +20,7 @@
- Client: Replace `std::io::Error` with a custom error type that handles both
protocol and transport errors.

## v0.12.0 (2024-03-21)
## v0.12.0 (2024-03-21, yanked)

- Client: Support handling of _Modbus_ exceptions by using nested `Result`s.

Expand Down
5 changes: 4 additions & 1 deletion examples/rtu-client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {

let mut ctx = rtu::attach_slave(port, slave);
println!("Reading a sensor value");
let rsp = ctx.read_holding_registers(0x082B, 2).await?;
let rsp = ctx.read_holding_registers(0x082B, 2).await??;
println!("Sensor value is: {rsp:?}");

println!("Disconnecting");
ctx.disconnect().await??;

Ok(())
}
3 changes: 3 additions & 0 deletions examples/tcp-client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let id = String::from_utf8(bytes).unwrap();
println!("The coupler ID is '{id}'");

println!("Disconnecting");
ctx.disconnect().await??;

Ok(())
}
8 changes: 6 additions & 2 deletions src/codec/rtu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,12 @@ impl<'a> Encoder<RequestAdu<'a>> for ClientCodec {
type Error = Error;

fn encode(&mut self, adu: RequestAdu<'a>, buf: &mut BytesMut) -> Result<()> {
if adu.disconnect {
let RequestAdu {
hdr,
pdu,
disconnect,
} = adu;
if disconnect {
// The disconnect happens implicitly after letting this request
// fail by returning an error. This will drop the attached
// transport, e.g. for closing a stale, exclusive connection
Expand All @@ -339,7 +344,6 @@ impl<'a> Encoder<RequestAdu<'a>> for ClientCodec {
"Disconnecting - not an error",
));
}
let RequestAdu { hdr, pdu, .. } = adu;
let pdu_data: Bytes = pdu.try_into()?;
buf.reserve(pdu_data.len() + 3);
buf.put_u8(hdr.slave_id);
Expand Down
8 changes: 6 additions & 2 deletions src/codec/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,12 @@ impl<'a> Encoder<RequestAdu<'a>> for ClientCodec {
type Error = Error;

fn encode(&mut self, adu: RequestAdu<'a>, buf: &mut BytesMut) -> Result<()> {
if adu.disconnect {
let RequestAdu {
hdr,
pdu,
disconnect,
} = adu;
if disconnect {
// The disconnect happens implicitly after letting this request
// fail by returning an error. This will drop the attached
// transport, e.g. for terminating an open connection.
Expand All @@ -146,7 +151,6 @@ impl<'a> Encoder<RequestAdu<'a>> for ClientCodec {
"Disconnecting - not an error",
));
}
let RequestAdu { hdr, pdu, .. } = adu;
let pdu_data: Bytes = pdu.try_into()?;
buf.reserve(pdu_data.len() + 7);
buf.put_u16(hdr.transaction_id);
Expand Down
51 changes: 29 additions & 22 deletions src/frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,42 +46,49 @@ pub enum FunctionCode {

/// Custom Modbus Function Code.
Custom(u8),

Disconnect,
}

impl FunctionCode {
/// Create a new [`FunctionCode`] with `value`.
#[must_use]
pub const fn new(value: u8) -> Self {
match value {
0x01 => FunctionCode::ReadCoils,
0x02 => FunctionCode::ReadDiscreteInputs,
0x05 => FunctionCode::WriteSingleCoil,
0x06 => FunctionCode::WriteSingleRegister,
0x03 => FunctionCode::ReadHoldingRegisters,
0x04 => FunctionCode::ReadInputRegisters,
0x0F => FunctionCode::WriteMultipleCoils,
0x10 => FunctionCode::WriteMultipleRegisters,
0x16 => FunctionCode::MaskWriteRegister,
0x17 => FunctionCode::ReadWriteMultipleRegisters,
code => FunctionCode::Custom(code),
0x01 => Self::ReadCoils,
0x02 => Self::ReadDiscreteInputs,
0x05 => Self::WriteSingleCoil,
0x06 => Self::WriteSingleRegister,
0x03 => Self::ReadHoldingRegisters,
0x04 => Self::ReadInputRegisters,
0x0F => Self::WriteMultipleCoils,
0x10 => Self::WriteMultipleRegisters,
0x16 => Self::MaskWriteRegister,
0x17 => Self::ReadWriteMultipleRegisters,
code => Self::Custom(code),
}
}

/// Get the [`u8`] value of the current [`FunctionCode`].
///
/// # Panics
///
/// Panics on [`Disconnect`](Self::Disconnect) which has no corresponding Modbus function code.
#[must_use]
pub const fn value(self) -> u8 {
match self {
FunctionCode::ReadCoils => 0x01,
FunctionCode::ReadDiscreteInputs => 0x02,
FunctionCode::WriteSingleCoil => 0x05,
FunctionCode::WriteSingleRegister => 0x06,
FunctionCode::ReadHoldingRegisters => 0x03,
FunctionCode::ReadInputRegisters => 0x04,
FunctionCode::WriteMultipleCoils => 0x0F,
FunctionCode::WriteMultipleRegisters => 0x10,
FunctionCode::MaskWriteRegister => 0x16,
FunctionCode::ReadWriteMultipleRegisters => 0x17,
FunctionCode::Custom(code) => code,
Self::ReadCoils => 0x01,
Self::ReadDiscreteInputs => 0x02,
Self::WriteSingleCoil => 0x05,
Self::WriteSingleRegister => 0x06,
Self::ReadHoldingRegisters => 0x03,
Self::ReadInputRegisters => 0x04,
Self::WriteMultipleCoils => 0x0F,
Self::WriteMultipleRegisters => 0x10,
Self::MaskWriteRegister => 0x16,
Self::ReadWriteMultipleRegisters => 0x17,
Self::Custom(code) => code,
Self::Disconnect => unreachable!(),
}
}
}
Expand Down
23 changes: 16 additions & 7 deletions src/service/rtu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,27 +48,36 @@ where
}

async fn call(&mut self, req: Request<'_>) -> Result<Response> {
let disconnect = req == Request::Disconnect;
let req_function_code = req.function_code();
let req_adu = self.next_request_adu(req, disconnect);
let req_function_code = if matches!(req, Request::Disconnect) {
None
} else {
Some(req.function_code())
};
let req_adu = self.next_request_adu(req, req_function_code.is_none());
let req_hdr = req_adu.hdr;

self.framed.read_buffer_mut().clear();

self.framed.send(req_adu).await?;

let res_adu = self
.framed
.next()
.await
.unwrap_or_else(|| Err(io::Error::from(io::ErrorKind::BrokenPipe)))?;

let ResponsePdu(result) = res_adu.pdu;
let ResponseAdu {
hdr: res_hdr,
pdu: res_pdu,
} = res_adu;
let ResponsePdu(result) = res_pdu;

// Match headers of request and response.
if let Err(message) = verify_response_header(&req_hdr, &res_adu.hdr) {
if let Err(message) = verify_response_header(&req_hdr, &res_hdr) {
return Err(ProtocolError::HeaderMismatch { message, result }.into());
}

debug_assert!(req_function_code.is_some());
let req_function_code = req_function_code.unwrap();

// Match function codes of request and response.
let rsp_function_code = match &result {
Ok(response) => response.function_code(),
Expand Down
23 changes: 16 additions & 7 deletions src/service/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,27 +71,36 @@ where

pub(crate) async fn call(&mut self, req: Request<'_>) -> Result<Response> {
log::debug!("Call {:?}", req);
let disconnect = req == Request::Disconnect;
let req_function_code = req.function_code();
let req_adu = self.next_request_adu(req, disconnect);
let req_function_code = if matches!(req, Request::Disconnect) {
None
} else {
Some(req.function_code())
};
let req_adu = self.next_request_adu(req, req_function_code.is_none());
let req_hdr = req_adu.hdr;

self.framed.read_buffer_mut().clear();

self.framed.send(req_adu).await?;

let res_adu = self
.framed
.next()
.await
.ok_or_else(io::Error::last_os_error)??;

let ResponsePdu(result) = res_adu.pdu;
let ResponseAdu {
hdr: res_hdr,
pdu: res_pdu,
} = res_adu;
let ResponsePdu(result) = res_pdu;

// Match headers of request and response.
if let Err(message) = verify_response_header(&req_hdr, &res_adu.hdr) {
if let Err(message) = verify_response_header(&req_hdr, &res_hdr) {
return Err(ProtocolError::HeaderMismatch { message, result }.into());
}

debug_assert!(req_function_code.is_some());
let req_function_code = req_function_code.unwrap();

// Match function codes of request and response.
let rsp_function_code = match &result {
Ok(response) => response.function_code(),
Expand Down

0 comments on commit 8b04b2f

Please sign in to comment.