Skip to content

Commit

Permalink
Merge pull request #93 from boschglobal/feature/missing_min_max_restr…
Browse files Browse the repository at this point in the history 
…iction_for_sdv_databroker_v1

Add missing min max restriction for sdv.databroker.v1
  • Loading branch information
@rafaeling
rafaeling authored Nov 5, 2024
2 parents aa4444f + e737ae0 commit f9f7f6f
Show file tree
Hide file tree
Showing 4 changed files with 276 additions and 65 deletions.
6 changes: 3 additions & 3 deletions databroker-cli/src/sdv_cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,7 @@ mod test {
entry_type: proto::v1::EntryType::Sensor.into(),
change_type: proto::v1::ChangeType::OnChange.into(),
description: "".into(),
allowed: None,
value_restriction: None,
},
proto::v1::Metadata {
id: 2,
Expand All @@ -1271,7 +1271,7 @@ mod test {
entry_type: proto::v1::EntryType::Sensor.into(),
change_type: proto::v1::ChangeType::OnChange.into(),
description: "".into(),
allowed: None,
value_restriction: None,
},
proto::v1::Metadata {
id: 3,
Expand All @@ -1280,7 +1280,7 @@ mod test {
entry_type: proto::v1::EntryType::Sensor.into(),
change_type: proto::v1::ChangeType::OnChange.into(),
description: "".into(),
allowed: None,
value_restriction: None,
},
]
.to_vec();
Expand Down
82 changes: 82 additions & 0 deletions databroker/src/grpc/sdv_databroker_v1/collector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,3 +270,85 @@ impl proto::collector_server::Collector for broker::DataBroker {
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::{broker::DataBroker, permissions};
use proto::collector_server::Collector;

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

let entry_id_1 = authorized_access
.add_entry(
"test.datapoint1".to_owned(),
broker::DataType::Uint8,
broker::ChangeType::OnChange,
broker::EntryType::Sensor,
"Test datapoint 1".to_owned(),
Some(broker::types::DataValue::Uint32(3)), // min
Some(broker::types::DataValue::Uint32(26)), // max
None,
None,
)
.await
.unwrap();

let entry_id_2 = authorized_access
.add_entry(
"test.datapoint1.Speed".to_owned(),
broker::DataType::Float,
broker::ChangeType::OnChange,
broker::EntryType::Sensor,
"Test datapoint 1".to_owned(),
Some(broker::types::DataValue::Float(1.0)), // min
Some(broker::types::DataValue::Float(100.0)), // max
None,
None,
)
.await
.unwrap();

let datapoint: proto::Datapoint = proto::Datapoint {
timestamp: None,
value: Some(proto::datapoint::Value::Int32Value(50)),
};

let mut datapoints = HashMap::new();
datapoints.insert(entry_id_1, datapoint.clone());
datapoints.insert(entry_id_2, datapoint);

let request = proto::UpdateDatapointsRequest { datapoints };

// Manually insert permissions
let mut publish_value_request = tonic::Request::new(request);
publish_value_request
.extensions_mut()
.insert(permissions::ALLOW_ALL.clone());

match broker.update_datapoints(publish_value_request).await {
Ok(response) => {
let response = response.into_inner();
assert_eq!(response.errors.len(), 2);

let error_entry_1 = response.errors.get(&entry_id_1);
assert_eq!(
error_entry_1.unwrap().clone(),
proto::DatapointError::OutOfBounds as i32
);

let error_entry_2 = response.errors.get(&entry_id_2);
assert_eq!(
error_entry_2.unwrap().clone(),
proto::DatapointError::InvalidType as i32
);
}
Err(_) => {
panic!("Should not happen!");
}
}
}
}
195 changes: 147 additions & 48 deletions databroker/src/grpc/sdv_databroker_v1/conversions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use databroker_proto::sdv::databroker::v1 as proto;
use prost_types::Timestamp;
use std::convert::TryInto;
use std::time::SystemTime;
use tracing::debug;

use crate::broker;

Expand Down Expand Up @@ -302,6 +303,151 @@ impl From<&proto::ChangeType> for broker::ChangeType {
}
}

