Skip to content

Commit

Permalink
Small refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
flosse committed Mar 19, 2024
1 parent 71a1e55 commit 3d2df15
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 88 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ keywords = ["fieldbus", "modbus", "hardware", "automation"]
homepage = "https://github.com/slowtec/modbus-core"
repository = "https://github.com/slowtec/modbus-core"
edition = "2021"
rust-version = "1.65"

[dependencies]
log = "0.4"
Expand Down
1 change: 0 additions & 1 deletion src/codec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ impl<'r> Encode for Request<'r> {

impl<'r> Encode for Response<'r> {
fn encode(&self, buf: &mut [u8]) -> Result<usize> {

if buf.len() < self.pdu_len() {
return Err(Error::BufferSize);
}
Expand Down
39 changes: 19 additions & 20 deletions src/codec/rtu/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,36 +46,35 @@ pub fn decode(
}
.and_then(|pdu_len| {
retry = false;
if let Some(pdu_len) = pdu_len {
extract_frame(raw_frame, pdu_len).map(|x| {
x.map(|res| {
(
res,
FrameLocation {
start: drop_cnt,
size: pdu_len + 3,
},
)
})
})
} else {
let Some(pdu_len) = pdu_len else {
// Incomplete frame
Ok(None)
}
return Ok(None);
};
extract_frame(raw_frame, pdu_len).map(|x| {
x.map(|res| {
let frame_location = FrameLocation {
start: drop_cnt,
size: pdu_len + 3, // TODO: use 'const FOO:usize = 3;'
};
(res, frame_location)
})
})
})
.or_else(|err| {
let pdu_type = match decoder_type {
Request => "request",
Response => "response",
};
if drop_cnt + 1 >= MAX_FRAME_LEN {
log::error!(
"Giving up to decode frame after dropping {drop_cnt} byte(s): {:X?}",
&buf[0..drop_cnt]
);
return Err(err);
}
log::warn!("Failed to decode {pdu_type} frame: {err}");
log::warn!(
"Failed to decode {} frame: {err}",
match decoder_type {
Request => "request",
Response => "response",
}
);
drop_cnt += 1;
retry = true;
Ok(None)
Expand Down
31 changes: 15 additions & 16 deletions src/codec/rtu/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,21 @@ use super::*;
pub fn decode_request(buf: &[u8]) -> Result<Option<RequestAdu>> {
decode(DecoderType::Request, buf)
.and_then(|frame| {
if let Some((DecodedFrame { slave, pdu }, _frame_pos)) = frame {
let hdr = Header { slave };
// Decoding of the PDU should are unlikely to fail due
// to transmission errors, because the frame's bytes
// have already been verified with the CRC.
Request::try_from(pdu)
.map(RequestPdu)
.map(|pdu| Some(RequestAdu { hdr, pdu }))
.map_err(|err| {
// Unrecoverable error
log::error!("Failed to decode request PDU: {err}");
err
})
} else {
Ok(None)
}
let Some((DecodedFrame { slave, pdu }, _frame_pos)) = frame else {
return Ok(None);
};
let hdr = Header { slave };
// Decoding of the PDU should are unlikely to fail due
// to transmission errors, because the frame's bytes
// have already been verified with the CRC.
Request::try_from(pdu)
.map(RequestPdu)
.map(|pdu| Some(RequestAdu { hdr, pdu }))
.map_err(|err| {
// Unrecoverable error
log::error!("Failed to decode request PDU: {err}");
err
})
})
.map_err(|_| {
// Decoding the transport frame is non-destructive and must
Expand Down
93 changes: 42 additions & 51 deletions src/codec/tcp/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,71 +3,62 @@ use super::*;

/// Decode an TCP request.
pub fn decode_request(buf: &[u8]) -> Result<Option<RequestAdu>> {
decode(DecoderType::Request, buf).and_then(|frame| {
if let Some((
DecodedFrame {
let frame = decode(DecoderType::Request, buf)?;
let Some((decoded_frame, _frame_pos)) = frame else {
return Ok(None);
};
let DecodedFrame {
transaction_id,
unit_id,
pdu,
} = decoded_frame;
let hdr = Header {
transaction_id,
unit_id,
};
// Decoding of the PDU should are unlikely to fail due
// to transmission errors, because the frame's bytes
// have already been verified at the TCP level.
Request::try_from(pdu)
.map(RequestPdu)
.map(|pdu| Some(RequestAdu { hdr, pdu }))
.map_err(|err| {
// Unrecoverable error
log::error!("Failed to decode request PDU: {err}");
err
})
}

// Decode a TCP response
pub fn decode_response(buf: &[u8]) -> Result<Option<ResponseAdu>> {
decode(DecoderType::Response, buf)
.and_then(|frame| {
let Some((decoded_frame, _frame_pos)) = frame else {
return Ok(None);
};
let DecodedFrame {
transaction_id,
unit_id,
pdu,
},
_frame_pos,
)) = frame
{
} = decoded_frame;
let hdr = Header {
transaction_id,
unit_id,
};
// Decoding of the PDU should are unlikely to fail due
// to transmission errors, because the frame's bytes
// have already been verified at the TCP level.
Request::try_from(pdu)
.map(RequestPdu)
.map(|pdu| Some(RequestAdu { hdr, pdu }))

Response::try_from(pdu)
.map(Ok)
.or_else(|_| ExceptionResponse::try_from(pdu).map(Err))
.map(ResponsePdu)
.map(|pdu| Some(ResponseAdu { hdr, pdu }))
.map_err(|err| {
// Unrecoverable error
log::error!("Failed to decode request PDU: {err}");
log::error!("Failed to decode response PDU: {err}");
err
})
} else {
Ok(None)
}
})
}

// Decode a TCP response
pub fn decode_response(buf: &[u8]) -> Result<Option<ResponseAdu>> {
decode(DecoderType::Response, buf)
.and_then(|frame| {
if let Some((
DecodedFrame {
transaction_id,
unit_id,
pdu,
},
_frame_pos,
)) = frame
{
let hdr = Header {
transaction_id,
unit_id,
};
// Decoding of the PDU should are unlikely to fail due
// to transmission errors, because the frame's bytes
// have already been verified at the TCP level.

Response::try_from(pdu)
.map(Ok)
.or_else(|_| ExceptionResponse::try_from(pdu).map(Err))
.map(ResponsePdu)
.map(|pdu| Some(ResponseAdu { hdr, pdu }))
.map_err(|err| {
// Unrecoverable error
log::error!("Failed to decode response PDU: {err}");
err
})
} else {
Ok(None)
}
})
.map_err(|_| {
// Decoding the transport frame is non-destructive and must
Expand Down

0 comments on commit 3d2df15

Please sign in to comment.