Skip to content

Commit 9fa0c6e

Browse files
refactor(doip): address PR review comments and Improvement
- Consolidate errors into DoipError; add TryFrom<u8> on all enums - Strongly type activation_type field (ActivationType, not u8) - Add DoipParseable / DoipSerializable traits + DoipPayload dispatch enum - Override serialized_len() on all impls; fix array size magic numbers - Make ParseError pub(crate); add serde(default), tracing
1 parent 1caf4c0 commit 9fa0c6e

16 files changed

Lines changed: 3000 additions & 1157 deletions

Cargo.lock

Lines changed: 877 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,24 @@ tokio-util = { version = "0.7", features = ["codec"] }
1010

1111
# Serialization
1212
serde = { version = "1.0", features = ["derive"] }
13-
toml = "0.9.11+spec-1.1.0"
13+
toml = "0.9"
1414

1515
# Logging
1616
tracing = "0.1"
1717
tracing-subscriber = { version = "0.3", features = ["env-filter"] }
1818

1919
# Error handling
20-
thiserror = "2.0.18"
20+
thiserror = "2.0"
2121
anyhow = "1.0"
2222

2323
# Data structures
24-
bytes = "1.10.1"
25-
dashmap = "6.1.0"
24+
bytes = "1.0"
25+
dashmap = "6.0"
2626
parking_lot = "0.12"
2727

2828
# Utilities
2929
uuid = { version = "1.6", features = ["v4"] }
30-
clap = { version = "4.4", features = ["derive"] }
30+
clap = { version = "4.4", features = ["derive"] }
31+
32+
[features]
33+
test-handlers = []

src/doip/alive_check.rs

Lines changed: 51 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,39 @@
1+
/*
2+
* Copyright (c) 2026 The Contributors to Eclipse OpenSOVD (see CONTRIBUTORS)
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information regarding copyright ownership.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Apache License Version 2.0 which is available at
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* SPDX-License-Identifier: Apache-2.0
12+
*/
113
//! Alive Check handlers (ISO 13400-2)
214
3-
use bytes::{BufMut, Bytes, BytesMut};
4-
5-
#[derive(Debug, Clone, PartialEq, Eq)]
6-
pub enum Error {
7-
PayloadTooShort { expected: usize, actual: usize },
8-
}
9-
10-
impl std::fmt::Display for Error {
11-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
12-
match self {
13-
Self::PayloadTooShort { expected, actual } => {
14-
write!(f, "payload too short: need {} bytes, got {}", expected, actual)
15-
}
16-
}
17-
}
18-
}
19-
20-
impl std::error::Error for Error {}
15+
use bytes::{BufMut, BytesMut};
16+
use tracing::warn;
17+
use crate::DoipError;
18+
use super::{DoipParseable, DoipSerializable};
2119

2220
// Alive Check Request (0x0007) - no payload
2321
// Server sends this to check if tester is still connected
2422
#[derive(Debug, Clone, PartialEq, Eq, Default)]
2523
pub struct Request;
2624

27-
impl Request {
28-
pub fn parse(_payload: &[u8]) -> Result<Self, Error> {
25+
impl DoipParseable for Request {
26+
fn parse(_payload: &[u8]) -> std::result::Result<Self, DoipError> {
2927
Ok(Self)
3028
}
29+
}
3130

32-
pub fn to_bytes(&self) -> Bytes {
33-
Bytes::new()
31+
impl DoipSerializable for Request {
32+
fn serialized_len(&self) -> Option<usize> {
33+
Some(0)
3434
}
35+
36+
fn write_to(&self, _buf: &mut BytesMut) {}
3537
}
3638

3739
// Alive Check Response (0x0008) - 2 byte source address
@@ -41,39 +43,45 @@ pub struct Response {
4143
pub source_address: u16,
4244
}
4345

44-
impl Response {
45-
pub const LEN: usize = 2;
46-
47-
pub fn new(source_address: u16) -> Self {
48-
Self { source_address }
49-
}
50-
51-
pub fn parse(payload: &[u8]) -> Result<Self, Error> {
52-
if payload.len() < Self::LEN {
53-
return Err(Error::PayloadTooShort {
54-
expected: Self::LEN,
55-
actual: payload.len(),
56-
});
57-
}
58-
59-
let source_address = u16::from_be_bytes([payload[0], payload[1]]);
46+
impl DoipParseable for Response {
47+
fn parse(payload: &[u8]) -> std::result::Result<Self, DoipError> {
48+
let bytes: [u8; 2] = payload
49+
.get(..Self::LEN)
50+
.and_then(|s| s.try_into().ok())
51+
.ok_or_else(|| {
52+
let e = DoipError::PayloadTooShort { expected: Self::LEN, actual: payload.len() };
53+
warn!("AliveCheck Response parse failed: {}", e);
54+
e
55+
})?;
56+
57+
let source_address = u16::from_be_bytes(bytes);
6058
Ok(Self { source_address })
6159
}
60+
}
6261

63-
pub fn to_bytes(&self) -> Bytes {
64-
let mut buf = BytesMut::with_capacity(Self::LEN);
65-
self.write_to(&mut buf);
66-
buf.freeze()
62+
impl DoipSerializable for Response {
63+
fn serialized_len(&self) -> Option<usize> {
64+
Some(Self::LEN)
6765
}
6866

69-
pub fn write_to(&self, buf: &mut BytesMut) {
67+
fn write_to(&self, buf: &mut BytesMut) {
7068
buf.put_u16(self.source_address);
7169
}
7270
}
7371

72+
impl Response {
73+
pub const LEN: usize = 2;
74+
75+
#[must_use]
76+
pub fn new(source_address: u16) -> Self {
77+
Self { source_address }
78+
}
79+
}
80+
7481
#[cfg(test)]
7582
mod tests {
7683
use super::*;
84+
use crate::doip::{DoipParseable, DoipSerializable};
7785

7886
#[test]
7987
fn parse_request() {

0 commit comments

Comments
 (0)