fn value_restriction_from(metadata: &broker::Metadata) -> Option<proto::ValueRestriction> {
match metadata.data_type {
broker::DataType::String | broker::DataType::StringArray => {
let allowed = match metadata.allowed.as_ref() {
Some(broker::DataValue::StringArray(vec)) => vec.clone(),
_ => Vec::new(),
};

if !allowed.is_empty() {
return Some(proto::ValueRestriction {
r#type: Some(proto::value_restriction::Type::String(
proto::ValueRestrictionString {
allowed_values: allowed,
},
)),
});
};
}
broker::DataType::Int8
| broker::DataType::Int16
| broker::DataType::Int32
| broker::DataType::Int64
| broker::DataType::Int8Array
| broker::DataType::Int16Array
| broker::DataType::Int32Array
| broker::DataType::Int64Array => {
let min_value = match metadata.min {
Some(broker::DataValue::Int32(value)) => Some(i64::from(value)),
Some(broker::DataValue::Int64(value)) => Some(value),
_ => None,
};
let max_value = match metadata.max {
Some(broker::DataValue::Int32(value)) => Some(i64::from(value)),
Some(broker::DataValue::Int64(value)) => Some(value),
_ => None,
};
let allowed = match metadata.allowed.as_ref() {
Some(allowed) => match allowed {
broker::DataValue::Int32Array(vec) => {
vec.iter().cloned().map(i64::from).collect()
}
broker::DataValue::Int64Array(vec) => vec.to_vec(),
_ => Vec::new(),
},
_ => Vec::new(),
};

if min_value.is_some() | max_value.is_some() | !allowed.is_empty() {
return Some(proto::ValueRestriction {
r#type: Some(proto::value_restriction::Type::Signed(
proto::ValueRestrictionInt {
allowed_values: allowed,
min: min_value,
max: max_value,
},
)),
});
};
}
broker::DataType::Uint8
| broker::DataType::Uint16
| broker::DataType::Uint32
| broker::DataType::Uint64
| broker::DataType::Uint8Array
| broker::DataType::Uint16Array
| broker::DataType::Uint32Array
| broker::DataType::Uint64Array => {
let min_value = match metadata.min {
Some(broker::DataValue::Uint32(value)) => Some(u64::from(value)),
Some(broker::DataValue::Uint64(value)) => Some(value),
_ => None,
};
let max_value = match metadata.max {
Some(broker::DataValue::Uint32(value)) => Some(u64::from(value)),
Some(broker::DataValue::Uint64(value)) => Some(value),
_ => None,
};
let allowed = match metadata.allowed.as_ref() {
Some(allowed) => match allowed {
broker::DataValue::Uint32Array(vec) => {
vec.iter().cloned().map(u64::from).collect()
}
broker::DataValue::Uint64Array(vec) => vec.to_vec(),
_ => Vec::new(),
},
_ => Vec::new(),
};

if min_value.is_some() | max_value.is_some() | !allowed.is_empty() {
return Some(proto::ValueRestriction {
r#type: Some(proto::value_restriction::Type::Unsigned(
proto::ValueRestrictionUint {
allowed_values: allowed,
min: min_value,
max: max_value,
},
)),
});
};
}
broker::DataType::Float
| broker::DataType::Double
| broker::DataType::FloatArray
| broker::DataType::DoubleArray => {
let min_value = match metadata.min {
Some(broker::DataValue::Float(value)) => Some(f64::from(value)),
Some(broker::DataValue::Double(value)) => Some(value),
_ => None,
};
let max_value = match metadata.max {
Some(broker::DataValue::Float(value)) => Some(f64::from(value)),
Some(broker::DataValue::Double(value)) => Some(value),
_ => None,
};
let allowed = match metadata.allowed.as_ref() {
Some(allowed) => match allowed {
broker::DataValue::FloatArray(vec) => {
vec.iter().cloned().map(f64::from).collect()
}
broker::DataValue::DoubleArray(vec) => vec.to_vec(),
_ => Vec::new(),
},
_ => Vec::new(),
};

if min_value.is_some() | max_value.is_some() | !allowed.is_empty() {
return Some(proto::ValueRestriction {
r#type: Some(proto::value_restriction::Type::FloatingPoint(
proto::ValueRestrictionFloat {
allowed_values: allowed,
min: min_value,
max: max_value,
},
)),
});
};
}

_ => {
debug!("Datatype {:?} not yet handled", metadata.data_type);
}
};
None
}

