Skip to content

Commit

Permalink
Merge pull request #88 from boschglobal/fix/actuate_and_batch_actuate…
Browse files Browse the repository at this point in the history 
…_out_of_range

Fix actuate and batch actuate out of range error
  • Loading branch information
@rafaeling
rafaeling authored Oct 29, 2024
2 parents 95e3041 + 3368c07 commit 3bacf45
Show file tree
Hide file tree
Showing 3 changed files with 305 additions and 1 deletion.
69 changes: 69 additions & 0 deletions databroker/src/broker.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1758,6 +1758,8 @@ impl<'a, 'b> AuthorizedAccess<'a, 'b> {
for actuation_change in &actuation_changes {
let vss_id = actuation_change.id;
self.can_write_actuator_target(&vss_id).await?;
self.validate_actuator_value(&vss_id, &actuation_change.data_value)
.await?;
}

let actuation_changes_per_vss_id = &self
Expand Down Expand Up @@ -1809,6 +1811,7 @@ impl<'a, 'b> AuthorizedAccess<'a, 'b> {
let vss_id = *vss_id;

self.can_write_actuator_target(&vss_id).await?;
self.validate_actuator_value(&vss_id, data_value).await?;

let read_subscription_guard = self.broker.subscriptions.read().await;
let opt_actuation_subscription = &read_subscription_guard
Expand Down Expand Up @@ -1882,6 +1885,72 @@ impl<'a, 'b> AuthorizedAccess<'a, 'b> {
)),
}
}

async fn validate_actuator_value(
&self,
vss_id: &i32,
data_value: &DataValue,
) -> Result<(), (ActuationError, String)> {
let result_entry = self.get_entry_by_id(*vss_id).await;
match result_entry {
Ok(entry) => {
let validation = entry.validate_value(data_value);
let vss_path = entry.metadata.path;
match validation {
Ok(_) => Ok(()),
Err(UpdateError::OutOfBounds) => {
let message = format!(
"Out of bounds value provided for {}: {} | Expected range [min: {}, max: {}]",
vss_path,
data_value,
entry.metadata.min.map_or("None".to_string(), |value| value.to_string()),
entry.metadata.max.map_or("None".to_string(), |value| value.to_string()),
);
Err((ActuationError::OutOfBounds, message))
}
Err(UpdateError::UnsupportedType) => {
let message = format!(
"Unsupported type for vss_path {}. Expected type: {}",
vss_path, entry.metadata.data_type
);
Err((ActuationError::UnsupportedType, message))
}
Err(UpdateError::WrongType) => {
let message = format!(
"Wrong type for vss_path {}. Expected type: {}",
vss_path, entry.metadata.data_type
);
Err((ActuationError::WrongType, message))
}
// Redundant errors in case UpdateError includes new errors in the future
Err(UpdateError::NotFound) => {
let message = format!("Could not resolve vss_path {}", vss_path);
Err((ActuationError::NotFound, message))
}
Err(UpdateError::PermissionDenied) => {
let message = format!("Permission denied for vss_path {}", vss_path);
Err((ActuationError::PermissionDenied, message))
}
Err(UpdateError::PermissionExpired) => Err((
ActuationError::PermissionExpired,
"Permission expired".to_string(),
)),
}
}
Err(ReadError::NotFound) => {
let message = format!("Could not resolve vss_path of vss_id {}", vss_id);
Err((ActuationError::NotFound, message))
}
Err(ReadError::PermissionDenied) => {
let message = format!("Permission denied for vss_id {}", vss_id);
Err((ActuationError::PermissionDenied, message))
}
Err(ReadError::PermissionExpired) => Err((
ActuationError::PermissionExpired,
"Permission expired".to_string(),
)),
}
}
}

impl DataBroker {
Expand Down
181 changes: 181 additions & 0 deletions databroker/src/grpc/kuksa_val_v2/val.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2380,6 +2380,64 @@ mod tests {
}
}

#[tokio::test]
async fn test_actuate_out_of_range() {
let broker = DataBroker::default();
let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL);

authorized_access
.add_entry(
"Vehicle.Cabin.Infotainment.Navigation.Volume".to_owned(),
broker::DataType::Uint8,
broker::ChangeType::OnChange,
broker::EntryType::Actuator,
"Some funny description".to_owned(),
Some(broker::types::DataValue::Uint32(0)), // min
Some(broker::types::DataValue::Uint32(100)), // max
None,
None,
)
.await
.expect("Register datapoint should succeed");

