[bt][hfp] Defer validation of HF Indicators.
Validation should happen in the HFP AG component so that the parsing is
robust to partially valid AT commands. Valid indicators should be
accepted while invalid indicators should be gracefully ignored.
Bug: 74311
Test: Added unit tests
Test: Passes PTS
Change-Id: I19f863d875bd724e2649d7d80de523cf14df7611
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/514486
Commit-Queue: Jeff Belgum <belgum@google.com>
Fuchsia-Auto-Submit: Jeff Belgum <belgum@google.com>
Reviewed-by: Ani Ramakrishnan <aniramakri@google.com>
diff --git a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/BUILD.gn b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/BUILD.gn
index 8428786..0397958 100644
--- a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/BUILD.gn
+++ b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/BUILD.gn
@@ -27,6 +27,7 @@
"//third_party/rust_crates:bitflags",
"//third_party/rust_crates:futures",
"//third_party/rust_crates:log",
+ "//third_party/rust_crates:num-traits",
"//third_party/rust_crates:regex",
"//third_party/rust_crates:serde",
"//third_party/rust_crates:serde_json",
diff --git a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/procedure/slc_initialization.rs b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/procedure/slc_initialization.rs
index 6fa04dd..38d3331 100644
--- a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/procedure/slc_initialization.rs
+++ b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/procedure/slc_initialization.rs
@@ -15,7 +15,7 @@
},
};
-use at_commands as at;
+use {at_commands as at, num_traits::FromPrimitive};
/// This mode's behavior is to forward unsolicited result codes directly per
/// 3GPP TS 27.007 version 6.8.0, Section 8.10.
@@ -333,6 +333,10 @@
match update {
at::Command::Bind { indicators } => {
+ let indicators = indicators
+ .into_iter()
+ .filter_map(at_commands::BluetoothHFIndicator::from_i64)
+ .collect();
state.hf_indicators.enable_indicators(indicators);
Box::new(HfSupportedIndicatorsReceived)
}
@@ -648,7 +652,7 @@
// Optional - HF sends its supported HF indicators.
let inds =
vec![at::BluetoothHFIndicator::EnhancedSafety, at::BluetoothHFIndicator::BatteryLevel];
- let update9 = at::Command::Bind { indicators: inds.clone() };
+ let update9 = at::Command::Bind { indicators: inds.iter().map(|i| *i as i64).collect() };
let expected_messages9 = vec![at::Response::Ok];
assert_matches!(slc_proc.hf_update(update9, &mut state),
ProcedureRequest::SendMessages(m) if m == expected_messages9
@@ -735,4 +739,65 @@
state.state.hf_features.set(HfFeatures::HF_INDICATORS, true);
assert!(state.is_terminal());
}
+
+ /// Assert that the current procedure state is not an error.
+ /// This is done by checking that the produced .request() value is not an error.
+ #[track_caller]
+ fn assert_not_error(state: &dyn SlcProcedureState) {
+ if let ProcedureRequest::Error(e) = state.request() {
+ panic!("Error state: {}", e);
+ }
+ }
+
+ #[test]
+ fn procedure_ignores_invalid_bind_values() {
+ // All bind values are invalid.
+ let mut state = SlcState::default();
+ state.hf_features = HfFeatures::all();
+ state.ag_features = AgFeatures::all();
+
+ assert!(!state.hf_indicators.enhanced_safety_enabled());
+ assert!(!state.hf_indicators.battery_level_enabled());
+
+ let tws_recv = ThreeWaySupportReceived { state: state.clone() };
+ let command = at::Command::Bind { indicators: vec![0, 99] };
+ let next = tws_recv.hf_update(command, &mut state);
+
+ // Next state is not an error
+ assert_not_error(&*next);
+
+ // No indicator states have changed.
+ assert!(!state.hf_indicators.enhanced_safety_enabled());
+ assert!(!state.hf_indicators.battery_level_enabled());
+
+ // First bind value is invalid
+ let mut state = SlcState::default();
+ state.hf_features = HfFeatures::all();
+ state.ag_features = AgFeatures::all();
+ let tws_recv = ThreeWaySupportReceived { state: state.clone() };
+ let command = at::Command::Bind { indicators: vec![0, 1] };
+ let next = tws_recv.hf_update(command, &mut state);
+
+ // Next state is not an error
+ assert_not_error(&*next);
+
+ // Only valid indicator, enhanced safety, has changed.
+ assert!(state.hf_indicators.enhanced_safety_enabled());
+ assert!(!state.hf_indicators.battery_level_enabled());
+
+ // Second bind value is invalid
+ let mut state = SlcState::default();
+ state.hf_features = HfFeatures::all();
+ state.ag_features = AgFeatures::all();
+ let tws_recv = ThreeWaySupportReceived { state: state.clone() };
+ let command = at::Command::Bind { indicators: vec![2, 99] };
+ let next = tws_recv.hf_update(command, &mut state);
+
+ // Next state is not an error
+ assert_not_error(&*next);
+
+ // Only valid indicator, battery level, has changed.
+ assert!(!state.hf_indicators.enhanced_safety_enabled());
+ assert!(state.hf_indicators.battery_level_enabled());
+ }
}
diff --git a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/protocol/indicators.rs b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/protocol/indicators.rs
index 39582a0..22ee44f 100644
--- a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/protocol/indicators.rs
+++ b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/protocol/indicators.rs
@@ -53,6 +53,16 @@
/// Defined in HFP v1.8 Section 4.35.
const MAX_BATTERY_LEVEL: u8 = 100;
+ #[cfg(test)]
+ pub fn enhanced_safety_enabled(&self) -> bool {
+ self.enhanced_safety.enabled
+ }
+
+ #[cfg(test)]
+ pub fn battery_level_enabled(&self) -> bool {
+ self.battery_level.enabled
+ }
+
/// Enables the supported HF indicators based on the provided AT `indicators`.
pub fn enable_indicators(&mut self, indicators: Vec<at::BluetoothHFIndicator>) {
for ind in indicators {
diff --git a/src/connectivity/bluetooth/profiles/tests/bt-hfp-audio-gateway-integration-tests/src/main.rs b/src/connectivity/bluetooth/profiles/tests/bt-hfp-audio-gateway-integration-tests/src/main.rs
index 9fbf4fd..904f136 100644
--- a/src/connectivity/bluetooth/profiles/tests/bt-hfp-audio-gateway-integration-tests/src/main.rs
+++ b/src/connectivity/bluetooth/profiles/tests/bt-hfp-audio-gateway-integration-tests/src/main.rs
@@ -356,7 +356,7 @@
.await;
let peer_supported_indicators_cmd =
- at::Command::Bind { indicators: vec![at::BluetoothHFIndicator::BatteryLevel] };
+ at::Command::Bind { indicators: vec![at::BluetoothHFIndicator::BatteryLevel as i64] };
send_command_and_expect_response(
&mut remote,
peer_supported_indicators_cmd,
diff --git a/src/connectivity/lib/at-commands/definitions/hfp.at b/src/connectivity/lib/at-commands/definitions/hfp.at
index 345b3ce..21e75b6 100644
--- a/src/connectivity/lib/at-commands/definitions/hfp.at
+++ b/src/connectivity/lib/at-commands/definitions/hfp.at
@@ -61,7 +61,7 @@
# Bluetooth HF Indicators Feature list command
# HFP 1.8 4.35.1
-command { AT+BIND=indicators: List<BluetoothHFIndicator> }
+command { AT+BIND=indicators: List<Integer> }
# Bluetooth HF Indicators Feature read command
# HFP 1.8 4.35.1
