[bt][hfp] ServiceLevelConnection codecs_supported
Add new CodecId type which represents a type identifying a codec used in
HFP. The initial IDs are taken from HFP v1.8, Sec 10 / Appendix B.
Add support for querying the supported codecs from the
ServiceLevelConnection, including when codec negotiation is not
supported.
Add protection against codec IDs over the wire that are out of the
allowed range.
Will be used to setup codecs during audio connection setup.
Includes minor cleanup / DRYing of tests.
Bug: 64523
Test: added and updated unit tests
Test: fx test bt-hfp-audio-gateway-tests
Change-Id: Ifca50cc28fc6b62b3af940e44049fbbb0b222678
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/519922
Fuchsia-Auto-Submit: Marie Janssen 💖 <jamuraa@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Reviewed-by: Jeff Belgum <belgum@google.com>
diff --git a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/features.rs b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/features.rs
index 86a39bd..f4d4c95 100644
--- a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/features.rs
+++ b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/features.rs
@@ -72,3 +72,39 @@
this
}
}
+
+/// Codec IDs. See HFP 1.8, Section 10 / Appendix B.
+#[derive(PartialEq, Debug, Clone, Copy)]
+pub struct CodecId(u8);
+
+impl CodecId {
+ #[cfg(test)]
+ pub const CVSD: CodecId = CodecId(0x01);
+ pub const _MSBC: CodecId = CodecId(0x02);
+}
+
+impl From<u8> for CodecId {
+ fn from(x: u8) -> Self {
+ Self(x)
+ }
+}
+
+impl Into<u8> for CodecId {
+ fn into(self) -> u8 {
+ self.0
+ }
+}
+
+// Convenience conversions for interacting with AT library.
+// TODO(fxbug.dev/71403): Remove this once AT library supports specifying correct widths.
+impl Into<i64> for CodecId {
+ fn into(self) -> i64 {
+ self.0 as i64
+ }
+}
+
+impl PartialEq<i64> for CodecId {
+ fn eq(&self, other: &i64) -> bool {
+ self.0 as i64 == *other
+ }
+}
diff --git a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/peer/procedure/slc_initialization.rs b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/peer/procedure/slc_initialization.rs
index 8a78470..bcda2c3 100644
--- a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/peer/procedure/slc_initialization.rs
+++ b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/peer/procedure/slc_initialization.rs
@@ -14,7 +14,7 @@
},
};
-use {at_commands as at, num_traits::FromPrimitive};
+use {at_commands as at, num_traits::FromPrimitive, std::convert::TryFrom};
/// A singular state within the SLC Initialization Procedure.
pub trait SlcProcedureState {
@@ -168,7 +168,12 @@
// availability specified in the feature flags of the HF and AG.
match (update, state.codec_negotiation()) {
(at::Command::Bac { codecs }, true) => {
- state.hf_supported_codecs = Some(codecs.into_iter().map(|x| x as u32).collect());
+ state.hf_supported_codecs = Some(
+ codecs
+ .into_iter()
+ .filter_map(|x| u8::try_from(x).ok().map(Into::into))
+ .collect(),
+ );
Box::new(AvailableCodecsReceived)
}
(at::Command::CindTest {}, false) => Box::new(AgSupportedIndicatorsRequested),
@@ -437,6 +442,8 @@
#[cfg(test)]
mod tests {
use super::*;
+
+ use crate::features::{CodecId, HfFeatures};
use matches::assert_matches;
#[test]
@@ -448,12 +455,16 @@
};
let mut procedure =
SlcInitProcedure::new_at_state(AgFeaturesReceived { state: state.clone() });
- let update = at::Command::Bac { codecs: Vec::new() };
+ let codecs = vec![CodecId::CVSD.into(), 0xf09f9296, 0x04];
+ let update = at::Command::Bac { codecs };
// Both parties support codec negotiation, so upon receiving the Codec HF message, we
// expect to successfully transition to the codec state and the resulting event
// should be an Ack to the codecs.
let event = procedure.hf_update(update, &mut state);
assert_matches!(event, ProcedureRequest::SendMessages(msgs) if msgs == vec![at::Response::Ok]);
+
+ // At this point, the codecs received should be a filtered into allowed of the above.
+ assert_eq!(Some(vec![CodecId::CVSD, (0x04).into()]), state.hf_supported_codecs);
}
#[test]
diff --git a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/peer/service_level_connection.rs b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/peer/service_level_connection.rs
index 4ec0d36..6f9bc64 100644
--- a/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/peer/service_level_connection.rs
+++ b/src/connectivity/bluetooth/profiles/bt-hfp-audio-gateway/src/peer/service_level_connection.rs
@@ -26,7 +26,8 @@
slc_request::SlcRequest,
update::AgUpdate,
};
-use crate::features::{AgFeatures, HfFeatures};
+
+use crate::features::{AgFeatures, CodecId, HfFeatures};
/// The maximum number of concurrent procedures currently supported by this SLC.
/// This value is chosen as a number significantly more than the total number of procedures
@@ -63,8 +64,8 @@
pub ag_features: AgFeatures,
/// The features of the HF.
pub hf_features: HfFeatures,
- /// The codecs supported by the HF.
- pub hf_supported_codecs: Option<Vec<u32>>,
+ /// The codecs supported by the HF. If initialized, None if Codec Negotiation is not supported.
+ pub hf_supported_codecs: Option<Vec<CodecId>>,
/// The indicators supported by the HF and its current status.
pub hf_indicators: HfIndicators,
/// The current AG indicator events reporting state.
@@ -99,6 +100,15 @@
self.hf_features.contains(HfFeatures::HF_INDICATORS)
&& self.ag_features.contains(AgFeatures::HF_INDICATORS)
}
+
+ /// Returns the codecs that are known to be supported by the peer.
+ /// May be inaccurate if initialization has not completed.
+ #[cfg(test)]
+ pub fn codecs_supported(&self) -> Vec<CodecId> {
+ // All HFs are required to support CVSD, even if they don't support Codec Negotiation.
+ // See HFP v1.8, Sec 5.7 for more information.
+ self.hf_supported_codecs.clone().unwrap_or(vec![CodecId::CVSD])
+ }
}
/// Provides an API for managing data packets to be sent over the provided `channel`.
@@ -283,6 +293,12 @@
self.state.initialized = true;
}
+ /// Returns the codecs supported by the by the peer.
+ #[cfg(test)]
+ pub fn codecs_supported(&self) -> Vec<CodecId> {
+ self.state.codecs_supported()
+ }
+
pub fn network_operator_name_format(&self) -> &Option<at::NetworkOperatorNameFormat> {
&self.state.ag_network_operator_name_format
}
@@ -699,6 +715,12 @@
assert_matches!(exec.run_until_stalled(&mut remote_fut), Poll::Pending);
}
+ /// Expects the service level connection to be pending, and polls to check that it is.
+ #[track_caller]
+ fn expect_slc_pending(exec: &mut fasync::Executor, slc: &mut ServiceLevelConnection) {
+ assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ }
+
/// Serializes the AT Response into a byte buffer.
#[track_caller]
pub fn serialize_at_response(response: at::Response) -> Vec<u8> {
@@ -765,7 +787,7 @@
let _ = remote.as_ref().write(&unexpected);
// No requests should be received on the stream.
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
// Channel should be disconnected now.
assert!(!slc.connected());
@@ -829,13 +851,13 @@
do_ag_update(&mut exec, &mut slc, slci_marker, response_fn1(features));
expect_peer_ready(&mut exec, &mut remote, None);
// No further requests - waiting on peer response.
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
// Peer sends us an HF supported indicators request - since the SLC can handle the request,
// we expect no item in the SLC stream. The response should directly be sent to the peer.
let command2 = format!("AT+CIND=?\r").into_bytes();
let _ = remote.as_ref().write(&command2);
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
expect_peer_ready(&mut exec, &mut remote, None);
// Peer requests the indicator status. Since this status is not managed by the SLC, we
@@ -853,22 +875,39 @@
do_ag_update(&mut exec, &mut slc, slci_marker, response_fn2(AgIndicators::default()));
expect_peer_ready(&mut exec, &mut remote, None);
// No further requests - waiting on peer response.
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
// Peer requests to enable the Indicator Status update in the AG - since the SLC can
// handle the request, we expect no item in the SLC stream, and the response should directly
// be sent to the peer.
let command4 = format!("AT+CMER=3,0,0,1\r").into_bytes();
let _ = remote.as_ref().write(&command4);
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
expect_peer_ready(&mut exec, &mut remote, None);
// The SLC should be considered initialized and the SLCI Procedure is done.
assert!(slc.initialized());
+
+ // Without Codec Negotiation, only CVSD is supported.
+ assert_eq!(vec![CodecId::CVSD], slc.codecs_supported());
+
assert!(!slc.is_active(&slci_marker));
}
#[test]
+ fn slc_state_codecs_supported() {
+ let mut state = SlcState::default();
+ // Without any codecs supported, only CVSD is returned.
+ assert_eq!(vec![CodecId::CVSD], state.codecs_supported());
+
+ // When codec negotiation is done, and we have a list, the list is returned.
+ let codecs_raw: [u8; 4] = [0xf0, 0x9f, 0x92, 0x96];
+ let codecs: Vec<CodecId> = codecs_raw.iter().cloned().map(Into::into).collect();
+ state.hf_supported_codecs = Some(codecs.clone());
+ assert_eq!(codecs, state.codecs_supported());
+ }
+
+ #[test]
fn slci_command_after_initialization_returns_error() {
let _exec = fasync::Executor::new().unwrap();
let (mut slc, _remote) = create_and_connect_slc();
@@ -930,7 +969,7 @@
value: 0i64,
});
do_ag_update(&mut exec, &mut slc, ProcedureMarker::PhoneStatus, status1.into());
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
expect_peer_pending(&mut exec, &mut remote);
// Peer sends us HF features - we expect a request for the AG features on the
@@ -958,13 +997,13 @@
value: 1i64,
});
do_ag_update(&mut exec, &mut slc, ProcedureMarker::PhoneStatus, status2.into());
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
expect_peer_pending(&mut exec, &mut remote);
// Peer continues the SLCI procedure.
let command2 = format!("AT+CIND=?\r").into_bytes();
let _ = remote.as_ref().write(&command2);
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
expect_peer_ready(&mut exec, &mut remote, None);
let command3 = format!("AT+CIND?\r").into_bytes();
let _ = remote.as_ref().write(&command3);
@@ -980,12 +1019,12 @@
// Simulate local response with AG indicators status - expect this to go to the peer.
do_ag_update(&mut exec, &mut slc, ProcedureMarker::SlcInitialization, ag_indicators);
expect_peer_ready(&mut exec, &mut remote, None);
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
// Peer requests to enable the Indicator Status update in the AG.
let command4 = format!("AT+CMER=3,0,0,1\r").into_bytes();
let _ = remote.as_ref().write(&command4);
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
expect_peer_ready(&mut exec, &mut remote, None);
// At this point, the mandatory portion of the SLCI procedure is complete. There are no optional
@@ -1004,7 +1043,7 @@
// The next time the SLC stream is polled, we expect the updates to be processed.
// Since these are phone status updates, we expect the one-shot procedures to send data
// to the peer and therefore no SLC stream items.
- assert_matches!(exec.run_until_stalled(&mut slc.next()), Poll::Pending);
+ expect_slc_pending(&mut exec, &mut slc);
expect_peer_ready(&mut exec, &mut remote, Some(serialize_at_response(expected1)));
expect_peer_ready(&mut exec, &mut remote, Some(serialize_at_response(expected2)));
expect_peer_ready(&mut exec, &mut remote, Some(serialize_at_response(expected3)));