let vss_id = authorized_access
.get_id_by_path("Vehicle.Cabin.Infotainment.Navigation.Volume")
.await
.expect(
"Resolving the id of Vehicle.Cabin.Infotainment.Navigation.Volume should succeed",
);
let vss_ids = vec![vss_id];

let (sender, _) = mpsc::channel(10);
let actuation_provider = Provider { sender };
authorized_access
.provide_actuation(vss_ids, Box::new(actuation_provider))
.await
.expect("Registering a new Actuation Provider should succeed");

let mut request = tonic::Request::new(ActuateRequest {
signal_id: Some(SignalId {
signal: Some(proto::signal_id::Signal::Path(
"Vehicle.Cabin.Infotainment.Navigation.Volume".to_string(),
)),
}),
value: Some(Value {
typed_value: Some(proto::value::TypedValue::Uint32(200)),
}),
});

request
.extensions_mut()
.insert(permissions::ALLOW_ALL.clone());

let result_response = proto::val_server::Val::actuate(&broker, request).await;
assert!(result_response.is_err());
assert_eq!(
result_response.unwrap_err().code(),
tonic::Code::InvalidArgument
)
}

#[tokio::test]
async fn test_actuate_signal_not_found() {
let broker = DataBroker::default();
Expand Down Expand Up @@ -2502,6 +2560,129 @@ mod tests {
result_response.expect("Result should be Ok");
}

#[tokio::test]
async fn test_batch_actuate_out_of_range() {
let broker = DataBroker::default();
let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL);

authorized_access
.add_entry(
"Vehicle.ADAS.ABS.IsEnabled".to_owned(),
broker::DataType::Bool,
broker::ChangeType::OnChange,
broker::EntryType::Actuator,
"Some funny description".to_owned(),
None, // min
None, // max
None,
None,
)
.await
.expect("Register datapoint 'Vehicle.ADAS.ABS.IsEnabled' should succeed");

authorized_access
.add_entry(
"Vehicle.ADAS.CruiseControl.IsActive".to_owned(),
broker::DataType::Bool,
broker::ChangeType::OnChange,
broker::EntryType::Actuator,
"Some funny description".to_owned(),
None, // min
None, // max
None,
None,
)
.await
.expect("Register 'Vehicle.ADAS.CruiseControl.IsActive' datapoint should succeed");

authorized_access
.add_entry(
"Vehicle.Cabin.Infotainment.Navigation.Volume".to_owned(),
broker::DataType::Uint8,
broker::ChangeType::OnChange,
broker::EntryType::Actuator,
"Some funny description".to_owned(),
Some(broker::types::DataValue::Uint32(0)), // min
Some(broker::types::DataValue::Uint32(100)), // max
None,
None,
)
.await
.expect(
"Register datapoint 'Vehicle.Cabin.Infotainment.Navigation.Volume' should succeed",
);

let vss_id_abs = authorized_access
.get_id_by_path("Vehicle.ADAS.ABS.IsEnabled")
.await
.expect("Resolving the id of Vehicle.ADAS.ABS.IsEnabled should succeed");
let vss_id_cruise_control = authorized_access
.get_id_by_path("Vehicle.ADAS.CruiseControl.IsActive")
.await
.expect("Resolving the id of Vehicle.ADAS.CruiseControl.IsActive should succeed");
let vss_id_navigation_volume = authorized_access
.get_id_by_path("Vehicle.Cabin.Infotainment.Navigation.Volume")
.await
.expect(
"Resolving the id of Vehicle.Cabin.Infotainment.Navigation.Volume should succeed",
);

let vss_ids = vec![vss_id_abs, vss_id_cruise_control, vss_id_navigation_volume];

let (sender, _receiver) = mpsc::channel(10);
let actuation_provider = Provider { sender };
authorized_access
.provide_actuation(vss_ids, Box::new(actuation_provider))
.await
.expect("Registering a new Actuation Provider should succeed");

let mut request = tonic::Request::new(BatchActuateRequest {
actuate_requests: vec![
ActuateRequest {
signal_id: Some(SignalId {
signal: Some(proto::signal_id::Signal::Path(
"Vehicle.ADAS.ABS.IsEnabled".to_string(),
)),
}),
value: Some(Value {
typed_value: Some(proto::value::TypedValue::Bool(true)),
}),
},
ActuateRequest {
signal_id: Some(SignalId {
signal: Some(proto::signal_id::Signal::Path(
"Vehicle.ADAS.CruiseControl.IsActive".to_string(),
)),
}),
value: Some(Value {
typed_value: Some(proto::value::TypedValue::Bool(true)),
}),
},
ActuateRequest {
signal_id: Some(SignalId {
signal: Some(proto::signal_id::Signal::Path(
"Vehicle.Cabin.Infotainment.Navigation.Volume".to_string(),
)),
}),
value: Some(Value {
typed_value: Some(proto::value::TypedValue::Uint32(200)),
}),
},
],
});

request
.extensions_mut()
.insert(permissions::ALLOW_ALL.clone());

let result_response = proto::val_server::Val::batch_actuate(&broker, request).await;
assert!(result_response.is_err());
assert_eq!(
result_response.unwrap_err().code(),
tonic::Code::InvalidArgument
)
}

