From eb3bea659ccf20926ab0531aa33b2a8477f4499a Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Thu, 2 Oct 2025 16:43:23 -0700 Subject: [PATCH 1/2] feat: Add DirectWriteAbsTime time sync procedure for non-conformant outstations Add support for time synchronization via direct write of Group 50 Var 1 without delay measurement. This is a workaround for outstations that do not support the DELAY_MEASURE function code. This method does not account for network latency and should only be used when standard time synchronization procedures are not supported. Closes #374 --- dnp3/src/master/request.rs | 8 +++ dnp3/src/master/tasks/time.rs | 74 ++++++++++++++++++++++++++++ ffi/dnp3-ffi/src/master/functions.rs | 2 + ffi/dnp3-schema/src/master/mod.rs | 8 +++ 4 files changed, 92 insertions(+) diff --git a/dnp3/src/master/request.rs b/dnp3/src/master/request.rs index a9383f0f..f491db1a 100644 --- a/dnp3/src/master/request.rs +++ b/dnp3/src/master/request.rs @@ -34,11 +34,19 @@ pub enum CommandMode { feature = "serialization", derive(serde::Serialize, serde::Deserialize) )] +#[non_exhaustive] pub enum TimeSyncProcedure { /// Master will use the LAN procedure: RECORD_CURRENT_TIME followed by WRITE g50v3 Lan, /// Master will use the non-LAN procedure: DELAY_MEASUREMENT followed by WRITE g50v1 NonLan, + /// Master will directly WRITE g50v1 (absolute time) without delay measurement + /// + /// **WARNING**: This is a non-conformant workaround for outstations that do not support + /// the DELAY_MEASURE function code. This method does NOT account for network latency + /// and may result in inaccurate time synchronization. Use only when the outstation + /// does not support standard time synchronization procedures. + DirectWriteAbsTime, } /// struct recording which event classes are enabled diff --git a/dnp3/src/master/tasks/time.rs b/dnp3/src/master/tasks/time.rs index 93618e27..f3f45e67 100644 --- a/dnp3/src/master/tasks/time.rs +++ b/dnp3/src/master/tasks/time.rs @@ -19,6 +19,7 @@ enum State { WriteAbsoluteTime(Timestamp), RecordCurrentTime(Option), WriteLastRecordedTime(Timestamp), + DirectWriteAbsTime, } pub(crate) struct TimeSyncTask { @@ -37,6 +38,7 @@ impl TimeSyncProcedure { match self { TimeSyncProcedure::Lan => State::RecordCurrentTime(None), TimeSyncProcedure::NonLan => State::MeasureDelay(None), + TimeSyncProcedure::DirectWriteAbsTime => State::DirectWriteAbsTime, } } } @@ -87,6 +89,16 @@ impl TimeSyncTask { } } State::WriteLastRecordedTime(_) => Some(self), + State::DirectWriteAbsTime => match association.get_system_time() { + Some(timestamp) => { + self.state = State::WriteAbsoluteTime(timestamp); + Some(self) + } + None => { + self.report_error(association, TimeSyncError::SystemTimeNotAvailable); + None + } + }, } } @@ -96,6 +108,7 @@ impl TimeSyncTask { State::WriteAbsoluteTime(_) => FunctionCode::Write, State::RecordCurrentTime(_) => FunctionCode::RecordCurrentTime, State::WriteLastRecordedTime(_) => FunctionCode::Write, + State::DirectWriteAbsTime => FunctionCode::Write, } } @@ -105,6 +118,7 @@ impl TimeSyncTask { State::WriteAbsoluteTime(x) => writer.write_count_of_one(Group50Var1 { time: x }), State::RecordCurrentTime(_) => Ok(()), State::WriteLastRecordedTime(x) => writer.write_count_of_one(Group50Var3 { time: x }), + State::DirectWriteAbsTime => Ok(()), // Should never be reached as it transitions in start() } } @@ -133,6 +147,10 @@ impl TimeSyncTask { State::WriteLastRecordedTime(_) => { self.handle_write_last_recorded_time(association, response) } + State::DirectWriteAbsTime => { + // Should never be reached as it transitions to WriteAbsoluteTime in start() + self.handle_write_absolute_time(association, response) + } } } @@ -969,4 +987,60 @@ mod tests { task } } + + mod direct_write_abs_time { + use super::*; + + #[test] + fn transitions_to_write_absolute_time() { + let (task, system_time, mut association, _rx) = direct_write_setup(); + let task = task.start(&mut association).unwrap(); + + // Should have transitioned to WriteAbsoluteTime + assert_eq!(task.function(), FunctionCode::Write); + + // Verify it writes g50v1 with the system time + let mut buffer = [0; 20]; + let mut cursor = WriteCursor::new(&mut buffer); + let mut writer = start_request( + ControlField::request(Sequence::default()), + task.function(), + &mut cursor, + ) + .unwrap(); + task.write(&mut writer).unwrap(); + let request = writer.to_parsed().to_request().unwrap(); + + let header = request.objects.unwrap().get_only_header().unwrap(); + match header.details.count().unwrap() { + CountVariation::Group50Var1(seq) => { + assert_eq!(seq.single().unwrap().time, system_time); + } + _ => panic!("expected g50v1, not g50v3"), + } + } + + #[test] + fn no_system_time_available() { + let (task, _system_time, mut association, mut rx) = direct_write_setup(); + association.get_system_time(); // Empty the time + + assert!(task.start(&mut association).is_none()); + assert_eq!( + rx.try_recv().unwrap(), + Err(TimeSyncError::SystemTimeNotAvailable) + ); + } + + fn direct_write_setup() -> ( + NonReadTask, + Timestamp, + Association, + tokio::sync::oneshot::Receiver>, + ) { + time_sync_setup(TimeSyncProcedure::DirectWriteAbsTime, |time| { + Box::new(SingleTimestampTestHandler::new(time)) + }) + } + } } diff --git a/ffi/dnp3-ffi/src/master/functions.rs b/ffi/dnp3-ffi/src/master/functions.rs index acce3ddb..16f415d0 100644 --- a/ffi/dnp3-ffi/src/master/functions.rs +++ b/ffi/dnp3-ffi/src/master/functions.rs @@ -1047,6 +1047,7 @@ fn convert_auto_time_sync(config: &ffi::AutoTimeSync) -> Option None, ffi::AutoTimeSync::Lan => Some(TimeSyncProcedure::Lan), ffi::AutoTimeSync::NonLan => Some(TimeSyncProcedure::NonLan), + ffi::AutoTimeSync::DirectWriteAbsTime => Some(TimeSyncProcedure::DirectWriteAbsTime), } } @@ -1218,6 +1219,7 @@ impl From for TimeSyncProcedure { match x { ffi::TimeSyncMode::Lan => Self::Lan, ffi::TimeSyncMode::NonLan => Self::NonLan, + ffi::TimeSyncMode::DirectWriteAbsTime => Self::DirectWriteAbsTime, } } } diff --git a/ffi/dnp3-schema/src/master/mod.rs b/ffi/dnp3-schema/src/master/mod.rs index 466a073f..b8af709e 100644 --- a/ffi/dnp3-schema/src/master/mod.rs +++ b/ffi/dnp3-schema/src/master/mod.rs @@ -861,6 +861,10 @@ fn define_association_config( "non_lan", "Perform automatic time sync with Delay Measurement (0x17) function code", )? + .push( + "direct_write_abs_time", + "Directly write absolute time (g50v1) without delay measurement. WARNING: This is a non-conformant workaround for outstations that do not support DELAY_MEASURE. Does not account for network latency.", + )? .doc("Automatic time synchronization configuration")? .build()?; @@ -1492,6 +1496,10 @@ fn define_time_sync_mode(lib: &mut LibraryBuilder) -> BackTraced { "non_lan", "Perform a non-LAN time sync with Delay Measurement (0x17) function code", )? + .push( + "direct_write_abs_time", + "Directly write absolute time (g50v1) without delay measurement. WARNING: This is a non-conformant workaround for outstations that do not support DELAY_MEASURE. Does not account for network latency.", + )? .doc("Time synchronization mode")? .build()?; From bb93fbda148ece24f7637e31134a30175dda1bd7 Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Thu, 2 Oct 2025 16:59:38 -0700 Subject: [PATCH 2/2] refactor: Use Option instead of separate DirectWriteAbsTime state Eliminates the DirectWriteAbsTime state and all 'unreachable' code paths by using WriteAbsoluteTime(Option) instead. This makes the invariant more local: timestamp must be populated before write() is called, which is enforced by start(). - WriteAbsoluteTime(None) signals timestamp should be fetched in start() - WriteAbsoluteTime(Some(timestamp)) signals timestamp is ready - write() uses expect() with clear error message if invariant is violated --- dnp3/src/master/tasks/time.rs | 42 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/dnp3/src/master/tasks/time.rs b/dnp3/src/master/tasks/time.rs index f3f45e67..66292d27 100644 --- a/dnp3/src/master/tasks/time.rs +++ b/dnp3/src/master/tasks/time.rs @@ -16,10 +16,9 @@ use tokio::time::Instant; enum State { MeasureDelay(Option), - WriteAbsoluteTime(Timestamp), + WriteAbsoluteTime(Option), RecordCurrentTime(Option), WriteLastRecordedTime(Timestamp), - DirectWriteAbsTime, } pub(crate) struct TimeSyncTask { @@ -38,7 +37,7 @@ impl TimeSyncProcedure { match self { TimeSyncProcedure::Lan => State::RecordCurrentTime(None), TimeSyncProcedure::NonLan => State::MeasureDelay(None), - TimeSyncProcedure::DirectWriteAbsTime => State::DirectWriteAbsTime, + TimeSyncProcedure::DirectWriteAbsTime => State::WriteAbsoluteTime(None), } } } @@ -76,7 +75,20 @@ impl TimeSyncTask { } } } - State::WriteAbsoluteTime(_) => Some(self), + State::WriteAbsoluteTime(time) => { + if time.is_none() { + *time = association.get_system_time(); + match time { + Some(_) => Some(self), + None => { + self.report_error(association, TimeSyncError::SystemTimeNotAvailable); + None + } + } + } else { + Some(self) + } + } State::RecordCurrentTime(time) => { *time = association.get_system_time(); @@ -89,16 +101,6 @@ impl TimeSyncTask { } } State::WriteLastRecordedTime(_) => Some(self), - State::DirectWriteAbsTime => match association.get_system_time() { - Some(timestamp) => { - self.state = State::WriteAbsoluteTime(timestamp); - Some(self) - } - None => { - self.report_error(association, TimeSyncError::SystemTimeNotAvailable); - None - } - }, } } @@ -108,17 +110,17 @@ impl TimeSyncTask { State::WriteAbsoluteTime(_) => FunctionCode::Write, State::RecordCurrentTime(_) => FunctionCode::RecordCurrentTime, State::WriteLastRecordedTime(_) => FunctionCode::Write, - State::DirectWriteAbsTime => FunctionCode::Write, } } pub(crate) fn write(&self, writer: &mut HeaderWriter) -> Result<(), scursor::WriteError> { match self.state { State::MeasureDelay(_) => Ok(()), - State::WriteAbsoluteTime(x) => writer.write_count_of_one(Group50Var1 { time: x }), + State::WriteAbsoluteTime(x) => writer.write_count_of_one(Group50Var1 { + time: x.expect("WriteAbsoluteTime timestamp must be set by start()"), + }), State::RecordCurrentTime(_) => Ok(()), State::WriteLastRecordedTime(x) => writer.write_count_of_one(Group50Var3 { time: x }), - State::DirectWriteAbsTime => Ok(()), // Should never be reached as it transitions in start() } } @@ -147,10 +149,6 @@ impl TimeSyncTask { State::WriteLastRecordedTime(_) => { self.handle_write_last_recorded_time(association, response) } - State::DirectWriteAbsTime => { - // Should never be reached as it transitions to WriteAbsoluteTime in start() - self.handle_write_absolute_time(association, response) - } } } @@ -228,7 +226,7 @@ impl TimeSyncTask { }; Ok(Some( - self.change_state(State::WriteAbsoluteTime(timestamp)) + self.change_state(State::WriteAbsoluteTime(Some(timestamp))) .wrap(), )) }