diff --git a/samples/demo-server/src/machine.rs b/samples/demo-server/src/machine.rs index 72aeceb17..99c06589c 100644 --- a/samples/demo-server/src/machine.rs +++ b/samples/demo-server/src/machine.rs @@ -84,10 +84,10 @@ fn add_machine(address_space: &mut AddressSpace, folder_id: NodeId, name: &str, VariableBuilder::new(&counter_id, "Counter", "Counter") .property_of(machine_id.clone()) .has_type_definition(VariableTypeId::PropertyType) - .value_getter(move |_, _, _, _, _| -> Result, StatusCode> { + .value_getter(AttrFnGetter::new_boxed(move |_, _, _, _, _| -> Result, StatusCode> { let value = counter.load(Ordering::Relaxed); Ok(Some(DataValue::new(value))) - }) + })) .insert(address_space); machine_id diff --git a/samples/modbus-server/modbus.conf b/samples/modbus-server/modbus.conf index 131cf34be..7f89055ff 100644 --- a/samples/modbus-server/modbus.conf +++ b/samples/modbus-server/modbus.conf @@ -1,9 +1,9 @@ --- slave_address: "127.0.0.1:502" -read_interval: 1000 +read_interval: 100 output_coils: base_address: 0 - count: 2 + count: 30 input_coils: base_address: 0 count: 20 diff --git a/samples/modbus-server/src/config.rs b/samples/modbus-server/src/config.rs index e10a27745..1bade02ba 100644 --- a/samples/modbus-server/src/config.rs +++ b/samples/modbus-server/src/config.rs @@ -4,6 +4,8 @@ use std::{ path::Path, }; +use opcua_server::prelude::*; + use crate::Table; #[derive(Deserialize, Clone, Copy, PartialEq)] @@ -22,6 +24,42 @@ pub enum AliasType { Double, } +impl Into for AliasType { + fn into(self) -> DataTypeId { + match self { + Self::Boolean => DataTypeId::Boolean, + Self::Byte => DataTypeId::Byte, + Self::SByte => DataTypeId::SByte, + Self::UInt16 | Self::Default => DataTypeId::UInt16, + Self::Int16 => DataTypeId::Int16, + Self::UInt32 => DataTypeId::UInt32, + Self::Int32 => DataTypeId::Int32, + Self::UInt64 => DataTypeId::UInt64, + Self::Int64 => DataTypeId::Int64, + Self::Float => DataTypeId::Float, + Self::Double => DataTypeId::Double, + } + } +} + +impl Into for AliasType { + fn into(self) -> VariantTypeId { + match self { + Self::Boolean => VariantTypeId::Boolean, + Self::Byte => VariantTypeId::Byte, + Self::SByte => VariantTypeId::SByte, + Self::UInt16 | AliasType::Default => VariantTypeId::UInt16, + Self::Int16 => VariantTypeId::Int16, + Self::UInt32 => VariantTypeId::UInt32, + Self::Int32 => VariantTypeId::Int32, + Self::UInt64 => VariantTypeId::UInt64, + Self::Int64 => VariantTypeId::Int64, + Self::Float => VariantTypeId::Float, + Self::Double => VariantTypeId::Double, + } + } +} + impl AliasType { /// Returns the size of the type in number of registers pub fn size_in_words(&self) -> u16 { diff --git a/samples/modbus-server/src/main.rs b/samples/modbus-server/src/main.rs index 794e73b28..b29b76b27 100644 --- a/samples/modbus-server/src/main.rs +++ b/samples/modbus-server/src/main.rs @@ -87,6 +87,8 @@ fn main() { std::process::exit(1); }; + opcua_console_logging::init(); + run(config, m.is_present("run-demo-slave")); } diff --git a/samples/modbus-server/src/opcua.rs b/samples/modbus-server/src/opcua.rs index 569c14d67..5fa180b97 100644 --- a/samples/modbus-server/src/opcua.rs +++ b/samples/modbus-server/src/opcua.rs @@ -135,27 +135,28 @@ fn add_aliases(runtime: &Arc>, modbus: &Arc>, addr .add_folder("Aliases", "Aliases", parent_folder_id) .unwrap(); - let variables = aliases.into_iter().map(move |alias| { - // Alias node ids are just their name in the list + // Create variables for all of the aliases + aliases.into_iter().for_each(move |alias| { + // Create a getter/setter + let getter_setter = Arc::new(Mutex::new(AliasGetterSetter::new(runtime.clone(), modbus.clone(), alias.clone()))); + // Create a variable for the alias let node_id = NodeId::new(nsidx, alias.name.clone()); - let mut v = Variable::new(&node_id, &alias.name, &alias.name, 0u16); - - let writable = alias.writable; - - // Set the reader - let getter_setter = Arc::new(Mutex::new(AliasGetterSetter::new(runtime.clone(), modbus.clone(), alias))); - v.set_value_getter(getter_setter.clone()); - - // Writable aliases need write permission - if writable { - v.set_access_level(AccessLevel::CURRENT_READ | AccessLevel::CURRENT_WRITE); - v.set_user_access_level(UserAccessLevel::CURRENT_READ | UserAccessLevel::CURRENT_WRITE); - v.set_value_setter(getter_setter); - } - - v - }).collect(); - let _ = address_space.add_variables(variables, &parent_folder_id); + let data_type: DataTypeId = alias.data_type.into(); + let v = VariableBuilder::new(&node_id, &alias.name, &alias.name) + .organized_by(&parent_folder_id) + .data_type(data_type) + .value(0u16) + .value_getter(getter_setter.clone()); + + let v = if alias.writable { + v.value_setter(getter_setter) + .writable() + } else { + v + }; + + v.insert(address_space); + }); } } @@ -164,62 +165,66 @@ fn make_variables(modbus: &Arc>, address_space: &mut AddressSpa where T: 'static + Copy + Send + Sync + Into { // Create variables - let variables = (start..end).for_each(|i| { + (start..end).for_each(|i| { let addr = i as u16; - let name = name_formatter(i); - - let mut v = VariableBuilder::new(&make_node_id(nsidx, table, addr), &name, &name) - .value(default_value); - let values = values.clone(); - v = v.value_getter(move |_node_id, _attribute_id, _numeric_range, _name, _f| -> Result, StatusCode> { - let values = values.read().unwrap(); - let value = *values.get(i - start).unwrap(); - Ok(Some(DataValue::new(value))) - }); + let v = VariableBuilder::new(&make_node_id(nsidx, table, addr), &name, &name) + .organized_by(parent_folder_id) + .value(default_value) + .value_getter(AttrFnGetter::new_boxed(move |_node_id, _attribute_id, _numeric_range, _name, _f| -> Result, StatusCode> { + let values = values.read().unwrap(); + let value = *values.get(i - start).unwrap(); + Ok(Some(DataValue::new(value))) + })); // Output tables have setters too let v = match table { + Table::InputCoils => { + v.data_type(DataTypeId::Boolean) + } Table::OutputCoils => { let modbus = modbus.clone(); - v.value_setter(move |_node_id: &NodeId, _attribute_id: AttributeId, value: DataValue| -> Result<(), StatusCode> { - // Try to cast to a bool - let value = if let Some(value) = value.value { - value.cast(VariantTypeId::Boolean) - } else { - Variant::Empty - }; - if let Variant::Boolean(value) = value { - let modbus = modbus.lock().unwrap(); - modbus.write_to_coil(addr, value); - Ok(()) - } else { - Err(StatusCode::BadTypeMismatch) - } - }).writable() + v.data_type(DataTypeId::Boolean) + .value_setter(AttrFnSetter::new_boxed(move |_node_id, _attribute_id, value| { + // Try to cast to a bool + let value = if let Some(value) = value.value { + value.cast(VariantTypeId::Boolean) + } else { + Variant::Empty + }; + if let Variant::Boolean(value) = value { + let modbus = modbus.lock().unwrap(); + modbus.write_to_coil(addr, value); + Ok(()) + } else { + Err(StatusCode::BadTypeMismatch) + } + })).writable() + } + Table::InputRegisters => { + v.data_type(DataTypeId::UInt16) } Table::OutputRegisters => { let modbus = modbus.clone(); - v.value_setter(move |_node_id: &NodeId, _attribute_id: AttributeId, value: DataValue| -> Result<(), StatusCode> { // Try to cast to a u16 - let value = if let Some(value) = value.value { - value.cast(VariantTypeId::UInt16) - } else { - Variant::Empty - }; - if let Variant::UInt16(value) = value { - let modbus = modbus.lock().unwrap(); - modbus.write_to_register(addr, value); - Ok(()) - } else { - Err(StatusCode::BadTypeMismatch) - } - }).writable() + v.data_type(DataTypeId::UInt16) + .value_setter(AttrFnSetter::new_boxed(move |_node_id, _attribute_id, value| { + let value = if let Some(value) = value.value { + value.cast(VariantTypeId::UInt16) + } else { + Variant::Empty + }; + if let Variant::UInt16(value) = value { + let modbus = modbus.lock().unwrap(); + modbus.write_to_register(addr, value); + Ok(()) + } else { + Err(StatusCode::BadTypeMismatch) + } + })).writable() } - _ => v }; - v.organized_by(parent_folder_id) - .insert(address_space); + v.insert(address_space); }); } @@ -290,19 +295,7 @@ impl AliasGetterSetter { } Table::OutputRegisters => { // Cast to the alias' expected type - let variant_type = match data_type { - AliasType::Boolean => VariantTypeId::Boolean, - AliasType::Byte => VariantTypeId::Byte, - AliasType::SByte => VariantTypeId::SByte, - AliasType::UInt16 | AliasType::Default => VariantTypeId::UInt16, - AliasType::Int16 => VariantTypeId::Int16, - AliasType::UInt32 => VariantTypeId::UInt32, - AliasType::Int32 => VariantTypeId::Int32, - AliasType::UInt64 => VariantTypeId::UInt64, - AliasType::Int64 => VariantTypeId::Int64, - AliasType::Float => VariantTypeId::Float, - AliasType::Double => VariantTypeId::Double, - }; + let variant_type: VariantTypeId = data_type.into(); let value = value.cast(variant_type); // Write the words let (_, words) = Self::value_to_words(value).map_err(|_| StatusCode::BadUnexpectedError)?; diff --git a/server/src/address_space/mod.rs b/server/src/address_space/mod.rs index 1d4133e86..78a780539 100644 --- a/server/src/address_space/mod.rs +++ b/server/src/address_space/mod.rs @@ -1,7 +1,10 @@ //! Provides functionality to create an address space, find nodes, add nodes, change attributes //! and values on nodes. -use std::result::Result; +use std::{ + result::Result, + sync::{Arc, Mutex}, +}; use opcua_types::{AttributeId, DataValue, NodeId, NumericRange, QualifiedName}; use opcua_types::status_code::StatusCode; @@ -23,6 +26,10 @@ impl AttributeGetter for AttrFnGetter where F: FnMut(&NodeId, AttributeId, impl AttrFnGetter where F: FnMut(&NodeId, AttributeId, NumericRange, &QualifiedName, f64) -> Result, StatusCode> + Send { pub fn new(getter: F) -> AttrFnGetter { AttrFnGetter { getter } } + + pub fn new_boxed(getter: F) -> Arc>> { + Arc::new(Mutex::new(Self::new(getter))) + } } /// An implementation of attribute setter that can be easily constructed using a mutable function @@ -38,6 +45,10 @@ impl AttributeSetter for AttrFnSetter where F: FnMut(&NodeId, AttributeId, impl AttrFnSetter where F: FnMut(&NodeId, AttributeId, DataValue) -> Result<(), StatusCode> + Send { pub fn new(setter: F) -> AttrFnSetter { AttrFnSetter { setter } } + + pub fn new_boxed(setter: F) -> Arc>> { + Arc::new(Mutex::new(Self::new(setter))) + } } // A macro for creating builders. Builders can be used for more conveniently creating objects, diff --git a/server/src/address_space/variable.rs b/server/src/address_space/variable.rs index c5529baea..33b6243e4 100644 --- a/server/src/address_space/variable.rs +++ b/server/src/address_space/variable.rs @@ -8,8 +8,7 @@ use opcua_types::service_types::VariableAttributes; use crate::{ address_space::{ - AccessLevel, AttrFnGetter, - AttrFnSetter, base::Base, + AccessLevel, base::Base, node::{Node, NodeBase}, UserAccessLevel, }, @@ -93,22 +92,18 @@ impl VariableBuilder { } /// Sets a value getter function for the variable. Whenever the value of a variable - /// needs to be fetched (e.g. from a monitored item subscription), this function will be called + /// needs to be fetched (e.g. from a monitored item subscription), this trait will be called /// to get the value. - pub fn value_getter(mut self, getter: F) -> Self where - F: FnMut(&NodeId, AttributeId, NumericRange, &QualifiedName, f64) -> Result, StatusCode> + Send + 'static - { - self.node.set_value_getter(Arc::new(Mutex::new(AttrFnGetter::new(getter)))); + pub fn value_getter(mut self, getter: Arc>) -> Self { + self.node.set_value_getter(getter); self } /// Sets a value setter function for the variable. Whenever the value of a variable is set via - /// a service, this function will be called to set the value. It is up to the implementation + /// a service, this trait will be called to set the value. It is up to the implementation /// to decide what to do if that happens. - pub fn value_setter(mut self, setter: F) -> Self where - F: FnMut(&NodeId, AttributeId, DataValue) -> Result<(), StatusCode> + Send + 'static - { - self.node.set_value_setter(Arc::new(Mutex::new(AttrFnSetter::new(setter)))); + pub fn value_setter(mut self, setter: Arc>) -> Self { + self.node.set_value_setter(setter); self } diff --git a/server/src/services/attribute.rs b/server/src/services/attribute.rs index 6ebe0d63f..95825b908 100644 --- a/server/src/services/attribute.rs +++ b/server/src/services/attribute.rs @@ -106,6 +106,7 @@ impl AttributeService { /// elements or to write ranges of elements of the composite. pub fn write(&self, _server_state: Arc>, session: Arc>, address_space: Arc>, request: &WriteRequest) -> SupportedMessage { if is_empty_option_vec!(request.nodes_to_write) { + debug!("Empty list passed to write {:?}", request); self.service_fault(&request.request_header, StatusCode::BadNothingToDo) } else { let session = trace_read_lock_unwrap!(session); diff --git a/server/src/services/mod.rs b/server/src/services/mod.rs index e06b451d2..320d90306 100644 --- a/server/src/services/mod.rs +++ b/server/src/services/mod.rs @@ -7,7 +7,7 @@ trait Service { fn name(&self) -> String; fn service_fault(&self, request_header: &RequestHeader, service_result: StatusCode) -> SupportedMessage { - warn!("Service {}, request {} generated a service fault with status code {}", self.name(), request_header.request_handle, service_result); + warn!("Service {}, request handle {} generated a service fault with status code {}", self.name(), request_header.request_handle, service_result); ServiceFault::new_supported_message(request_header, service_result) } } diff --git a/server/src/tests/services/subscription.rs b/server/src/tests/services/subscription.rs index 827752063..3e86ac4e4 100644 --- a/server/src/tests/services/subscription.rs +++ b/server/src/tests/services/subscription.rs @@ -40,15 +40,6 @@ fn publish_request(subscription_acknowledgements: Option RepublishRequest { - let request = RepublishRequest { - request_header: RequestHeader::dummy(), - subscription_id, - retransmit_sequence_number, - }; - debug!("RepublishRequest {:#?}", request); - request -} #[test] fn create_modify_destroy_subscription() { @@ -127,7 +118,6 @@ fn publish_response_subscription() { let notification_message = { let request_id = 1001; let request = publish_request(None); - // Tick subscriptions to trigger a change let _ = ss.async_publish(&now, session.clone(), address_space.clone(), request_id, &request); let now = now.add(chrono::Duration::seconds(2)); @@ -207,7 +197,6 @@ fn publish_keep_alive() { let notification_message = { let request_id = 1001; let request = publish_request(None); - let now = Utc::now(); // Don't expect a response right away @@ -252,16 +241,13 @@ fn multiple_publish_response_subscription() { let subscription_id = create_subscription(server_state, session.clone(), &ss); let now = Utc::now(); - let request_id = 1001; - // Send a publish and expect nothing let request = publish_request(None); let response = ss.async_publish(&now, session.clone(), address_space.clone(), request_id, &request); assert!(response.is_none()); - // TODO Tick a change // TODO Expect a publish response containing the subscription to be pushed //unimplemented!(); @@ -271,7 +257,20 @@ fn multiple_publish_response_subscription() { #[test] fn acknowledge_unknown_sequence_nr() { do_subscription_service_test(|server_state, session, address_space, ss, mis| { - // TODO acknowledge an unknown seqid, test the response + let subscription_id = create_subscription(server_state, session.clone(), &ss); + + let now = Utc::now(); + let request_id = 1001; + + // Acknowledge an unknown seqid, test the response + let ack = SubscriptionAcknowledgement { + subscription_id, + sequence_number: 10001, + }; + let request = publish_request(Some(vec![ack])); + let response = ss.async_publish(&now, session.clone(), address_space.clone(), request_id, &request); + + // TODO //unimplemented!(); }) } @@ -296,19 +295,31 @@ fn republish() { }; // try for a notification message known to exist - let request = republish_request(subscription_id, sequence_number); + let request = RepublishRequest { + request_header: RequestHeader::dummy(), + subscription_id, + retransmit_sequence_number: sequence_number, + }; let response = ss.republish(session.clone(), &request); trace!("republish response {:#?}", response); let response: RepublishResponse = supported_message_as!(response, RepublishResponse); assert!(response.notification_message.sequence_number != 0); // try for a subscription id that does not exist, expect service fault - let request = republish_request(subscription_id + 1, sequence_number); + let request = RepublishRequest { + request_header: RequestHeader::dummy(), + subscription_id: subscription_id + 1, + retransmit_sequence_number: sequence_number, + }; let response: ServiceFault = supported_message_as!(ss.republish(session.clone(), &request), ServiceFault); assert_eq!(response.response_header.service_result, StatusCode::BadSubscriptionIdInvalid); // try for a sequence nr that does not exist - let request = republish_request(subscription_id, sequence_number + 1); + let request = RepublishRequest { + request_header: RequestHeader::dummy(), + subscription_id, + retransmit_sequence_number: sequence_number + 1, + }; let response: ServiceFault = supported_message_as!(ss.republish(session.clone(), &request), ServiceFault); assert_eq!(response.response_header.service_result, StatusCode::BadMessageNotAvailable); })