#[tokio::test]
async fn test_batch_actuate_signal_not_found() {
let broker = DataBroker::default();
Expand Down
56 changes: 55 additions & 1 deletion databroker/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* SPDX-License-Identifier: Apache-2.0
********************************************************************************/

use std::convert::TryFrom;
use std::{convert::TryFrom, fmt};

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum DataType {
Expand Down Expand Up @@ -41,6 +41,37 @@ pub enum DataType {
DoubleArray,
}

impl fmt::Display for DataType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
DataType::String => write!(f, "String"),
DataType::Bool => write!(f, "Bool"),
DataType::Int8 => write!(f, "Int8"),
DataType::Int16 => write!(f, "Int16"),
DataType::Int32 => write!(f, "Int32"),
DataType::Int64 => write!(f, "Int64"),
DataType::Uint8 => write!(f, "Uint8"),
DataType::Uint16 => write!(f, "Uint16"),
DataType::Uint32 => write!(f, "Uint32"),
DataType::Uint64 => write!(f, "Uint64"),
DataType::Float => write!(f, "Float"),
DataType::Double => write!(f, "Double"),
DataType::StringArray => write!(f, "StringArray"),
DataType::BoolArray => write!(f, "BoolArray"),
DataType::Int8Array => write!(f, "Int8Array"),
DataType::Int16Array => write!(f, "Int16Array"),
DataType::Int32Array => write!(f, "Int32Array"),
DataType::Int64Array => write!(f, "Int64Array"),
DataType::Uint8Array => write!(f, "Uint8Array"),
DataType::Uint16Array => write!(f, "Uint16Array"),
DataType::Uint32Array => write!(f, "Uint32Array"),
DataType::Uint64Array => write!(f, "Uint64Array"),
DataType::FloatArray => write!(f, "FloatArray"),
DataType::DoubleArray => write!(f, "DoubleArray"),
}
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum EntryType {
Sensor,
Expand Down Expand Up @@ -78,6 +109,29 @@ pub enum DataValue {

#[derive(Debug)]
pub struct CastError {}
impl fmt::Display for DataValue {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
DataValue::NotAvailable => write!(f, "Not Available"),
DataValue::Bool(value) => write!(f, "{}", value),
DataValue::String(value) => write!(f, "{}", value),
DataValue::Int32(value) => write!(f, "{}", value),
DataValue::Int64(value) => write!(f, "{}", value),
DataValue::Uint32(value) => write!(f, "{}", value),
DataValue::Uint64(value) => write!(f, "{}", value),
DataValue::Float(value) => write!(f, "{}", value),
DataValue::Double(value) => write!(f, "{}", value),
DataValue::BoolArray(values) => write!(f, "{:?}", values),
DataValue::StringArray(values) => write!(f, "{:?}", values),
DataValue::Int32Array(values) => write!(f, "{:?}", values),
DataValue::Int64Array(values) => write!(f, "{:?}", values),
DataValue::Uint32Array(values) => write!(f, "{:?}", values),
DataValue::Uint64Array(values) => write!(f, "{:?}", values),
DataValue::FloatArray(values) => write!(f, "{:?}", values),
DataValue::DoubleArray(values) => write!(f, "{:?}", values),
}
}
}

impl DataValue {
pub fn greater_than(&self, other: &DataValue) -> Result<bool, CastError> {
Expand Down

0 comments on commit 3bacf45

Please sign in to comment.