impl From<&broker::Metadata> for proto::Metadata {
fn from(metadata: &broker::Metadata) -> Self {
proto::Metadata {
Expand All @@ -311,54 +457,7 @@ impl From<&broker::Metadata> for proto::Metadata {
data_type: proto::DataType::from(&metadata.data_type) as i32,
change_type: proto::ChangeType::Continuous as i32, // TODO: Add to metadata
description: metadata.description.to_owned(),
allowed: match metadata.allowed.as_ref() {
Some(broker::DataValue::StringArray(vec)) => Some(proto::Allowed {
values: Some(proto::allowed::Values::StringValues(proto::StringArray {
values: vec.clone(),
})),
}),
Some(broker::DataValue::Int32Array(vec)) => Some(proto::Allowed {
values: Some(proto::allowed::Values::Int32Values(proto::Int32Array {
values: vec.clone(),
})),
}),
Some(broker::DataValue::Int64Array(vec)) => Some(proto::Allowed {
values: Some(proto::allowed::Values::Int64Values(proto::Int64Array {
values: vec.clone(),
})),
}),
Some(broker::DataValue::Uint32Array(vec)) => Some(proto::Allowed {
values: Some(proto::allowed::Values::Uint32Values(proto::Uint32Array {
values: vec.clone(),
})),
}),
Some(broker::DataValue::Uint64Array(vec)) => Some(proto::Allowed {
values: Some(proto::allowed::Values::Uint64Values(proto::Uint64Array {
values: vec.clone(),
})),
}),
Some(broker::DataValue::FloatArray(vec)) => Some(proto::Allowed {
values: Some(proto::allowed::Values::FloatValues(proto::FloatArray {
values: vec.clone(),
})),
}),
Some(broker::DataValue::DoubleArray(vec)) => Some(proto::Allowed {
values: Some(proto::allowed::Values::DoubleValues(proto::DoubleArray {
values: vec.clone(),
})),
}),
Some(broker::DataValue::BoolArray(_))
| Some(broker::DataValue::NotAvailable)
| Some(broker::DataValue::Bool(_))
| Some(broker::DataValue::String(_))
| Some(broker::DataValue::Int32(_))
| Some(broker::DataValue::Int64(_))
| Some(broker::DataValue::Uint32(_))
| Some(broker::DataValue::Uint64(_))
| Some(broker::DataValue::Float(_))
| Some(broker::DataValue::Double(_))
| None => None,
},
value_restriction: value_restriction_from(metadata),
}
}
}
Expand Down
58 changes: 44 additions & 14 deletions proto/sdv/databroker/v1/types.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,18 +143,6 @@ message Datapoint {
}
}

message Allowed {
oneof values {
StringArray string_values = 1;
Int32Array int32_values = 3;
Int64Array int64_values = 4;
Uint32Array uint32_values = 5;
Uint64Array uint64_values = 6;
FloatArray float_values = 7;
DoubleArray double_values = 8;
}
}

message Metadata {
int32 id = 1;
EntryType entry_type = 2;
Expand All @@ -163,7 +151,49 @@ message Metadata {
ChangeType change_type = 6; // CONTINUOUS or STATIC or ON_CHANGE
string description = 7;

Allowed allowed = 10;
ValueRestriction value_restriction = 10;
// int32 min_update_hz = 10; // Only for CONTINUOUS
// int32 max_update_hz = 11; // Only for CONTINUOUS
};
}

// Value restriction
//
// One ValueRestriction{type} for each type, since
// they don't make sense unless the types match
//
message ValueRestriction {
oneof type {
ValueRestrictionString string = 21;
// For signed VSS integers
ValueRestrictionInt signed = 22;
// For unsigned VSS integers
ValueRestrictionUint unsigned = 23;
// For floating point VSS values (float and double)
ValueRestrictionFloat floating_point = 24;
}
}

message ValueRestrictionInt {
optional sint64 min = 1;
optional sint64 max = 2;
repeated sint64 allowed_values = 3;
}

message ValueRestrictionUint {
optional uint64 min = 1;
optional uint64 max = 2;
repeated uint64 allowed_values = 3;
}

message ValueRestrictionFloat {
optional double min = 1;
optional double max = 2;

// allowed for doubles/floats not recommended
repeated double allowed_values = 3;
}

// min, max doesn't make much sense for a string
message ValueRestrictionString {
repeated string allowed_values = 1;
}

0 comments on commit f9f7f6f

Please sign in to comment.