diff options
| author | 2024-06-13 10:29:02 -0700 | |
|---|---|---|
| committer | 2024-07-16 21:19:05 +0000 | |
| commit | 84fde0a3e197ef7b22a0c227a479e3164711cd29 (patch) | |
| tree | 61b389ed55f102b43b4ff658950309992ea0a368 | |
| parent | 83e9172361fd4d96f60d276457441d4ce592c37a (diff) | |
system/rust/gatt: Switch to the default PDL rust generator
The generator rust_no_alloc is no longer maintained and the
default rust generator is now the preferred rust generator.
Bug: 331817295
Test: m com.android.btservices
Test: atest --host libbluetooth_core_rs_test
Flag: EXEMPT, mechanical refactor
Change-Id: I18da339ffb2537229fb870594edd344b6a924241
36 files changed, 836 insertions, 1186 deletions
diff --git a/system/rust/Android.bp b/system/rust/Android.bp index dfb0521323..95f50cf9cf 100644 --- a/system/rust/Android.bp +++ b/system/rust/Android.bp @@ -36,8 +36,10 @@ rust_defaults { "libbitflags", "libbt_common", "libbt_shim", + "libbytes", "libcxx", "liblog_rust", + "libpdl_runtime", "libscopeguard", ], whole_static_libs: [ @@ -193,7 +195,7 @@ gensrcs { genrule { name: "bluetooth_core_rust_packets", - defaults: ["pdl_rust_noalloc_generator_defaults"], + defaults: ["pdl_rust_generator_defaults"], srcs: ["src/packets.pdl"], out: ["_packets.rs"], } diff --git a/system/rust/Cargo.toml b/system/rust/Cargo.toml index 21dfee2f20..a9ab090891 100644 --- a/system/rust/Cargo.toml +++ b/system/rust/Cargo.toml @@ -31,13 +31,14 @@ android_logger = "*" jni = "*" paste = "*" async-trait = "*" -pdl-runtime = "0.3.0" +pdl-runtime = "0.3.1" tokio-test = "0.4.2" tokio = { version = "1.23.0", features = ["macros"] } scopeguard = "1.1.0" +bytes = "1.5.0" [build-dependencies] -pdl-compiler = "0.3.0" +pdl-compiler = "0.3.1" [lib] crate-type = ["rlib"] diff --git a/system/rust/build.rs b/system/rust/build.rs index c296fb8e0b..bae31cdcaa 100644 --- a/system/rust/build.rs +++ b/system/rust/build.rs @@ -2,7 +2,6 @@ //! //! Run `cargo install --path .` in `external/rust/crates/pdl-compiler` to ensure `pdlc` //! is in your path. -use pdl_compiler; use std::{env, fs::File, io::Write, path::Path}; fn main() { @@ -13,9 +12,8 @@ fn main() { let mut sources = pdl_compiler::ast::SourceDatabase::new(); let file = pdl_compiler::parser::parse_file(&mut sources, "src/packets.pdl") .expect("failed to parse input pdl file"); - let schema = pdl_compiler::backends::intermediate::generate(&file).unwrap(); - let generated = pdl_compiler::backends::rust_no_allocation::generate(&file, &schema).unwrap(); + let generated = pdl_compiler::backends::rust::generate(&sources, &file, &[]); dest_file.write_all(generated.as_bytes()).unwrap(); println!("cargo:rerun-if-changed=build.rs"); diff --git a/system/rust/src/core/uuid.rs b/system/rust/src/core/uuid.rs index d519854063..fe1756c7fe 100644 --- a/system/rust/src/core/uuid.rs +++ b/system/rust/src/core/uuid.rs @@ -1,8 +1,6 @@ //! A UUID (See Core Spec 5.3 Vol 1E 2.9.1. Basic Types) -use crate::packets::{ - ParseError, Uuid128Builder, Uuid128View, Uuid16Builder, Uuid16View, UuidBuilder, UuidView, -}; +use crate::packets::att; /// A UUID (See Core Spec 5.3 Vol 1E 2.9.1. Basic Types) /// @@ -33,83 +31,74 @@ impl Uuid { } } -impl TryFrom<UuidView<'_>> for Uuid { - type Error = ParseError; +impl TryFrom<att::Uuid> for Uuid { + type Error = att::Uuid; - fn try_from(value: UuidView<'_>) -> Result<Self, ParseError> { - let bytes = value.get_data_iter().collect::<Vec<_>>(); + fn try_from(value: att::Uuid) -> Result<Self, att::Uuid> { + let bytes = value.data.as_slice(); Ok(match bytes.len() { 2 => Self::new(u16::from_le_bytes([bytes[0], bytes[1]]) as u32), 4 => Self::new(u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])), // TODO(aryarahul) - should we handle >16 byte Uuids and drop extra bytes? - _ => Self::new_from_le_bytes( - bytes.try_into().map_err(|_| ParseError::OutOfBoundsAccess)?, - ), + _ => Self::new_from_le_bytes(bytes.try_into().map_err(|_| value)?), }) } } -impl From<Uuid16View<'_>> for Uuid { - fn from(uuid: Uuid16View) -> Self { - Self::new(uuid.get_data() as u32) +impl From<att::Uuid16> for Uuid { + fn from(uuid: att::Uuid16) -> Self { + Self::new(uuid.data as u32) } } -impl From<Uuid128View<'_>> for Uuid { - fn from(uuid: Uuid128View) -> Self { - Self::new_from_le_bytes( - uuid.get_data_iter() - .collect::<Vec<_>>() - .try_into() - .expect("Uuid128View MUST have exactly 16 bytes"), - ) +impl From<att::Uuid128> for Uuid { + fn from(uuid: att::Uuid128) -> Self { + Self::new_from_le_bytes(uuid.data) } } -impl From<Uuid> for UuidBuilder { +impl From<Uuid> for att::Uuid { fn from(value: Uuid) -> Self { // TODO(aryarahul): compress to UUID-16 if possible - UuidBuilder { data: value.le_bytes().into_iter().collect() } + att::Uuid { data: value.le_bytes().to_vec() } } } -impl TryFrom<Uuid> for Uuid16Builder { +impl TryFrom<Uuid> for att::Uuid16 { type Error = Uuid; fn try_from(value: Uuid) -> Result<Self, Self::Error> { let backing = u128::from_be_bytes(value.0); if backing & ((1u128 << 96) - 1) == BASE_UUID { if let Ok(data) = u16::try_from(backing >> 96) { - return Ok(Uuid16Builder { data }); + return Ok(att::Uuid16 { data }); } } Err(value) } } -impl From<Uuid> for Uuid128Builder { +impl From<Uuid> for att::Uuid128 { fn from(value: Uuid) -> Self { - Uuid128Builder { data: value.le_bytes().to_vec().into() } + att::Uuid128 { data: value.le_bytes() } } } #[cfg(test)] mod test { - use crate::utils::packet::build_view_or_crash; - use super::*; #[test] fn test_uuid16_builder_successful() { let uuid = Uuid::new(0x0102); - let builder: Uuid16Builder = uuid.try_into().unwrap(); + let builder: att::Uuid16 = uuid.try_into().unwrap(); assert_eq!(builder.data, 0x0102); } #[test] fn test_uuid16_builder_fail_nonzero_trailing_bytes() { let uuid = Uuid::new(0x01020304); - let res: Result<Uuid16Builder, _> = uuid.try_into(); + let res: Result<att::Uuid16, _> = uuid.try_into(); assert!(res.is_err()); } @@ -118,14 +107,14 @@ mod test { let mut uuid = Uuid::new(0x0102); uuid.0[0] = 1; - let res: Result<Uuid16Builder, _> = uuid.try_into(); + let res: Result<att::Uuid16, _> = uuid.try_into(); assert!(res.is_err()); } #[test] fn test_uuid128_builder() { let uuid = Uuid::new(0x01020304); - let builder: Uuid128Builder = uuid.into(); + let builder: att::Uuid128 = uuid.into(); assert_eq!(builder.data[..12], BASE_UUID.to_le_bytes()[..12]); assert_eq!(builder.data[12..], [4, 3, 2, 1]); } @@ -133,7 +122,7 @@ mod test { #[test] fn test_uuid_builder() { let uuid = Uuid::new(0x01020304); - let builder: UuidBuilder = uuid.into(); + let builder: att::Uuid = uuid.into(); assert_eq!(builder.data[..12], BASE_UUID.to_le_bytes()[..12]); assert_eq!(builder.data[12..], [4, 3, 2, 1]); } @@ -141,7 +130,7 @@ mod test { #[test] fn test_uuid_from_16_fixed_view() { let expected = Uuid::new(0x0102); - let actual: Uuid = build_view_or_crash(Uuid16Builder { data: 0x0102 }).view().into(); + let actual: Uuid = att::Uuid16 { data: 0x0102 }.try_into().unwrap(); assert_eq!(expected, actual); } @@ -149,25 +138,21 @@ mod test { fn test_uuid_from_128_fixed_view() { let data = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]; let expected = Uuid::new_from_le_bytes(data); - let actual: Uuid = build_view_or_crash(Uuid128Builder { data: data.into() }).view().into(); + let actual: Uuid = att::Uuid128 { data }.try_into().unwrap(); assert_eq!(expected, actual); } #[test] fn test_uuid_from_16_view() { let expected = Uuid::new(0x0102); - let actual: Uuid = - build_view_or_crash(UuidBuilder { data: [2, 1].into() }).view().try_into().unwrap(); + let actual: Uuid = att::Uuid { data: vec![2, 1] }.try_into().unwrap(); assert_eq!(expected, actual); } #[test] fn test_uuid_from_32_view() { let expected = Uuid::new(0x01020304); - let actual: Uuid = build_view_or_crash(UuidBuilder { data: [4, 3, 2, 1].into() }) - .view() - .try_into() - .unwrap(); + let actual: Uuid = att::Uuid { data: vec![4, 3, 2, 1] }.try_into().unwrap(); assert_eq!(expected, actual); } @@ -175,16 +160,14 @@ mod test { fn test_uuid_from_128_view() { let data = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]; let expected = Uuid::new_from_le_bytes(data); - let actual: Uuid = - build_view_or_crash(UuidBuilder { data: data.into() }).view().try_into().unwrap(); + let actual: Uuid = att::Uuid { data: data.into() }.try_into().unwrap(); assert_eq!(expected, actual); } #[test] fn test_uuid_from_invalid_view() { - let packet = - build_view_or_crash(UuidBuilder { data: [10, 9, 8, 7, 6, 5, 4, 3, 2, 1].into() }); - let res = Uuid::try_from(packet.view()); + let packet = att::Uuid { data: vec![10, 9, 8, 7, 6, 5, 4, 3, 2, 1] }; + let res = Uuid::try_from(packet); assert!(res.is_err()); } } diff --git a/system/rust/src/gatt/arbiter.rs b/system/rust/src/gatt/arbiter.rs index 77a3867d02..f25250043b 100644 --- a/system/rust/src/gatt/arbiter.rs +++ b/system/rust/src/gatt/arbiter.rs @@ -1,15 +1,13 @@ //! This module handles "arbitration" of ATT packets, to determine whether they //! should be handled by the primary stack or by the Rust stack +use pdl_runtime::Packet; use std::sync::{Arc, Mutex}; use log::{error, trace, warn}; use std::sync::RwLock; -use crate::{ - do_in_rust_thread, - packets::{AttOpcode, OwnedAttView, OwnedPacket}, -}; +use crate::{do_in_rust_thread, packets::att}; use super::{ ffi::{InterceptAction, StoreCallbacksFromRust}, @@ -62,19 +60,19 @@ pub fn has_arbiter() -> bool { fn try_parse_att_server_packet( isolation_manager: &IsolationManager, tcb_idx: TransportIndex, - packet: Box<[u8]>, -) -> Option<OwnedAttView> { + packet: &[u8], +) -> Option<att::Att> { isolation_manager.get_server_id(tcb_idx)?; - let att = OwnedAttView::try_parse(packet).ok()?; + let att = att::Att::decode_full(packet).ok()?; - if att.view().get_opcode() == AttOpcode::EXCHANGE_MTU_REQUEST { + if att.opcode == att::AttOpcode::ExchangeMtuRequest { // special case: this server opcode is handled by legacy stack, and we snoop // on its handling, since the MTU is shared between the client + server return None; } - match classify_opcode(att.view().get_opcode()) { + match classify_opcode(att.opcode) { OperationType::Command | OperationType::Request | OperationType::Confirmation => Some(att), _ => None, } @@ -123,13 +121,13 @@ fn intercept_packet(tcb_idx: u8, packet: Vec<u8>) -> InterceptAction { } let tcb_idx = TransportIndex(tcb_idx); - if let Some(att) = with_arbiter(|arbiter| { - try_parse_att_server_packet(arbiter, tcb_idx, packet.into_boxed_slice()) - }) { + if let Some(att) = + with_arbiter(|arbiter| try_parse_att_server_packet(arbiter, tcb_idx, &packet)) + { do_in_rust_thread(move |modules| { trace!("pushing packet to GATT"); if let Some(bearer) = modules.gatt_module.get_bearer(tcb_idx) { - bearer.handle_packet(att.view()) + bearer.handle_packet(att) } else { error!("Bearer for {tcb_idx:?} not found"); } @@ -160,10 +158,7 @@ mod test { use crate::{ gatt::ids::{AttHandle, ServerId}, - packets::{ - AttBuilder, AttExchangeMtuRequestBuilder, AttOpcode, AttReadRequestBuilder, - Serializable, - }, + packets::att, }; const TCB_IDX: TransportIndex = TransportIndex(1); @@ -183,15 +178,12 @@ mod test { #[test] fn test_packet_capture_when_isolated() { let isolation_manager = create_manager_with_isolated_connection(TCB_IDX, SERVER_ID); - let packet = AttBuilder { - opcode: AttOpcode::READ_REQUEST, - _child_: AttReadRequestBuilder { attribute_handle: AttHandle(1).into() }.into(), - }; + let packet = att::AttReadRequest { attribute_handle: AttHandle(1).into() }; let out = try_parse_att_server_packet( &isolation_manager, TCB_IDX, - packet.to_vec().unwrap().into(), + &packet.encode_to_vec().unwrap(), ); assert!(out.is_some()); @@ -200,15 +192,16 @@ mod test { #[test] fn test_packet_bypass_when_isolated() { let isolation_manager = create_manager_with_isolated_connection(TCB_IDX, SERVER_ID); - let packet = AttBuilder { - opcode: AttOpcode::ERROR_RESPONSE, - _child_: AttReadRequestBuilder { attribute_handle: AttHandle(1).into() }.into(), + let packet = att::AttErrorResponse { + opcode_in_error: att::AttOpcode::ReadResponse, + handle_in_error: AttHandle(1).into(), + error_code: att::AttErrorCode::InvalidHandle, }; let out = try_parse_att_server_packet( &isolation_manager, TCB_IDX, - packet.to_vec().unwrap().into(), + &packet.encode_to_vec().unwrap(), ); assert!(out.is_none()); @@ -217,15 +210,12 @@ mod test { #[test] fn test_mtu_bypass() { let isolation_manager = create_manager_with_isolated_connection(TCB_IDX, SERVER_ID); - let packet = AttBuilder { - opcode: AttOpcode::EXCHANGE_MTU_REQUEST, - _child_: AttExchangeMtuRequestBuilder { mtu: 64 }.into(), - }; + let packet = att::AttExchangeMtuRequest { mtu: 64 }; let out = try_parse_att_server_packet( &isolation_manager, TCB_IDX, - packet.to_vec().unwrap().into(), + &packet.encode_to_vec().unwrap(), ); assert!(out.is_none()); @@ -234,15 +224,12 @@ mod test { #[test] fn test_packet_bypass_when_not_isolated() { let isolation_manager = IsolationManager::new(); - let packet = AttBuilder { - opcode: AttOpcode::READ_REQUEST, - _child_: AttReadRequestBuilder { attribute_handle: AttHandle(1).into() }.into(), - }; + let packet = att::AttReadRequest { attribute_handle: AttHandle(1).into() }; let out = try_parse_att_server_packet( &isolation_manager, TCB_IDX, - packet.to_vec().unwrap().into(), + &packet.encode_to_vec().unwrap(), ); assert!(out.is_none()); diff --git a/system/rust/src/gatt/callbacks.rs b/system/rust/src/gatt/callbacks.rs index 2d6a58cbdd..377d5a81f6 100644 --- a/system/rust/src/gatt/callbacks.rs +++ b/system/rust/src/gatt/callbacks.rs @@ -9,7 +9,7 @@ pub use callback_transaction_manager::{CallbackResponseError, CallbackTransactio use async_trait::async_trait; use log::warn; -use crate::packets::AttErrorCode; +use crate::packets::att::AttErrorCode; use super::{ ffi::AttributeBackingType, @@ -166,7 +166,7 @@ impl<T: GattDatastore + ?Sized> RawGattDatastore for T { ) -> Result<Vec<u8>, AttErrorCode> { if offset != 0 { warn!("got read blob request for non-long attribute {handle:?}"); - return Err(AttErrorCode::ATTRIBUTE_NOT_LONG); + return Err(AttErrorCode::AttributeNotLong); } self.read(tcb_idx, handle, attr_type).await } @@ -183,7 +183,7 @@ impl<T: GattDatastore + ?Sized> RawGattDatastore for T { match write_type { GattWriteRequestType::Prepare { .. } => { warn!("got prepare write attempt on {tcb_idx:?} to characteristic {handle:?} not supporting write_without_response"); - Err(AttErrorCode::WRITE_REQUEST_REJECTED) + Err(AttErrorCode::WriteRequestRejected) } GattWriteRequestType::Request => self.write(tcb_idx, handle, attr_type, data).await, } @@ -270,10 +270,10 @@ mod test { let MockDatastoreEvents::Read(_, _, _, resp) = resp else { unreachable!(); }; - resp.send(Err(AttErrorCode::APPLICATION_ERROR)).unwrap(); + resp.send(Err(AttErrorCode::ApplicationError)).unwrap(); // assert: got the supplied response - assert_eq!(pending.await.unwrap(), Err(AttErrorCode::APPLICATION_ERROR)); + assert_eq!(pending.await.unwrap(), Err(AttErrorCode::ApplicationError)); }); } @@ -292,7 +292,7 @@ mod test { )); // assert: got the correct error code - assert_eq!(resp, Err(AttErrorCode::ATTRIBUTE_NOT_LONG)); + assert_eq!(resp, Err(AttErrorCode::AttributeNotLong)); // assert: no pending events assert_eq!(rx.try_recv().unwrap_err(), TryRecvError::Empty); } @@ -353,10 +353,10 @@ mod test { let MockDatastoreEvents::Write(_, _, _, _, resp) = resp else { unreachable!(); }; - resp.send(Err(AttErrorCode::APPLICATION_ERROR)).unwrap(); + resp.send(Err(AttErrorCode::ApplicationError)).unwrap(); // assert: got the supplied response - assert_eq!(pending.await.unwrap(), Err(AttErrorCode::APPLICATION_ERROR)); + assert_eq!(pending.await.unwrap(), Err(AttErrorCode::ApplicationError)); }); } @@ -376,7 +376,7 @@ mod test { )); // assert: got the correct error code - assert_eq!(resp, Err(AttErrorCode::WRITE_REQUEST_REJECTED)); + assert_eq!(resp, Err(AttErrorCode::WriteRequestRejected)); // assert: no event sent up assert_eq!(rx.try_recv().unwrap_err(), TryRecvError::Empty); } diff --git a/system/rust/src/gatt/callbacks/callback_transaction_manager.rs b/system/rust/src/gatt/callbacks/callback_transaction_manager.rs index 4210d64535..f877f17d81 100644 --- a/system/rust/src/gatt/callbacks/callback_transaction_manager.rs +++ b/system/rust/src/gatt/callbacks/callback_transaction_manager.rs @@ -9,7 +9,7 @@ use crate::{ ids::{AttHandle, ConnectionId, ServerId, TransactionId, TransportIndex}, GattCallbacks, }, - packets::AttErrorCode, + packets::att::AttErrorCode, }; use super::{ @@ -121,7 +121,7 @@ impl PendingTransactionWatcher { .pending_transactions .remove(&(self.conn_id, self.trans_id)); warn!("no response received from Java after timeout - returning UNLIKELY_ERROR"); - Err(AttErrorCode::UNLIKELY_ERROR) + Err(AttErrorCode::UnlikelyError) } } } diff --git a/system/rust/src/gatt/channel.rs b/system/rust/src/gatt/channel.rs index 6625eb1911..57d1b53b06 100644 --- a/system/rust/src/gatt/channel.rs +++ b/system/rust/src/gatt/channel.rs @@ -1,7 +1,8 @@ //! This represents the TX end of an ATT Transport, to be either mocked (in //! test) or linked to FFI (in production). -use crate::packets::{AttBuilder, SerializeError}; +use crate::packets::att; +use pdl_runtime::EncodeError; use super::ids::TransportIndex; @@ -14,9 +15,5 @@ pub trait AttTransport { /// /// The tcb_idx is an identifier for this transport supplied from the /// native stack, and represents an underlying ACL-LE connection. - fn send_packet( - &self, - tcb_idx: TransportIndex, - packet: AttBuilder, - ) -> Result<(), SerializeError>; + fn send_packet(&self, tcb_idx: TransportIndex, packet: att::Att) -> Result<(), EncodeError>; } diff --git a/system/rust/src/gatt/ffi.rs b/system/rust/src/gatt/ffi.rs index 71007ebaf2..110de114d3 100644 --- a/system/rust/src/gatt/ffi.rs +++ b/system/rust/src/gatt/ffi.rs @@ -1,6 +1,8 @@ //! FFI interfaces for the GATT module. Some structs are exported so that //! core::init can instantiate and pass them into the main loop. +use pdl_runtime::EncodeError; +use pdl_runtime::Packet; use std::iter::Peekable; use anyhow::{bail, Result}; @@ -11,7 +13,7 @@ use tokio::task::spawn_local; use crate::{ do_in_rust_thread, - packets::{AttBuilder, AttErrorCode, Serializable, SerializeError}, + packets::att::{self, AttErrorCode}, }; use super::{ @@ -266,12 +268,8 @@ impl GattCallbacks for GattCallbacksImpl { pub struct AttTransportImpl(); impl AttTransport for AttTransportImpl { - fn send_packet( - &self, - tcb_idx: TransportIndex, - packet: AttBuilder, - ) -> Result<(), SerializeError> { - SendPacketToPeer(tcb_idx.0, packet.to_vec()?); + fn send_packet(&self, tcb_idx: TransportIndex, packet: att::Att) -> Result<(), EncodeError> { + SendPacketToPeer(tcb_idx.0, packet.encode_to_vec()?); Ok(()) } } @@ -418,7 +416,7 @@ fn send_response(_server_id: u8, conn_id: u16, trans_id: u32, status: u8, value: let value = if status == 0 { Ok(value.to_vec()) } else { - Err(AttErrorCode::try_from(status).unwrap_or(AttErrorCode::UNLIKELY_ERROR)) + Err(AttErrorCode::try_from(status).unwrap_or(AttErrorCode::UnlikelyError)) }; trace!("send_response {conn_id:?}, {trans_id:?}, {:?}", value.as_ref().err()); @@ -438,7 +436,7 @@ fn send_response(_server_id: u8, conn_id: u16, trans_id: u32, status: u8, value: fn send_indication(_server_id: u8, handle: u16, conn_id: u16, value: &[u8]) { let handle = AttHandle(handle); let conn_id = ConnectionId(conn_id); - let value = value.into(); + let value = value.to_vec(); trace!("send_indication {handle:?}, {conn_id:?}"); diff --git a/system/rust/src/gatt/mocks/mock_datastore.rs b/system/rust/src/gatt/mocks/mock_datastore.rs index 901fe75565..0e024efd91 100644 --- a/system/rust/src/gatt/mocks/mock_datastore.rs +++ b/system/rust/src/gatt/mocks/mock_datastore.rs @@ -6,7 +6,7 @@ use crate::{ ffi::AttributeBackingType, ids::{AttHandle, TransportIndex}, }, - packets::AttErrorCode, + packets::att::AttErrorCode, }; use async_trait::async_trait; use log::info; diff --git a/system/rust/src/gatt/mocks/mock_raw_datastore.rs b/system/rust/src/gatt/mocks/mock_raw_datastore.rs index 0deb220d28..c43b47d586 100644 --- a/system/rust/src/gatt/mocks/mock_raw_datastore.rs +++ b/system/rust/src/gatt/mocks/mock_raw_datastore.rs @@ -6,7 +6,7 @@ use crate::{ ffi::AttributeBackingType, ids::{AttHandle, TransportIndex}, }, - packets::AttErrorCode, + packets::att::AttErrorCode, }; use async_trait::async_trait; use log::info; diff --git a/system/rust/src/gatt/mocks/mock_transport.rs b/system/rust/src/gatt/mocks/mock_transport.rs index 9227eeb127..4db740c71a 100644 --- a/system/rust/src/gatt/mocks/mock_transport.rs +++ b/system/rust/src/gatt/mocks/mock_transport.rs @@ -2,29 +2,27 @@ use crate::{ gatt::{channel::AttTransport, ids::TransportIndex}, - packets::{AttBuilder, Serializable, SerializeError}, + packets::att, }; +use pdl_runtime::EncodeError; +use pdl_runtime::Packet; use tokio::sync::mpsc::{self, unbounded_channel, UnboundedReceiver}; /// Routes calls to AttTransport into a channel containing AttBuilders -pub struct MockAttTransport(mpsc::UnboundedSender<(TransportIndex, AttBuilder)>); +pub struct MockAttTransport(mpsc::UnboundedSender<(TransportIndex, att::Att)>); impl MockAttTransport { /// Constructor. Returns Self and the RX side of a channel containing /// AttBuilders sent on TransportIndices - pub fn new() -> (Self, UnboundedReceiver<(TransportIndex, AttBuilder)>) { + pub fn new() -> (Self, UnboundedReceiver<(TransportIndex, att::Att)>) { let (tx, rx) = unbounded_channel(); (Self(tx), rx) } } impl AttTransport for MockAttTransport { - fn send_packet( - &self, - tcb_idx: TransportIndex, - packet: AttBuilder, - ) -> Result<(), SerializeError> { - packet.to_vec()?; // trigger SerializeError if needed + fn send_packet(&self, tcb_idx: TransportIndex, packet: att::Att) -> Result<(), EncodeError> { + packet.encode_to_vec()?; // trigger SerializeError if needed self.0.send((tcb_idx, packet)).unwrap(); Ok(()) } diff --git a/system/rust/src/gatt/opcode_types.rs b/system/rust/src/gatt/opcode_types.rs index 0c932eb886..d92da99ea4 100644 --- a/system/rust/src/gatt/opcode_types.rs +++ b/system/rust/src/gatt/opcode_types.rs @@ -1,6 +1,6 @@ //! This module lets us classify AttOpcodes to determine how to handle them -use crate::packets::AttOpcode; +use crate::packets::att::AttOpcode; /// The type of ATT operation performed by the packet /// (see Core Spec 5.3 Vol 3F 3.3 Attribute PDU for details) @@ -23,40 +23,40 @@ pub enum OperationType { /// bitmasking, but is done explicitly for clarity. pub fn classify_opcode(opcode: AttOpcode) -> OperationType { match opcode { - AttOpcode::ERROR_RESPONSE => OperationType::Response, - AttOpcode::EXCHANGE_MTU_RESPONSE => OperationType::Response, - AttOpcode::FIND_INFORMATION_RESPONSE => OperationType::Response, - AttOpcode::FIND_BY_TYPE_VALUE_RESPONSE => OperationType::Response, - AttOpcode::READ_BY_TYPE_RESPONSE => OperationType::Response, - AttOpcode::READ_RESPONSE => OperationType::Response, - AttOpcode::READ_BLOB_RESPONSE => OperationType::Response, - AttOpcode::READ_MULTIPLE_RESPONSE => OperationType::Response, - AttOpcode::READ_BY_GROUP_TYPE_RESPONSE => OperationType::Response, - AttOpcode::WRITE_RESPONSE => OperationType::Response, - AttOpcode::PREPARE_WRITE_RESPONSE => OperationType::Response, - AttOpcode::EXECUTE_WRITE_RESPONSE => OperationType::Response, - AttOpcode::READ_MULTIPLE_VARIABLE_RESPONSE => OperationType::Response, - - AttOpcode::EXCHANGE_MTU_REQUEST => OperationType::Request, - AttOpcode::FIND_INFORMATION_REQUEST => OperationType::Request, - AttOpcode::FIND_BY_TYPE_VALUE_REQUEST => OperationType::Request, - AttOpcode::READ_BY_TYPE_REQUEST => OperationType::Request, - AttOpcode::READ_REQUEST => OperationType::Request, - AttOpcode::READ_BLOB_REQUEST => OperationType::Request, - AttOpcode::READ_MULTIPLE_REQUEST => OperationType::Request, - AttOpcode::READ_BY_GROUP_TYPE_REQUEST => OperationType::Request, - AttOpcode::WRITE_REQUEST => OperationType::Request, - AttOpcode::PREPARE_WRITE_REQUEST => OperationType::Request, - AttOpcode::EXECUTE_WRITE_REQUEST => OperationType::Request, - AttOpcode::READ_MULTIPLE_VARIABLE_REQUEST => OperationType::Request, - - AttOpcode::WRITE_COMMAND => OperationType::Command, - AttOpcode::SIGNED_WRITE_COMMAND => OperationType::Command, - - AttOpcode::HANDLE_VALUE_NOTIFICATION => OperationType::Notification, - - AttOpcode::HANDLE_VALUE_INDICATION => OperationType::Indication, - - AttOpcode::HANDLE_VALUE_CONFIRMATION => OperationType::Confirmation, + AttOpcode::ErrorResponse => OperationType::Response, + AttOpcode::ExchangeMtuResponse => OperationType::Response, + AttOpcode::FindInformationResponse => OperationType::Response, + AttOpcode::FindByTypeValueResponse => OperationType::Response, + AttOpcode::ReadByTypeResponse => OperationType::Response, + AttOpcode::ReadResponse => OperationType::Response, + AttOpcode::ReadBlobResponse => OperationType::Response, + AttOpcode::ReadMultipleResponse => OperationType::Response, + AttOpcode::ReadByGroupTypeResponse => OperationType::Response, + AttOpcode::WriteResponse => OperationType::Response, + AttOpcode::PrepareWriteResponse => OperationType::Response, + AttOpcode::ExecuteWriteResponse => OperationType::Response, + AttOpcode::ReadMultipleVariableResponse => OperationType::Response, + + AttOpcode::ExchangeMtuRequest => OperationType::Request, + AttOpcode::FindInformationRequest => OperationType::Request, + AttOpcode::FindByTypeValueRequest => OperationType::Request, + AttOpcode::ReadByTypeRequest => OperationType::Request, + AttOpcode::ReadRequest => OperationType::Request, + AttOpcode::ReadBlobRequest => OperationType::Request, + AttOpcode::ReadMultipleRequest => OperationType::Request, + AttOpcode::ReadByGroupTypeRequest => OperationType::Request, + AttOpcode::WriteRequest => OperationType::Request, + AttOpcode::PrepareWriteRequest => OperationType::Request, + AttOpcode::ExecuteWriteRequest => OperationType::Request, + AttOpcode::ReadMultipleVariableRequest => OperationType::Request, + + AttOpcode::WriteCommand => OperationType::Command, + AttOpcode::SignedWriteCommand => OperationType::Command, + + AttOpcode::HandleValueNotification => OperationType::Notification, + + AttOpcode::HandleValueIndication => OperationType::Indication, + + AttOpcode::HandleValueConfirmation => OperationType::Confirmation, } } diff --git a/system/rust/src/gatt/server/att_database.rs b/system/rust/src/gatt/server/att_database.rs index 47e67270a2..cd322c6f8c 100644 --- a/system/rust/src/gatt/server/att_database.rs +++ b/system/rust/src/gatt/server/att_database.rs @@ -4,18 +4,18 @@ use bitflags::bitflags; use crate::{ core::uuid::Uuid, gatt::ids::AttHandle, - packets::{AttErrorCode, AttHandleBuilder, AttHandleView}, + packets::att::{self, AttErrorCode}, }; -impl From<AttHandleView<'_>> for AttHandle { - fn from(value: AttHandleView) -> Self { - AttHandle(value.get_handle()) +impl From<att::AttHandle> for AttHandle { + fn from(value: att::AttHandle) -> Self { + AttHandle(value.handle) } } -impl From<AttHandle> for AttHandleBuilder { +impl From<AttHandle> for att::AttHandle { fn from(value: AttHandle) -> Self { - AttHandleBuilder { handle: value.0 } + att::AttHandle { handle: value.0 } } } diff --git a/system/rust/src/gatt/server/att_server_bearer.rs b/system/rust/src/gatt/server/att_server_bearer.rs index b6ba9c5ebb..0d78ad1278 100644 --- a/system/rust/src/gatt/server/att_server_bearer.rs +++ b/system/rust/src/gatt/server/att_server_bearer.rs @@ -2,6 +2,7 @@ //! It handles ATT transactions and unacknowledged operations, backed by an //! AttDatabase (that may in turn be backed by an upper-layer protocol) +use pdl_runtime::EncodeError; use std::{cell::Cell, future::Future}; use anyhow::Result; @@ -18,11 +19,8 @@ use crate::{ mtu::{AttMtu, MtuEvent}, opcode_types::{classify_opcode, OperationType}, }, - packets::{ - AttBuilder, AttChild, AttErrorCode, AttErrorResponseBuilder, AttView, Packet, - SerializeError, - }, - utils::{owned_handle::OwnedHandle, packet::HACK_child_to_opcode}, + packets::att::{self, AttErrorCode}, + utils::owned_handle::OwnedHandle, }; use super::{ @@ -42,7 +40,7 @@ enum AttRequestState<T: AttDatabase> { #[derive(Debug)] pub enum SendError { /// The packet failed to serialize - SerializeError(SerializeError), + SerializeError(EncodeError), /// The connection no longer exists ConnectionDropped, } @@ -52,7 +50,7 @@ pub enum SendError { /// take place at a time pub struct AttServerBearer<T: AttDatabase> { // general - send_packet: Box<dyn Fn(AttBuilder) -> Result<(), SerializeError>>, + send_packet: Box<dyn Fn(att::Att) -> Result<(), EncodeError>>, mtu: AttMtu, // request state @@ -69,10 +67,7 @@ pub struct AttServerBearer<T: AttDatabase> { impl<T: AttDatabase + Clone + 'static> AttServerBearer<T> { /// Constructor, wrapping an ATT channel (for outgoing packets) and an /// AttDatabase - pub fn new( - db: T, - send_packet: impl Fn(AttBuilder) -> Result<(), SerializeError> + 'static, - ) -> Self { + pub fn new(db: T, send_packet: impl Fn(att::Att) -> Result<(), EncodeError> + 'static) -> Self { let (indication_handler, pending_confirmation) = IndicationHandler::new(db.clone()); Self { send_packet: Box::new(send_packet), @@ -87,9 +82,7 @@ impl<T: AttDatabase + Clone + 'static> AttServerBearer<T> { } } - fn send_packet(&self, packet: impl Into<AttChild>) -> Result<(), SerializeError> { - let child = packet.into(); - let packet = AttBuilder { opcode: HACK_child_to_opcode(&child), _child_: child }; + fn send_packet(&self, packet: att::Att) -> Result<(), EncodeError> { (self.send_packet)(packet) } } @@ -97,8 +90,8 @@ impl<T: AttDatabase + Clone + 'static> AttServerBearer<T> { impl<T: AttDatabase + Clone + 'static> WeakBoxRef<'_, AttServerBearer<T>> { /// Handle an incoming packet, and send outgoing packets as appropriate /// using the owned ATT channel. - pub fn handle_packet(&self, packet: AttView<'_>) { - match classify_opcode(packet.get_opcode()) { + pub fn handle_packet(&self, packet: att::Att) { + match classify_opcode(packet.opcode) { OperationType::Command => { self.command_handler.process_packet(packet); } @@ -153,18 +146,18 @@ impl<T: AttDatabase + Clone + 'static> WeakBoxRef<'_, AttServerBearer<T>> { self.mtu.handle_event(mtu_event) } - fn handle_request(&self, packet: AttView<'_>) { + fn handle_request(&self, packet: att::Att) { let curr_request = self.curr_request.replace(AttRequestState::Replacing); self.curr_request.replace(match curr_request { AttRequestState::Idle(mut request_handler) => { // even if the MTU is updated afterwards, 5.3 3F 3.4.2.2 states that the // request-time MTU should be used let mtu = self.mtu.snapshot_or_default(); - let packet = packet.to_owned_packet(); let this = self.downgrade(); + let opcode = packet.opcode; let task = spawn_local(async move { trace!("starting ATT transaction"); - let reply = request_handler.process_packet(packet.view(), mtu).await; + let reply = request_handler.process_packet(packet, mtu).await; this.with(|this| { this.map(|this| { match this.send_packet(reply) { @@ -174,11 +167,11 @@ impl<T: AttDatabase + Clone + 'static> WeakBoxRef<'_, AttServerBearer<T>> { Err(err) => { error!("serializer failure {err:?}, dropping packet and sending failed reply"); // if this also fails, we're stuck - if let Err(err) = this.send_packet(AttErrorResponseBuilder { - opcode_in_error: packet.view().get_opcode(), + if let Err(err) = this.send_packet(att::AttErrorResponse { + opcode_in_error: opcode, handle_in_error: AttHandle(0).into(), - error_code: AttErrorCode::UNLIKELY_ERROR, - }) { + error_code: AttErrorCode::UnlikelyError, + }.try_into().unwrap()) { panic!("unexpected serialize error for known-good packet {err:?}") } } @@ -203,7 +196,7 @@ impl<T: AttDatabase + Clone + 'static> WeakBoxRef<'_, AttServerBearer<T>> { } impl<T: AttDatabase + Clone + 'static> WeakBox<AttServerBearer<T>> { - fn try_send_packet(&self, packet: impl Into<AttChild>) -> Result<(), SendError> { + fn try_send_packet(&self, packet: att::Att) -> Result<(), SendError> { self.with(|this| { this.ok_or_else(|| { warn!("connection dropped before packet sent"); @@ -237,14 +230,8 @@ mod test { test::test_att_db::TestAttDatabase, }, }, - packets::{ - AttHandleValueConfirmationBuilder, AttOpcode, AttReadRequestBuilder, - AttReadResponseBuilder, - }, - utils::{ - packet::build_att_view_or_crash, - task::{block_on_locally, try_await}, - }, + packets::att, + utils::task::{block_on_locally, try_await}, }; const VALID_HANDLE: AttHandle = AttHandle(3); @@ -254,7 +241,7 @@ mod test { const TCB_IDX: TransportIndex = TransportIndex(1); fn open_connection( - ) -> (SharedBox<AttServerBearer<TestAttDatabase>>, UnboundedReceiver<AttBuilder>) { + ) -> (SharedBox<AttServerBearer<TestAttDatabase>>, UnboundedReceiver<att::Att>) { let db = TestAttDatabase::new(vec![ ( AttAttribute { @@ -287,12 +274,9 @@ mod test { block_on_locally(async { let (conn, mut rx) = open_connection(); conn.as_ref().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: VALID_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: VALID_HANDLE.into() }.try_into().unwrap(), ); - assert_eq!(rx.recv().await.unwrap().opcode, AttOpcode::READ_RESPONSE); + assert_eq!(rx.recv().await.unwrap().opcode, att::AttOpcode::ReadResponse); assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); }); } @@ -302,21 +286,15 @@ mod test { block_on_locally(async { let (conn, mut rx) = open_connection(); conn.as_ref().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: INVALID_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: INVALID_HANDLE.into() }.try_into().unwrap(), ); - assert_eq!(rx.recv().await.unwrap().opcode, AttOpcode::ERROR_RESPONSE); + assert_eq!(rx.recv().await.unwrap().opcode, att::AttOpcode::ErrorResponse); assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); conn.as_ref().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: VALID_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: VALID_HANDLE.into() }.try_into().unwrap(), ); - assert_eq!(rx.recv().await.unwrap().opcode, AttOpcode::READ_RESPONSE); + assert_eq!(rx.recv().await.unwrap().opcode, att::AttOpcode::ReadResponse); assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); }); } @@ -361,15 +339,11 @@ mod test { // act: send two read requests before replying to either read // first request block_on_locally(async { - let req1 = build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: VALID_HANDLE.into(), - }); - conn.as_ref().handle_packet(req1.view()); + let req1 = att::AttReadRequest { attribute_handle: VALID_HANDLE.into() }; + conn.as_ref().handle_packet(req1.try_into().unwrap()); // second request - let req2 = build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: ANOTHER_VALID_HANDLE.into(), - }); - conn.as_ref().handle_packet(req2.view()); + let req2 = att::AttReadRequest { attribute_handle: ANOTHER_VALID_HANDLE.into() }; + conn.as_ref().handle_packet(req2.try_into().unwrap()); // handle first reply let MockDatastoreEvents::Read( TCB_IDX, @@ -385,13 +359,7 @@ mod test { // assert: that the first reply was made let resp = rx.recv().await.unwrap(); - assert_eq!( - resp, - AttBuilder { - opcode: AttOpcode::READ_RESPONSE, - _child_: AttReadResponseBuilder { value: data.into() }.into() - } - ); + assert_eq!(resp, att::AttReadResponse { value: data.to_vec() }.try_into().unwrap()); // assert no other replies were made assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); // assert no callbacks are pending @@ -408,12 +376,10 @@ mod test { // act: send an indication let pending_send = spawn_local(conn.as_ref().send_indication(VALID_HANDLE, vec![1, 2, 3])); - assert_eq!(rx.recv().await.unwrap().opcode, AttOpcode::HANDLE_VALUE_INDICATION); + assert_eq!(rx.recv().await.unwrap().opcode, att::AttOpcode::HandleValueIndication); assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); // and the confirmation - conn.as_ref().handle_packet( - build_att_view_or_crash(AttHandleValueConfirmationBuilder {}).view(), - ); + conn.as_ref().handle_packet(att::AttHandleValueConfirmation {}.try_into().unwrap()); // assert: the indication was correctly sent assert!(matches!(pending_send.await.unwrap(), Ok(()))); @@ -432,22 +398,18 @@ mod test { // wait for/capture the outgoing packet let sent1 = rx.recv().await.unwrap(); // send the response - conn.as_ref().handle_packet( - build_att_view_or_crash(AttHandleValueConfirmationBuilder {}).view(), - ); + conn.as_ref().handle_packet(att::AttHandleValueConfirmation {}.try_into().unwrap()); // send the second indication let pending_send2 = spawn_local(conn.as_ref().send_indication(VALID_HANDLE, vec![1, 2, 3])); // wait for/capture the outgoing packet let sent2 = rx.recv().await.unwrap(); // and the response - conn.as_ref().handle_packet( - build_att_view_or_crash(AttHandleValueConfirmationBuilder {}).view(), - ); + conn.as_ref().handle_packet(att::AttHandleValueConfirmation {}.try_into().unwrap()); // assert: exactly two indications were sent - assert_eq!(sent1.opcode, AttOpcode::HANDLE_VALUE_INDICATION); - assert_eq!(sent2.opcode, AttOpcode::HANDLE_VALUE_INDICATION); + assert_eq!(sent1.opcode, att::AttOpcode::HandleValueIndication); + assert_eq!(sent2.opcode, att::AttOpcode::HandleValueIndication); assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); // and that both got successful responses assert!(matches!(pending_send1.await.unwrap(), Ok(()))); @@ -467,7 +429,7 @@ mod test { let pending_send2 = spawn_local(conn.as_ref().send_indication(ANOTHER_VALID_HANDLE, vec![1, 2, 3])); // assert: only one was initially sent - assert_eq!(rx.recv().await.unwrap().opcode, AttOpcode::HANDLE_VALUE_INDICATION); + assert_eq!(rx.recv().await.unwrap().opcode, att::AttOpcode::HandleValueIndication); assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); // and both are still pending assert!(!pending_send1.is_finished()); @@ -489,9 +451,7 @@ mod test { // wait for/capture the outgoing packet let sent1 = rx.recv().await.unwrap(); // send response for the first one - conn.as_ref().handle_packet( - build_att_view_or_crash(AttHandleValueConfirmationBuilder {}).view(), - ); + conn.as_ref().handle_packet(att::AttHandleValueConfirmation {}.try_into().unwrap()); // wait for/capture the outgoing packet let sent2 = rx.recv().await.unwrap(); @@ -500,8 +460,8 @@ mod test { assert!(matches!(pending_send1.await.unwrap(), Ok(()))); assert!(!pending_send2.is_finished()); // and that both indications have been sent - assert_eq!(sent1.opcode, AttOpcode::HANDLE_VALUE_INDICATION); - assert_eq!(sent2.opcode, AttOpcode::HANDLE_VALUE_INDICATION); + assert_eq!(sent1.opcode, att::AttOpcode::HandleValueIndication); + assert_eq!(sent2.opcode, att::AttOpcode::HandleValueIndication); assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); }); } @@ -520,22 +480,18 @@ mod test { // wait for/capture the outgoing packet let sent1 = rx.recv().await.unwrap(); // send response for the first one - conn.as_ref().handle_packet( - build_att_view_or_crash(AttHandleValueConfirmationBuilder {}).view(), - ); + conn.as_ref().handle_packet(att::AttHandleValueConfirmation {}.try_into().unwrap()); // wait for/capture the outgoing packet let sent2 = rx.recv().await.unwrap(); // and now the second - conn.as_ref().handle_packet( - build_att_view_or_crash(AttHandleValueConfirmationBuilder {}).view(), - ); + conn.as_ref().handle_packet(att::AttHandleValueConfirmation {}.try_into().unwrap()); // assert: both futures have completed successfully assert!(matches!(pending_send1.await.unwrap(), Ok(()))); assert!(matches!(pending_send2.await.unwrap(), Ok(()))); // and both indications have been sent - assert_eq!(sent1.opcode, AttOpcode::HANDLE_VALUE_INDICATION); - assert_eq!(sent2.opcode, AttOpcode::HANDLE_VALUE_INDICATION); + assert_eq!(sent1.opcode, att::AttOpcode::HandleValueIndication); + assert_eq!(sent2.opcode, att::AttOpcode::HandleValueIndication); assert_eq!(rx.try_recv(), Err(TryRecvError::Empty)); }); } @@ -573,7 +529,7 @@ mod test { conn.as_ref().handle_mtu_event(MtuEvent::IncomingResponse(100)).unwrap(); // assert: the indication was sent - assert_eq!(rx.recv().await.unwrap().opcode, AttOpcode::HANDLE_VALUE_INDICATION); + assert_eq!(rx.recv().await.unwrap().opcode, att::AttOpcode::HandleValueIndication); }); } @@ -606,14 +562,11 @@ mod test { // act: send server packet conn.as_ref().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: VALID_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: VALID_HANDLE.into() }.try_into().unwrap(), ); // assert: that we reply even while the MTU req is outstanding - assert_eq!(rx.recv().await.unwrap().opcode, AttOpcode::READ_RESPONSE); + assert_eq!(rx.recv().await.unwrap().opcode, att::AttOpcode::ReadResponse); }); } @@ -631,12 +584,10 @@ mod test { conn.as_ref().handle_mtu_event(MtuEvent::OutgoingRequest).unwrap(); conn.as_ref().handle_mtu_event(MtuEvent::IncomingResponse(512)).unwrap(); // finally resolve the first indication, so the second indication can be sent - conn.as_ref().handle_packet( - build_att_view_or_crash(AttHandleValueConfirmationBuilder {}).view(), - ); + conn.as_ref().handle_packet(att::AttHandleValueConfirmation {}.try_into().unwrap()); // assert: the second indication successfully sent (so it used the new MTU) - assert_eq!(rx.recv().await.unwrap().opcode, AttOpcode::HANDLE_VALUE_INDICATION); + assert_eq!(rx.recv().await.unwrap().opcode, att::AttOpcode::HandleValueIndication); }); } } diff --git a/system/rust/src/gatt/server/command_handler.rs b/system/rust/src/gatt/server/command_handler.rs index 763c68f7d0..da54bb1ad8 100644 --- a/system/rust/src/gatt/server/command_handler.rs +++ b/system/rust/src/gatt/server/command_handler.rs @@ -1,6 +1,6 @@ use log::warn; -use crate::packets::{AttOpcode, AttView, AttWriteCommandView, Packet}; +use crate::packets::att; use super::att_database::AttDatabase; @@ -14,21 +14,18 @@ impl<Db: AttDatabase> AttCommandHandler<Db> { Self { db } } - pub fn process_packet(&self, packet: AttView<'_>) { + pub fn process_packet(&self, packet: att::Att) { let snapshotted_db = self.db.snapshot(); - match packet.get_opcode() { - AttOpcode::WRITE_COMMAND => { - let Ok(packet) = AttWriteCommandView::try_parse(packet) else { + match packet.opcode { + att::AttOpcode::WriteCommand => { + let Ok(packet) = att::AttWriteCommand::try_from(packet) else { warn!("failed to parse WRITE_COMMAND packet"); return; }; - snapshotted_db.write_no_response_attribute( - packet.get_handle().into(), - &packet.get_value_iter().collect::<Vec<_>>(), - ); + snapshotted_db.write_no_response_attribute(packet.handle.into(), &packet.value); } _ => { - warn!("Dropping unsupported opcode {:?}", packet.get_opcode()); + warn!("Dropping unsupported opcode {:?}", packet.opcode); } } } @@ -47,8 +44,8 @@ mod test { test::test_att_db::TestAttDatabase, }, }, - packets::{AttErrorCode, AttErrorResponseBuilder, AttOpcode, AttWriteCommandBuilder}, - utils::{packet::build_att_view_or_crash, task::block_on_locally}, + packets::att, + utils::task::block_on_locally, }; #[test] @@ -66,11 +63,10 @@ mod test { let data = [1, 2]; // act: send write command - let att_view = build_att_view_or_crash(AttWriteCommandBuilder { - handle: AttHandle(3).into(), - value: data.into(), - }); - handler.process_packet(att_view.view()); + let att_view = att::AttWriteCommand { handle: AttHandle(3).into(), value: data.to_vec() } + .try_into() + .unwrap(); + handler.process_packet(att_view); // assert: the db has been updated assert_eq!(block_on_locally(db.read_attribute(AttHandle(3))).unwrap(), data); @@ -83,12 +79,14 @@ mod test { let handler = AttCommandHandler { db }; // act: send a packet that should not be handled here - let att_view = build_att_view_or_crash(AttErrorResponseBuilder { - opcode_in_error: AttOpcode::EXCHANGE_MTU_REQUEST, + let att_view = att::AttErrorResponse { + opcode_in_error: att::AttOpcode::ExchangeMtuRequest, handle_in_error: AttHandle(1).into(), - error_code: AttErrorCode::UNLIKELY_ERROR, - }); - handler.process_packet(att_view.view()); + error_code: att::AttErrorCode::UnlikelyError, + } + .try_into() + .unwrap(); + handler.process_packet(att_view); // assert: nothing happens (we crash if anything is unhandled within a mock) } diff --git a/system/rust/src/gatt/server/gatt_database.rs b/system/rust/src/gatt/server/gatt_database.rs index b1c688f0a7..d1590b3866 100644 --- a/system/rust/src/gatt/server/gatt_database.rs +++ b/system/rust/src/gatt/server/gatt_database.rs @@ -2,6 +2,7 @@ //! by converting a registry of services into a list of attributes, and proxying //! ATT read/write requests into characteristic reads/writes +use pdl_runtime::Packet; use std::{cell::RefCell, collections::BTreeMap, ops::RangeInclusive, rc::Rc}; use anyhow::{bail, Result}; @@ -18,11 +19,7 @@ use crate::{ ffi::AttributeBackingType, ids::{AttHandle, TransportIndex}, }, - packets::{ - AttErrorCode, GattCharacteristicDeclarationValueBuilder, - GattCharacteristicPropertiesBuilder, GattServiceDeclarationValueBuilder, Serializable, - UuidBuilder, - }, + packets::att::{self, AttErrorCode}, }; use super::{ @@ -186,8 +183,8 @@ impl GattDatabase { permissions: AttPermissions::READABLE, }, AttAttributeBackingValue::Static( - GattServiceDeclarationValueBuilder { uuid: UuidBuilder::from(service.type_) } - .to_vec() + att::GattServiceDeclarationValue { uuid: service.type_.into() } + .encode_to_vec() .map_err(|e| { anyhow::anyhow!("failed to encode primary service declaration: {e:?}") })?, @@ -210,8 +207,8 @@ impl GattDatabase { permissions: AttPermissions::READABLE, }, AttAttributeBackingValue::Static( - GattCharacteristicDeclarationValueBuilder { - properties: GattCharacteristicPropertiesBuilder { + att::GattCharacteristicDeclarationValue { + properties: att::GattCharacteristicProperties { broadcast: 0, read: characteristic.permissions.readable().into(), write_without_response: characteristic @@ -227,7 +224,7 @@ impl GattDatabase { handle: characteristic.handle.into(), uuid: characteristic.type_.into(), } - .to_vec() + .encode_to_vec() .map_err(|e| { anyhow::anyhow!("failed to encode characteristic declaration: {e:?}") })?, @@ -351,14 +348,14 @@ impl AttDatabase for AttDatabaseImpl { let value = self.gatt_db.with(|gatt_db| { let Some(gatt_db) = gatt_db else { // db must have been closed - return Err(AttErrorCode::INVALID_HANDLE); + return Err(AttErrorCode::InvalidHandle); }; let services = gatt_db.schema.borrow(); let Some(attr) = services.attributes.get(&handle) else { - return Err(AttErrorCode::INVALID_HANDLE); + return Err(AttErrorCode::InvalidHandle); }; if !attr.attribute.permissions.readable() { - return Err(AttErrorCode::READ_NOT_PERMITTED); + return Err(AttErrorCode::ReadNotPermitted); } Ok(attr.value.clone()) })?; @@ -392,14 +389,14 @@ impl AttDatabase for AttDatabaseImpl { let value = self.gatt_db.with(|gatt_db| { let Some(gatt_db) = gatt_db else { // db must have been closed - return Err(AttErrorCode::INVALID_HANDLE); + return Err(AttErrorCode::InvalidHandle); }; let services = gatt_db.schema.borrow(); let Some(attr) = services.attributes.get(&handle) else { - return Err(AttErrorCode::INVALID_HANDLE); + return Err(AttErrorCode::InvalidHandle); }; if !attr.attribute.permissions.writable_with_response() { - return Err(AttErrorCode::WRITE_NOT_PERMITTED); + return Err(AttErrorCode::WriteNotPermitted); } Ok(attr.value.clone()) })?; @@ -407,7 +404,7 @@ impl AttDatabase for AttDatabaseImpl { match value { AttAttributeBackingValue::Static(val) => { error!("A static attribute {val:?} is marked as writable - ignoring it and rejecting the write..."); - return Err(AttErrorCode::WRITE_NOT_PERMITTED); + return Err(AttErrorCode::WriteNotPermitted); } AttAttributeBackingValue::DynamicCharacteristic(datastore) => { datastore @@ -521,6 +518,7 @@ mod test { mock_datastore::{MockDatastore, MockDatastoreEvents}, mock_raw_datastore::{MockRawDatastore, MockRawDatastoreEvents}, }, + packets::att, utils::task::block_on_locally, }; @@ -545,7 +543,7 @@ mod test { let resp = tokio_test::block_on(att_db.read_attribute(AttHandle(1))); - assert_eq!(resp, Err(AttErrorCode::INVALID_HANDLE)) + assert_eq!(resp, Err(AttErrorCode::InvalidHandle)) } #[test] @@ -577,9 +575,9 @@ mod test { ); assert_eq!( service_value, - GattServiceDeclarationValueBuilder { uuid: SERVICE_TYPE.into() } - .to_vec() - .map_err(|_| AttErrorCode::UNLIKELY_ERROR) + att::GattServiceDeclarationValue { uuid: SERVICE_TYPE.into() } + .encode_to_vec() + .map_err(|_| AttErrorCode::UnlikelyError) ); } @@ -714,8 +712,8 @@ mod test { assert_eq!( characteristic_decl, - GattCharacteristicDeclarationValueBuilder { - properties: GattCharacteristicPropertiesBuilder { + att::GattCharacteristicDeclarationValue { + properties: att::GattCharacteristicProperties { read: 1, broadcast: 0, write_without_response: 0, @@ -728,8 +726,8 @@ mod test { handle: CHARACTERISTIC_VALUE_HANDLE.into(), uuid: CHARACTERISTIC_TYPE.into() } - .to_vec() - .map_err(|_| AttErrorCode::UNLIKELY_ERROR) + .encode_to_vec() + .map_err(|_| AttErrorCode::UnlikelyError) ); } @@ -762,8 +760,8 @@ mod test { tokio_test::block_on(att_db.read_attribute(CHARACTERISTIC_DECLARATION_HANDLE)); assert_eq!( characteristic_decl, - GattCharacteristicDeclarationValueBuilder { - properties: GattCharacteristicPropertiesBuilder { + att::GattCharacteristicDeclarationValue { + properties: att::GattCharacteristicProperties { read: 1, broadcast: 0, write_without_response: 1, @@ -776,8 +774,8 @@ mod test { handle: CHARACTERISTIC_VALUE_HANDLE.into(), uuid: CHARACTERISTIC_TYPE.into() } - .to_vec() - .map_err(|_| AttErrorCode::UNLIKELY_ERROR) + .encode_to_vec() + .map_err(|_| AttErrorCode::UnlikelyError) ); } @@ -852,7 +850,7 @@ mod test { gatt_db.get_att_database(TCB_IDX).read_attribute(CHARACTERISTIC_VALUE_HANDLE), ); - assert_eq!(characteristic_value, Err(AttErrorCode::READ_NOT_PERMITTED)); + assert_eq!(characteristic_value, Err(AttErrorCode::ReadNotPermitted)); } #[test] @@ -985,7 +983,7 @@ mod test { else { unreachable!(); }; - reply.send(Err(AttErrorCode::UNLIKELY_ERROR)).unwrap(); + reply.send(Err(AttErrorCode::UnlikelyError)).unwrap(); }, att_db.write_attribute(CHARACTERISTIC_VALUE_HANDLE, &data) ) @@ -993,7 +991,7 @@ mod test { }); // assert: the supplied value matches what the att datastore returned - assert_eq!(res, Err(AttErrorCode::UNLIKELY_ERROR)); + assert_eq!(res, Err(AttErrorCode::UnlikelyError)); } #[test] @@ -1021,7 +1019,7 @@ mod test { gatt_db.get_att_database(TCB_IDX).write_attribute(CHARACTERISTIC_VALUE_HANDLE, &data), ); - assert_eq!(characteristic_value, Err(AttErrorCode::WRITE_NOT_PERMITTED)); + assert_eq!(characteristic_value, Err(AttErrorCode::WriteNotPermitted)); } #[test] diff --git a/system/rust/src/gatt/server/indication_handler.rs b/system/rust/src/gatt/server/indication_handler.rs index 0a06003a7b..44ffe7ef27 100644 --- a/system/rust/src/gatt/server/indication_handler.rs +++ b/system/rust/src/gatt/server/indication_handler.rs @@ -6,10 +6,7 @@ use tokio::{ time::timeout, }; -use crate::{ - gatt::ids::AttHandle, - packets::{AttChild, AttHandleValueIndicationBuilder}, -}; +use crate::{gatt::ids::AttHandle, packets::att}; use super::{ att_database::{AttDatabase, StableAttDatabase}, @@ -53,7 +50,7 @@ impl<T: AttDatabase> IndicationHandler<T> { handle: AttHandle, data: &[u8], mtu: usize, - send_packet: impl FnOnce(AttChild) -> Result<(), SendError>, + send_packet: impl FnOnce(att::Att) -> Result<(), SendError>, ) -> Result<(), IndicationError> { let data_size = data.len(); // As per Core Spec 5.3 Vol 3F 3.4.7.2, the indicated value must be at most @@ -78,7 +75,9 @@ impl<T: AttDatabase> IndicationHandler<T> { let _ = self.pending_confirmation.try_recv(); send_packet( - AttHandleValueIndicationBuilder { handle: handle.into(), value: data.into() }.into(), + att::AttHandleValueIndication { handle: handle.into(), value: data.to_vec() } + .try_into() + .unwrap(), ) .map_err(IndicationError::SendError)?; @@ -116,6 +115,7 @@ impl ConfirmationWatcher { #[cfg(test)] mod test { + use crate::packets::att; use tokio::{sync::oneshot, task::spawn_local, time::Instant}; use crate::{ @@ -175,12 +175,11 @@ mod test { }); // assert: that an AttHandleValueIndication was sent on the channel - let AttChild::AttHandleValueIndication(indication) = rx.await.unwrap() else { - unreachable!() - }; + let indication = rx.await.unwrap(); assert_eq!( - indication, - AttHandleValueIndicationBuilder { handle: HANDLE.into(), value: DATA.into() } + Ok(indication), + att::AttHandleValueIndication { handle: HANDLE.into(), value: DATA.to_vec() } + .try_into() ); }); } @@ -354,7 +353,7 @@ mod test { IndicationHandler::new(get_att_database()); // act: send an indication with an ATT_MTU of 4 and data length of 3 - let res = indication_handler.send(HANDLE, &[1, 2, 3], 4, move |_| unreachable!()).await; + let res = indication_handler.send(HANDLE, &DATA, 4, move |_| unreachable!()).await; // assert: that we got the expected error, indicating the max data size (not the // ATT_MTU, but ATT_MTU-3) diff --git a/system/rust/src/gatt/server/request_handler.rs b/system/rust/src/gatt/server/request_handler.rs index 74cbb7f3e0..a84bdfcea9 100644 --- a/system/rust/src/gatt/server/request_handler.rs +++ b/system/rust/src/gatt/server/request_handler.rs @@ -1,13 +1,10 @@ use log::warn; +use pdl_runtime::DecodeError; +use pdl_runtime::EncodeError; use crate::{ gatt::ids::AttHandle, - packets::{ - AttChild, AttErrorCode, AttErrorResponseBuilder, AttFindByTypeValueRequestView, - AttFindInformationRequestView, AttOpcode, AttReadByGroupTypeRequestView, - AttReadByTypeRequestView, AttReadRequestView, AttView, AttWriteRequestView, Packet, - ParseError, - }, + packets::att::{self, AttErrorCode}, }; use super::{ @@ -27,6 +24,26 @@ pub struct AttRequestHandler<Db: AttDatabase> { db: Db, } +/// Type of errors raised by request handlers. +#[allow(dead_code)] +enum ProcessingError { + DecodeError(DecodeError), + EncodeError(EncodeError), + RequestNotSupported(att::AttOpcode), +} + +impl From<DecodeError> for ProcessingError { + fn from(err: DecodeError) -> Self { + Self::DecodeError(err) + } +} + +impl From<EncodeError> for ProcessingError { + fn from(err: EncodeError) -> Self { + Self::EncodeError(err) + } +} + impl<Db: AttDatabase> AttRequestHandler<Db> { pub fn new(db: Db) -> Self { Self { db } @@ -35,65 +52,53 @@ impl<Db: AttDatabase> AttRequestHandler<Db> { // Runs a task to process an incoming packet. Takes an exclusive reference to // ensure that only one request is outstanding at a time (notifications + // commands should take a different path) - pub async fn process_packet(&mut self, packet: AttView<'_>, mtu: usize) -> AttChild { - match self.try_parse_and_process_packet(packet, mtu).await { + pub async fn process_packet(&mut self, packet: att::Att, mtu: usize) -> att::Att { + match self.try_parse_and_process_packet(&packet, mtu).await { Ok(result) => result, Err(_) => { // parse error, assume it's an unsupported request // TODO(aryarahul): distinguish between REQUEST_NOT_SUPPORTED and INVALID_PDU - AttErrorResponseBuilder { - opcode_in_error: packet.get_opcode(), + att::AttErrorResponse { + opcode_in_error: packet.opcode, handle_in_error: AttHandle(0).into(), - error_code: AttErrorCode::REQUEST_NOT_SUPPORTED, + error_code: AttErrorCode::RequestNotSupported, } - .into() + .try_into() + .unwrap() } } } async fn try_parse_and_process_packet( &mut self, - packet: AttView<'_>, + packet: &att::Att, mtu: usize, - ) -> Result<AttChild, ParseError> { + ) -> Result<att::Att, ProcessingError> { let snapshotted_db = self.db.snapshot(); - match packet.get_opcode() { - AttOpcode::READ_REQUEST => { - Ok(handle_read_request(AttReadRequestView::try_parse(packet)?, mtu, &self.db).await) + match packet.opcode { + att::AttOpcode::ReadRequest => { + Ok(handle_read_request(packet.try_into()?, mtu, &self.db).await?) + } + att::AttOpcode::ReadByGroupTypeRequest => { + Ok(handle_read_by_group_type_request(packet.try_into()?, mtu, &snapshotted_db) + .await?) + } + att::AttOpcode::ReadByTypeRequest => { + Ok(handle_read_by_type_request(packet.try_into()?, mtu, &snapshotted_db).await?) } - AttOpcode::READ_BY_GROUP_TYPE_REQUEST => { - handle_read_by_group_type_request( - AttReadByGroupTypeRequestView::try_parse(packet)?, - mtu, - &snapshotted_db, - ) - .await + att::AttOpcode::FindInformationRequest => { + Ok(handle_find_information_request(packet.try_into()?, mtu, &snapshotted_db)?) } - AttOpcode::READ_BY_TYPE_REQUEST => { - handle_read_by_type_request( - AttReadByTypeRequestView::try_parse(packet)?, - mtu, - &snapshotted_db, - ) - .await + att::AttOpcode::FindByTypeValueRequest => { + Ok(handle_find_by_type_value_request(packet.try_into()?, mtu, &snapshotted_db) + .await?) } - AttOpcode::FIND_INFORMATION_REQUEST => Ok(handle_find_information_request( - AttFindInformationRequestView::try_parse(packet)?, - mtu, - &snapshotted_db, - )), - AttOpcode::FIND_BY_TYPE_VALUE_REQUEST => Ok(handle_find_by_type_value_request( - AttFindByTypeValueRequestView::try_parse(packet)?, - mtu, - &snapshotted_db, - ) - .await), - AttOpcode::WRITE_REQUEST => { - Ok(handle_write_request(AttWriteRequestView::try_parse(packet)?, &self.db).await) + att::AttOpcode::WriteRequest => { + Ok(handle_write_request(packet.try_into()?, &self.db).await?) } _ => { - warn!("Dropping unsupported opcode {:?}", packet.get_opcode()); - Err(ParseError::InvalidEnumValue) + warn!("Dropping unsupported opcode {:?}", packet.opcode); + Err(ProcessingError::RequestNotSupported(packet.opcode)) } } } @@ -110,8 +115,7 @@ mod test { request_handler::AttRequestHandler, test::test_att_db::TestAttDatabase, }, - packets::{AttReadRequestBuilder, AttReadResponseBuilder, AttWriteResponseBuilder}, - utils::packet::build_att_view_or_crash, + packets::att, }; #[test] @@ -126,18 +130,14 @@ mod test { vec![1, 2, 3], )]); let mut handler = AttRequestHandler { db }; - let att_view = build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: AttHandle(3).into(), - }); + let att_view = + att::AttReadRequest { attribute_handle: AttHandle(3).into() }.try_into().unwrap(); // act - let response = tokio_test::block_on(handler.process_packet(att_view.view(), 31)); + let response = tokio_test::block_on(handler.process_packet(att_view, 31)); // assert - assert_eq!( - response, - AttChild::AttReadResponse(AttReadResponseBuilder { value: [1, 2, 3].into() }) - ); + assert_eq!(Ok(response), att::AttReadResponse { value: vec![1, 2, 3] }.try_into()); } #[test] @@ -152,19 +152,20 @@ mod test { vec![1, 2, 3], )]); let mut handler = AttRequestHandler { db }; - let att_view = build_att_view_or_crash(AttWriteResponseBuilder {}); + let att_view = att::AttWriteResponse {}.try_into().unwrap(); // act - let response = tokio_test::block_on(handler.process_packet(att_view.view(), 31)); + let response = tokio_test::block_on(handler.process_packet(att_view, 31)); // assert assert_eq!( - response, - AttChild::AttErrorResponse(AttErrorResponseBuilder { - opcode_in_error: AttOpcode::WRITE_RESPONSE, + Ok(response), + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::WriteResponse, handle_in_error: AttHandle(0).into(), - error_code: AttErrorCode::REQUEST_NOT_SUPPORTED - }) + error_code: AttErrorCode::RequestNotSupported + } + .try_into() ); } } diff --git a/system/rust/src/gatt/server/services/gap.rs b/system/rust/src/gatt/server/services/gap.rs index f5476301ff..81853fa006 100644 --- a/system/rust/src/gatt/server/services/gap.rs +++ b/system/rust/src/gatt/server/services/gap.rs @@ -15,7 +15,7 @@ use crate::{ AttPermissions, GattCharacteristicWithHandle, GattDatabase, GattServiceWithHandle, }, }, - packets::AttErrorCode, + packets::att::AttErrorCode, }; struct GapService; @@ -44,7 +44,7 @@ impl GattDatastore for GapService { DEVICE_NAME_HANDLE => { // for non-bonded peers, don't let them read the device name // TODO(aryarahul): support discoverability, when we make this the main GATT server - Err(AttErrorCode::INSUFFICIENT_AUTHENTICATION) + Err(AttErrorCode::InsufficientAuthentication) } // 0x0000 from AssignedNumbers => "Unknown" DEVICE_APPEARANCE_HANDLE => Ok(vec![0x00, 0x00]), @@ -148,7 +148,7 @@ mod test { let name = block_on_locally(att_db.read_attribute(DEVICE_NAME_HANDLE)); // assert: the name is not readable - assert_eq!(name, Err(AttErrorCode::INSUFFICIENT_AUTHENTICATION)); + assert_eq!(name, Err(AttErrorCode::InsufficientAuthentication)); } #[test] diff --git a/system/rust/src/gatt/server/services/gatt.rs b/system/rust/src/gatt/server/services/gatt.rs index 6ba8e90238..40514b4812 100644 --- a/system/rust/src/gatt/server/services/gatt.rs +++ b/system/rust/src/gatt/server/services/gatt.rs @@ -1,5 +1,6 @@ //! The GATT service as defined in Core Spec 5.3 Vol 3G Section 7 +use pdl_runtime::Packet; use std::{cell::RefCell, collections::HashMap, ops::RangeInclusive, rc::Rc}; use anyhow::Result; @@ -24,10 +25,7 @@ use crate::{ }, }, }, - packets::{ - AttErrorCode, GattClientCharacteristicConfigurationBuilder, - GattClientCharacteristicConfigurationView, GattServiceChangedBuilder, Packet, Serializable, - }, + packets::att::{self, AttErrorCode}, }; #[derive(Default)] @@ -62,7 +60,7 @@ impl GattDatastore for GattService { _: AttributeBackingType, ) -> Result<Vec<u8>, AttErrorCode> { if handle == SERVICE_CHANGE_CCC_DESCRIPTOR_HANDLE { - GattClientCharacteristicConfigurationBuilder { + att::GattClientCharacteristicConfiguration { notification: 0, indication: self .clients @@ -72,8 +70,8 @@ impl GattDatastore for GattService { .unwrap_or(false) .into(), } - .to_vec() - .map_err(|_| AttErrorCode::UNLIKELY_ERROR) + .encode_to_vec() + .map_err(|_| AttErrorCode::UnlikelyError) } else { unreachable!() } @@ -87,18 +85,18 @@ impl GattDatastore for GattService { data: &[u8], ) -> Result<(), AttErrorCode> { if handle == SERVICE_CHANGE_CCC_DESCRIPTOR_HANDLE { - let ccc = GattClientCharacteristicConfigurationView::try_parse_from_buffer(data) - .map_err(|err| { + let ccc = + att::GattClientCharacteristicConfiguration::decode_full(data).map_err(|err| { warn!("failed to parse CCC descriptor, got: {err:?}"); - AttErrorCode::APPLICATION_ERROR + AttErrorCode::ApplicationError })?; let mut clients = self.clients.borrow_mut(); let state = clients.get_mut(&tcb_idx); let Some(state) = state else { error!("Received write request from disconnected client..."); - return Err(AttErrorCode::UNLIKELY_ERROR); + return Err(AttErrorCode::UnlikelyError); }; - state.registered_for_service_change = ccc.get_indication() != 0; + state.registered_for_service_change = ccc.indication != 0; Ok(()) } else { unreachable!() @@ -131,11 +129,11 @@ impl GattDatabaseCallbacks for GattService { spawn_local( bearer.send_indication( SERVICE_CHANGE_HANDLE, - GattServiceChangedBuilder { + att::GattServiceChanged { start_handle: (*range.start()).into(), end_handle: (*range.end()).into(), } - .to_vec() + .encode_to_vec() .unwrap(), ), ); @@ -191,7 +189,7 @@ mod test { }, }, }, - packets::{AttBuilder, AttChild}, + packets::att, utils::task::{block_on_locally, try_await}, }; @@ -209,7 +207,7 @@ mod test { fn add_connection( gatt_database: &SharedBox<GattDatabase>, tcb_idx: TransportIndex, - ) -> (AttDatabaseImpl, SharedBox<AttServerBearer<AttDatabaseImpl>>, UnboundedReceiver<AttBuilder>) + ) -> (AttDatabaseImpl, SharedBox<AttServerBearer<AttDatabaseImpl>>, UnboundedReceiver<att::Att>) { let att_database = gatt_database.get_att_database(tcb_idx); let (tx, rx) = unbounded_channel(); @@ -260,8 +258,8 @@ mod test { assert_eq!( Ok(resp), - GattClientCharacteristicConfigurationBuilder { notification: 0, indication: 0 } - .to_vec() + att::GattClientCharacteristicConfiguration { notification: 0, indication: 0 } + .encode_to_vec() ); } @@ -272,8 +270,8 @@ mod test { att_db .write_attribute( handle, - &GattClientCharacteristicConfigurationBuilder { notification: 0, indication: 1 } - .to_vec() + &att::GattClientCharacteristicConfiguration { notification: 0, indication: 1 } + .encode_to_vec() .unwrap(), ) .await @@ -295,8 +293,8 @@ mod test { // assert: we are registered for indications assert_eq!( Ok(resp), - GattClientCharacteristicConfigurationBuilder { notification: 0, indication: 1 } - .to_vec() + att::GattClientCharacteristicConfiguration { notification: 0, indication: 1 } + .encode_to_vec() ); } @@ -310,8 +308,8 @@ mod test { block_on_locally( att_db.write_attribute( SERVICE_CHANGE_CCC_DESCRIPTOR_HANDLE, - &GattClientCharacteristicConfigurationBuilder { notification: 0, indication: 1 } - .to_vec() + &att::GattClientCharacteristicConfiguration { notification: 0, indication: 1 } + .encode_to_vec() .unwrap(), ), ) @@ -320,8 +318,8 @@ mod test { block_on_locally( att_db.write_attribute( SERVICE_CHANGE_CCC_DESCRIPTOR_HANDLE, - &GattClientCharacteristicConfigurationBuilder { notification: 0, indication: 0 } - .to_vec() + &att::GattClientCharacteristicConfiguration { notification: 0, indication: 0 } + .encode_to_vec() .unwrap(), ), ) @@ -333,8 +331,8 @@ mod test { // assert: we are not registered for indications assert_eq!( Ok(resp), - GattClientCharacteristicConfigurationBuilder { notification: 0, indication: 0 } - .to_vec() + att::GattClientCharacteristicConfiguration { notification: 0, indication: 0 } + .encode_to_vec() ); } @@ -367,17 +365,14 @@ mod test { // assert: we received the service change indication let resp = rx.recv().await.unwrap(); - let AttChild::AttHandleValueIndication(resp) = resp._child_ else { + let Ok(resp): Result<att::AttHandleValueIndication, _> = resp.try_into() else { + unreachable!(); + }; + let Ok(resp) = att::GattServiceChanged::decode_full(resp.value.as_slice()) else { unreachable!(); }; - assert_eq!( - Ok(resp.value.into()), - GattServiceChangedBuilder { - start_handle: AttHandle(15).into(), - end_handle: AttHandle(17).into(), - } - .to_vec() - ); + assert_eq!(resp.start_handle.handle, 15); + assert_eq!(resp.end_handle.handle, 17); }); } @@ -415,8 +410,8 @@ mod test { // assert: both connections received the service change indication let resp1 = rx1.recv().await.unwrap(); let resp2 = rx2.recv().await.unwrap(); - assert!(matches!(resp1._child_, AttChild::AttHandleValueIndication(_))); - assert!(matches!(resp2._child_, AttChild::AttHandleValueIndication(_))); + assert!(matches!(resp1.try_into(), Ok(att::AttHandleValueIndication { .. }))); + assert!(matches!(resp2.try_into(), Ok(att::AttHandleValueIndication { .. }))); }); } @@ -452,7 +447,7 @@ mod test { // assert: the first connection received the service change indication let resp1 = rx1.recv().await.unwrap(); - assert!(matches!(resp1._child_, AttChild::AttHandleValueIndication(_))); + assert!(matches!(resp1.try_into(), Ok(att::AttHandleValueIndication { .. }))); // assert: the second connection received nothing assert!(try_await(async move { rx2.recv().await }).await.is_err()); }); @@ -494,7 +489,7 @@ mod test { // assert: the first connection received the service change indication let resp1 = rx1.recv().await.unwrap(); - assert!(matches!(resp1._child_, AttChild::AttHandleValueIndication(_))); + assert!(matches!(resp1.try_into(), Ok(att::AttHandleValueIndication { .. }))); // assert: the second connection is closed assert!(rx2.recv().await.is_none()); }); diff --git a/system/rust/src/gatt/server/test/test_att_db.rs b/system/rust/src/gatt/server/test/test_att_db.rs index 896f31d4f1..64752f51cf 100644 --- a/system/rust/src/gatt/server/test/test_att_db.rs +++ b/system/rust/src/gatt/server/test/test_att_db.rs @@ -3,7 +3,7 @@ use crate::{ ids::AttHandle, server::att_database::{AttAttribute, AttDatabase, StableAttDatabase}, }, - packets::AttErrorCode, + packets::att::AttErrorCode, }; use async_trait::async_trait; @@ -44,10 +44,10 @@ impl AttDatabase for TestAttDatabase { Some(TestAttributeWithData { attribute: AttAttribute { permissions, .. }, .. }) if !permissions.readable() => { - Err(AttErrorCode::READ_NOT_PERMITTED) + Err(AttErrorCode::ReadNotPermitted) } Some(TestAttributeWithData { data, .. }) => Ok(data.borrow().clone()), - None => Err(AttErrorCode::INVALID_HANDLE), + None => Err(AttErrorCode::InvalidHandle), } } async fn write_attribute(&self, handle: AttHandle, data: &[u8]) -> Result<(), AttErrorCode> { @@ -55,13 +55,13 @@ impl AttDatabase for TestAttDatabase { Some(TestAttributeWithData { attribute: AttAttribute { permissions, .. }, .. }) if !permissions.writable_with_response() => { - Err(AttErrorCode::WRITE_NOT_PERMITTED) + Err(AttErrorCode::WriteNotPermitted) } Some(TestAttributeWithData { data: data_cell, .. }) => { data_cell.replace(data.to_vec()); Ok(()) } - None => Err(AttErrorCode::INVALID_HANDLE), + None => Err(AttErrorCode::InvalidHandle), } } fn write_no_response_attribute(&self, handle: AttHandle, data: &[u8]) { diff --git a/system/rust/src/gatt/server/transactions/find_by_type_value.rs b/system/rust/src/gatt/server/transactions/find_by_type_value.rs index d3a463084a..382990f894 100644 --- a/system/rust/src/gatt/server/transactions/find_by_type_value.rs +++ b/system/rust/src/gatt/server/transactions/find_by_type_value.rs @@ -1,4 +1,5 @@ use log::warn; +use pdl_runtime::EncodeError; use crate::{ core::uuid::Uuid, @@ -6,10 +7,7 @@ use crate::{ ids::AttHandle, server::att_database::{AttAttribute, StableAttDatabase}, }, - packets::{ - AttChild, AttErrorCode, AttErrorResponseBuilder, AttFindByTypeValueRequestView, - AttFindByTypeValueResponseBuilder, AttOpcode, AttributeHandleRangeBuilder, - }, + packets::att::{self, AttErrorCode}, }; use super::helpers::{ @@ -18,34 +16,34 @@ use super::helpers::{ }; pub async fn handle_find_by_type_value_request( - request: AttFindByTypeValueRequestView<'_>, + request: att::AttFindByTypeValueRequest, mtu: usize, db: &impl StableAttDatabase, -) -> AttChild { +) -> Result<att::Att, EncodeError> { let Some(attrs) = filter_to_range( - request.get_starting_handle().into(), - request.get_ending_handle().into(), + request.starting_handle.clone().into(), + request.ending_handle.into(), db.list_attributes().into_iter(), ) else { - return AttErrorResponseBuilder { - opcode_in_error: AttOpcode::FIND_BY_TYPE_VALUE_REQUEST, - handle_in_error: AttHandle::from(request.get_starting_handle()).into(), - error_code: AttErrorCode::INVALID_HANDLE, + return att::AttErrorResponse { + opcode_in_error: att::AttOpcode::FindByTypeValueRequest, + handle_in_error: AttHandle::from(request.starting_handle).into(), + error_code: AttErrorCode::InvalidHandle, } - .into(); + .try_into(); }; // ATT_MTU-1 limit comes from Spec 5.3 Vol 3F Sec 3.4.3.4 let mut matches = PayloadAccumulator::new(mtu - 1); for attr @ AttAttribute { handle, type_, .. } in attrs { - if Uuid::from(request.get_attribute_type()) != type_ { + if Uuid::from(request.attribute_type.clone()) != type_ { continue; } if let Ok(value) = db.read_attribute(handle).await { - if value == request.get_attribute_value_iter().collect::<Vec<_>>() { + if value == request.attribute_value { // match found - if !matches.push(AttributeHandleRangeBuilder { + if !matches.push(att::AttributeHandleRange { found_attribute_handle: handle.into(), group_end_handle: find_group_end(db, attr) .map(|attr| attr.handle) @@ -61,14 +59,14 @@ pub async fn handle_find_by_type_value_request( } if matches.is_empty() { - AttErrorResponseBuilder { - opcode_in_error: AttOpcode::FIND_BY_TYPE_VALUE_REQUEST, - handle_in_error: AttHandle::from(request.get_starting_handle()).into(), - error_code: AttErrorCode::ATTRIBUTE_NOT_FOUND, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::FindByTypeValueRequest, + handle_in_error: request.starting_handle, + error_code: AttErrorCode::AttributeNotFound, } - .into() + .try_into() } else { - AttFindByTypeValueResponseBuilder { handles_info: matches.into_boxed_slice() }.into() + att::AttFindByTypeValueResponse { handles_info: matches.into_vec() }.try_into() } } @@ -84,8 +82,7 @@ mod test { test::test_att_db::TestAttDatabase, }, }, - packets::AttFindByTypeValueRequestBuilder, - utils::packet::build_view_or_crash, + packets::att, }; use super::*; @@ -127,34 +124,30 @@ mod test { ]); // act - let att_view = build_view_or_crash(AttFindByTypeValueRequestBuilder { + let att_view = att::AttFindByTypeValueRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(5).into(), attribute_type: UUID.try_into().unwrap(), - attribute_value: VALUE.into(), - }); - let response = - tokio_test::block_on(handle_find_by_type_value_request(att_view.view(), 128, &db)); + attribute_value: VALUE.to_vec(), + }; + let response = tokio_test::block_on(handle_find_by_type_value_request(att_view, 128, &db)); // assert: we only matched the ones with the correct UUID - let AttChild::AttFindByTypeValueResponse(response) = response else { - unreachable!("{response:?}") - }; assert_eq!( response, - AttFindByTypeValueResponseBuilder { - handles_info: [ - AttributeHandleRangeBuilder { + att::AttFindByTypeValueResponse { + handles_info: vec![ + att::AttributeHandleRange { found_attribute_handle: AttHandle(3).into(), group_end_handle: AttHandle(3).into(), }, - AttributeHandleRangeBuilder { + att::AttributeHandleRange { found_attribute_handle: AttHandle(5).into(), group_end_handle: AttHandle(5).into(), }, ] - .into() } + .try_into() ); } @@ -189,34 +182,30 @@ mod test { ]); // act - let att_view = build_view_or_crash(AttFindByTypeValueRequestBuilder { + let att_view = att::AttFindByTypeValueRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(5).into(), attribute_type: UUID.try_into().unwrap(), - attribute_value: VALUE.into(), - }); - let response = - tokio_test::block_on(handle_find_by_type_value_request(att_view.view(), 128, &db)); + attribute_value: VALUE.to_vec(), + }; + let response = tokio_test::block_on(handle_find_by_type_value_request(att_view, 128, &db)); // assert - let AttChild::AttFindByTypeValueResponse(response) = response else { - unreachable!("{response:?}") - }; assert_eq!( response, - AttFindByTypeValueResponseBuilder { - handles_info: [ - AttributeHandleRangeBuilder { + att::AttFindByTypeValueResponse { + handles_info: vec![ + att::AttributeHandleRange { found_attribute_handle: AttHandle(3).into(), group_end_handle: AttHandle(3).into(), }, - AttributeHandleRangeBuilder { + att::AttributeHandleRange { found_attribute_handle: AttHandle(5).into(), group_end_handle: AttHandle(5).into(), }, ] - .into() } + .try_into() ); } @@ -226,24 +215,23 @@ mod test { let db = TestAttDatabase::new(vec![]); // act: provide an invalid handle range - let att_view = build_view_or_crash(AttFindByTypeValueRequestBuilder { + let att_view = att::AttFindByTypeValueRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(1).into(), attribute_type: UUID.try_into().unwrap(), - attribute_value: VALUE.into(), - }); - let response = - tokio_test::block_on(handle_find_by_type_value_request(att_view.view(), 128, &db)); + attribute_value: VALUE.to_vec(), + }; + let response = tokio_test::block_on(handle_find_by_type_value_request(att_view, 128, &db)); // assert - let AttChild::AttErrorResponse(response) = response else { unreachable!("{response:?}") }; assert_eq!( response, - AttErrorResponseBuilder { - opcode_in_error: AttOpcode::FIND_BY_TYPE_VALUE_REQUEST, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::FindByTypeValueRequest, handle_in_error: AttHandle(3).into(), - error_code: AttErrorCode::INVALID_HANDLE, + error_code: AttErrorCode::InvalidHandle, } + .try_into() ); } @@ -260,24 +248,23 @@ mod test { )]); // act: query using a range that does not overlap with matching attributes - let att_view = build_view_or_crash(AttFindByTypeValueRequestBuilder { + let att_view = att::AttFindByTypeValueRequest { starting_handle: AttHandle(4).into(), ending_handle: AttHandle(5).into(), attribute_type: UUID.try_into().unwrap(), - attribute_value: VALUE.into(), - }); - let response = - tokio_test::block_on(handle_find_by_type_value_request(att_view.view(), 128, &db)); + attribute_value: VALUE.to_vec(), + }; + let response = tokio_test::block_on(handle_find_by_type_value_request(att_view, 128, &db)); - // assert: got ATTRIBUTE_NOT_FOUND erro - let AttChild::AttErrorResponse(response) = response else { unreachable!("{response:?}") }; + // assert: got ATTRIBUTE_NOT_FOUND error assert_eq!( response, - AttErrorResponseBuilder { - opcode_in_error: AttOpcode::FIND_BY_TYPE_VALUE_REQUEST, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::FindByTypeValueRequest, handle_in_error: AttHandle(4).into(), - error_code: AttErrorCode::ATTRIBUTE_NOT_FOUND, + error_code: AttErrorCode::AttributeNotFound, } + .try_into() ); } @@ -312,28 +299,24 @@ mod test { ]); // act: look for a particular characteristic declaration - let att_view = build_view_or_crash(AttFindByTypeValueRequestBuilder { + let att_view = att::AttFindByTypeValueRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(4).into(), attribute_type: CHARACTERISTIC_UUID.try_into().unwrap(), - attribute_value: VALUE.into(), - }); - let response = - tokio_test::block_on(handle_find_by_type_value_request(att_view.view(), 128, &db)); + attribute_value: VALUE.to_vec(), + }; + let response = tokio_test::block_on(handle_find_by_type_value_request(att_view, 128, &db)); // assert - let AttChild::AttFindByTypeValueResponse(response) = response else { - unreachable!("{response:?}") - }; assert_eq!( response, - AttFindByTypeValueResponseBuilder { - handles_info: [AttributeHandleRangeBuilder { + att::AttFindByTypeValueResponse { + handles_info: vec![att::AttributeHandleRange { found_attribute_handle: AttHandle(3).into(), group_end_handle: AttHandle(4).into(), },] - .into() } + .try_into() ); } @@ -360,28 +343,24 @@ mod test { ]); // act: use MTU = 5, so we can only fit one element in the output - let att_view = build_view_or_crash(AttFindByTypeValueRequestBuilder { + let att_view = att::AttFindByTypeValueRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(4).into(), attribute_type: UUID.try_into().unwrap(), - attribute_value: VALUE.into(), - }); - let response = - tokio_test::block_on(handle_find_by_type_value_request(att_view.view(), 5, &db)); + attribute_value: VALUE.to_vec(), + }; + let response = tokio_test::block_on(handle_find_by_type_value_request(att_view, 5, &db)); // assert: only one of the two matches produced - let AttChild::AttFindByTypeValueResponse(response) = response else { - unreachable!("{response:?}") - }; assert_eq!( response, - AttFindByTypeValueResponseBuilder { - handles_info: [AttributeHandleRangeBuilder { + att::AttFindByTypeValueResponse { + handles_info: vec![att::AttributeHandleRange { found_attribute_handle: AttHandle(3).into(), group_end_handle: AttHandle(3).into(), },] - .into() } + .try_into() ); } } diff --git a/system/rust/src/gatt/server/transactions/find_information_request.rs b/system/rust/src/gatt/server/transactions/find_information_request.rs index 26a573ffe0..e9079f3a1d 100644 --- a/system/rust/src/gatt/server/transactions/find_information_request.rs +++ b/system/rust/src/gatt/server/transactions/find_information_request.rs @@ -1,56 +1,40 @@ use crate::{ - gatt::{ - ids::AttHandle, - server::att_database::{AttAttribute, AttDatabase}, - }, - packets::{ - AttChild, AttErrorCode, AttErrorResponseBuilder, AttFindInformationLongResponseBuilder, - AttFindInformationRequestView, AttFindInformationResponseBuilder, - AttFindInformationResponseFormat, AttFindInformationResponseLongEntryBuilder, - AttFindInformationResponseShortEntryBuilder, AttFindInformationShortResponseBuilder, - AttOpcode, - }, + gatt::server::att_database::{AttAttribute, AttDatabase}, + packets::att::{self, AttErrorCode}, }; +use pdl_runtime::EncodeError; use super::helpers::{att_range_filter::filter_to_range, payload_accumulator::PayloadAccumulator}; pub fn handle_find_information_request<T: AttDatabase>( - request: AttFindInformationRequestView<'_>, + request: att::AttFindInformationRequest, mtu: usize, db: &T, -) -> AttChild { +) -> Result<att::Att, EncodeError> { let Some(attrs) = filter_to_range( - request.get_starting_handle().into(), - request.get_ending_handle().into(), + request.starting_handle.clone().into(), + request.ending_handle.into(), db.list_attributes().into_iter(), ) else { - return AttErrorResponseBuilder { - opcode_in_error: AttOpcode::FIND_INFORMATION_REQUEST, - handle_in_error: AttHandle::from(request.get_starting_handle()).into(), - error_code: AttErrorCode::INVALID_HANDLE, + return att::AttErrorResponse { + opcode_in_error: att::AttOpcode::FindInformationRequest, + handle_in_error: request.starting_handle.clone(), + error_code: AttErrorCode::InvalidHandle, } - .into(); + .try_into(); }; if let Some(resp) = handle_find_information_request_short(attrs.clone(), mtu) { - AttFindInformationResponseBuilder { - format: AttFindInformationResponseFormat::SHORT, - _child_: resp.into(), - } - .into() + resp.try_into() } else if let Some(resp) = handle_find_information_request_long(attrs, mtu) { - AttFindInformationResponseBuilder { - format: AttFindInformationResponseFormat::LONG, - _child_: resp.into(), - } - .into() + resp.try_into() } else { - AttErrorResponseBuilder { - opcode_in_error: AttOpcode::FIND_INFORMATION_REQUEST, - handle_in_error: AttHandle::from(request.get_starting_handle()).into(), - error_code: AttErrorCode::ATTRIBUTE_NOT_FOUND, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::FindInformationRequest, + handle_in_error: request.starting_handle, + error_code: AttErrorCode::AttributeNotFound, } - .into() + .try_into() } } @@ -59,13 +43,12 @@ pub fn handle_find_information_request<T: AttDatabase>( fn handle_find_information_request_short( attributes: impl Iterator<Item = AttAttribute>, mtu: usize, -) -> Option<AttFindInformationShortResponseBuilder> { +) -> Option<att::AttFindInformationShortResponse> { // Core Spec 5.3 Vol 3F 3.4.3.2 gives the ATT_MTU - 2 limit let mut out = PayloadAccumulator::new(mtu - 2); for AttAttribute { handle, type_: uuid, .. } in attributes { if let Ok(uuid) = uuid.try_into() { - if out.push(AttFindInformationResponseShortEntryBuilder { handle: handle.into(), uuid }) - { + if out.push(att::AttFindInformationResponseShortEntry { handle: handle.into(), uuid }) { // If we successfully pushed a 16-bit UUID, continue. In all other cases, we // should break. continue; @@ -77,19 +60,19 @@ fn handle_find_information_request_short( if out.is_empty() { None } else { - Some(AttFindInformationShortResponseBuilder { data: out.into_boxed_slice() }) + Some(att::AttFindInformationShortResponse { data: out.into_vec() }) } } fn handle_find_information_request_long( attributes: impl Iterator<Item = AttAttribute>, mtu: usize, -) -> Option<AttFindInformationLongResponseBuilder> { +) -> Option<att::AttFindInformationLongResponse> { // Core Spec 5.3 Vol 3F 3.4.3.2 gives the ATT_MTU - 2 limit let mut out = PayloadAccumulator::new(mtu - 2); for AttAttribute { handle, type_: uuid, .. } in attributes { - if !out.push(AttFindInformationResponseLongEntryBuilder { + if !out.push(att::AttFindInformationResponseLongEntry { handle: handle.into(), uuid: uuid.into(), }) { @@ -100,17 +83,17 @@ fn handle_find_information_request_long( if out.is_empty() { None } else { - Some(AttFindInformationLongResponseBuilder { data: out.into_boxed_slice() }) + Some(att::AttFindInformationLongResponse { data: out.into_vec() }) } } #[cfg(test)] mod test { + use crate::gatt::server::AttHandle; use crate::{ core::uuid::Uuid, gatt::server::{gatt_database::AttPermissions, test::test_att_db::TestAttDatabase}, - packets::AttFindInformationRequestBuilder, - utils::packet::build_view_or_crash, + packets::att, }; use super::*; @@ -146,35 +129,28 @@ mod test { ]); // act - let att_view = build_view_or_crash(AttFindInformationRequestBuilder { + let att_view = att::AttFindInformationRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(4).into(), - }); - let response = handle_find_information_request(att_view.view(), 128, &db); + }; + let response = handle_find_information_request(att_view, 128, &db); // assert - let AttChild::AttFindInformationResponse(response) = response else { - unreachable!("{response:?}"); - }; assert_eq!( response, - AttFindInformationResponseBuilder { - format: AttFindInformationResponseFormat::LONG, - _child_: AttFindInformationLongResponseBuilder { - data: [ - AttFindInformationResponseLongEntryBuilder { - handle: AttHandle(3).into(), - uuid: Uuid::new(0x01020304).into(), - }, - AttFindInformationResponseLongEntryBuilder { - handle: AttHandle(4).into(), - uuid: Uuid::new(0x01020305).into(), - } - ] - .into() - } - .into() + att::AttFindInformationLongResponse { + data: vec![ + att::AttFindInformationResponseLongEntry { + handle: AttHandle(3).into(), + uuid: Uuid::new(0x01020304).into(), + }, + att::AttFindInformationResponseLongEntry { + handle: AttHandle(4).into(), + uuid: Uuid::new(0x01020305).into(), + } + ] } + .try_into() ); } @@ -209,35 +185,28 @@ mod test { ]); // act - let att_view = build_view_or_crash(AttFindInformationRequestBuilder { + let att_view = att::AttFindInformationRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(5).into(), - }); - let response = handle_find_information_request(att_view.view(), 128, &db); + }; + let response = handle_find_information_request(att_view, 128, &db); // assert - let AttChild::AttFindInformationResponse(response) = response else { - unreachable!("{response:?}"); - }; assert_eq!( response, - AttFindInformationResponseBuilder { - format: AttFindInformationResponseFormat::SHORT, - _child_: AttFindInformationShortResponseBuilder { - data: [ - AttFindInformationResponseShortEntryBuilder { - handle: AttHandle(3).into(), - uuid: Uuid::new(0x0102).try_into().unwrap(), - }, - AttFindInformationResponseShortEntryBuilder { - handle: AttHandle(4).into(), - uuid: Uuid::new(0x0103).try_into().unwrap(), - } - ] - .into() - } - .into() + att::AttFindInformationShortResponse { + data: vec![ + att::AttFindInformationResponseShortEntry { + handle: AttHandle(3).into(), + uuid: Uuid::new(0x0102).try_into().unwrap(), + }, + att::AttFindInformationResponseShortEntry { + handle: AttHandle(4).into(), + uuid: Uuid::new(0x0103).try_into().unwrap(), + } + ] } + .try_into() ); } @@ -247,23 +216,21 @@ mod test { let db = TestAttDatabase::new(vec![]); // act: use an invalid handle range - let att_view = build_view_or_crash(AttFindInformationRequestBuilder { + let att_view = att::AttFindInformationRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(2).into(), - }); - let response = handle_find_information_request(att_view.view(), 128, &db); + }; + let response = handle_find_information_request(att_view, 128, &db); // assert: got INVALID_HANDLE - let AttChild::AttErrorResponse(response) = response else { - unreachable!("{response:?}"); - }; assert_eq!( response, - AttErrorResponseBuilder { - opcode_in_error: AttOpcode::FIND_INFORMATION_REQUEST, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::FindInformationRequest, handle_in_error: AttHandle(3).into(), - error_code: AttErrorCode::INVALID_HANDLE, + error_code: AttErrorCode::InvalidHandle, } + .try_into() ); } @@ -290,29 +257,22 @@ mod test { ]); // act: use MTU = 6, so only one entry can fit - let att_view = build_view_or_crash(AttFindInformationRequestBuilder { + let att_view = att::AttFindInformationRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(5).into(), - }); - let response = handle_find_information_request(att_view.view(), 6, &db); + }; + let response = handle_find_information_request(att_view, 6, &db); // assert: only one entry (not two) provided - let AttChild::AttFindInformationResponse(response) = response else { - unreachable!("{response:?}"); - }; assert_eq!( response, - AttFindInformationResponseBuilder { - format: AttFindInformationResponseFormat::SHORT, - _child_: AttFindInformationShortResponseBuilder { - data: [AttFindInformationResponseShortEntryBuilder { - handle: AttHandle(3).into(), - uuid: Uuid::new(0x0102).try_into().unwrap(), - },] - .into() - } - .into() + att::AttFindInformationShortResponse { + data: vec![att::AttFindInformationResponseShortEntry { + handle: AttHandle(3).into(), + uuid: Uuid::new(0x0102).try_into().unwrap(), + },] } + .try_into() ); } @@ -329,23 +289,21 @@ mod test { )]); // act: use a range that matches no attributes - let att_view = build_view_or_crash(AttFindInformationRequestBuilder { + let att_view = att::AttFindInformationRequest { starting_handle: AttHandle(4).into(), ending_handle: AttHandle(5).into(), - }); - let response = handle_find_information_request(att_view.view(), 6, &db); + }; + let response = handle_find_information_request(att_view, 6, &db); // assert: got ATTRIBUTE_NOT_FOUND - let AttChild::AttErrorResponse(response) = response else { - unreachable!("{response:?}"); - }; assert_eq!( response, - AttErrorResponseBuilder { - opcode_in_error: AttOpcode::FIND_INFORMATION_REQUEST, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::FindInformationRequest, handle_in_error: AttHandle(4).into(), - error_code: AttErrorCode::ATTRIBUTE_NOT_FOUND, + error_code: AttErrorCode::AttributeNotFound, } + .try_into() ); } } diff --git a/system/rust/src/gatt/server/transactions/helpers/att_filter_by_size_type.rs b/system/rust/src/gatt/server/transactions/helpers/att_filter_by_size_type.rs index ac93459bcf..1674597bc4 100644 --- a/system/rust/src/gatt/server/transactions/helpers/att_filter_by_size_type.rs +++ b/system/rust/src/gatt/server/transactions/helpers/att_filter_by_size_type.rs @@ -4,7 +4,7 @@ use crate::{ core::uuid::Uuid, gatt::server::att_database::{AttAttribute, StableAttDatabase}, - packets::AttErrorCode, + packets::att::AttErrorCode, }; /// An attribute and the value @@ -108,7 +108,7 @@ mod test { response.collect::<Vec<_>>(), vec![AttributeWithValue { attr: db.find_attribute(AttHandle(3)).unwrap(), - value: vec![4, 5] + value: vec![4, 5], }] ) } @@ -287,7 +287,7 @@ mod test { )); // assert: got READ_NOT_PERMITTED - assert!(matches!(response, Err(AttErrorCode::READ_NOT_PERMITTED))); + assert!(matches!(response, Err(AttErrorCode::ReadNotPermitted))); } #[test] diff --git a/system/rust/src/gatt/server/transactions/helpers/payload_accumulator.rs b/system/rust/src/gatt/server/transactions/helpers/payload_accumulator.rs index 72aa46b367..f372e1f6ae 100644 --- a/system/rust/src/gatt/server/transactions/helpers/payload_accumulator.rs +++ b/system/rust/src/gatt/server/transactions/helpers/payload_accumulator.rs @@ -1,21 +1,19 @@ -use crate::packets::Builder; - -pub struct PayloadAccumulator<T: Builder> { +pub struct PayloadAccumulator<T: pdl_runtime::Packet> { curr: usize, lim: usize, elems: Vec<T>, } -impl<T: Builder> PayloadAccumulator<T> { +impl<T: pdl_runtime::Packet> PayloadAccumulator<T> { pub fn new(size: usize) -> Self { - Self { curr: 0, lim: size * 8, elems: vec![] } + Self { curr: 0, lim: size, elems: vec![] } } #[must_use] pub fn push(&mut self, builder: T) -> bool { // if serialization fails we WANT to continue, to get a clean SerializeError at // the end - let elem_size = builder.size_in_bits().unwrap_or(0); + let elem_size = builder.encoded_len(); if elem_size + self.curr > self.lim { return false; } @@ -24,8 +22,8 @@ impl<T: Builder> PayloadAccumulator<T> { true } - pub fn into_boxed_slice(self) -> Box<[T]> { - self.elems.into_boxed_slice() + pub fn into_vec(self) -> Vec<T> { + self.elems } pub fn is_empty(&self) -> bool { @@ -35,23 +33,21 @@ impl<T: Builder> PayloadAccumulator<T> { #[cfg(test)] mod test { - use crate::packets::{AttBuilder, AttChild, AttOpcode}; + use crate::packets::att; use super::PayloadAccumulator; #[test] fn test_empty() { - let accumulator = PayloadAccumulator::<AttBuilder>::new(0); + let accumulator = PayloadAccumulator::<att::Att>::new(0); assert!(accumulator.is_empty()) } #[test] fn test_nonempty() { let mut accumulator = PayloadAccumulator::new(128); - let ok = accumulator.push(AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([1, 2].into()), - }); + let ok = accumulator + .push(att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![1, 2] }); assert!(ok); assert!(!accumulator.is_empty()) @@ -61,18 +57,13 @@ mod test { fn test_push_serialize() { let mut accumulator = PayloadAccumulator::new(128); - let ok = accumulator.push(AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([1, 2].into()), - }); + let ok = accumulator + .push(att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![1, 2] }); assert!(ok); assert_eq!( - accumulator.into_boxed_slice().as_ref(), - [AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([1, 2].into()), - }] + accumulator.into_vec().as_ref(), + [att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![1, 2] }] ); } @@ -81,25 +72,18 @@ mod test { let mut accumulator = PayloadAccumulator::new(5); // each builder is 3 bytes, so the first should succeed, the second should fail - let first_ok = accumulator.push(AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([1, 2].into()), - }); - let second_ok = accumulator.push(AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([3, 4].into()), - }); + let first_ok = accumulator + .push(att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![1, 2] }); + let second_ok = accumulator + .push(att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![3, 4] }); // assert: the first one is pushed and is correctly output, but the second is // dropped assert!(first_ok); assert!(!second_ok); assert_eq!( - accumulator.into_boxed_slice().as_ref(), - [AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([1, 2].into()), - }] + accumulator.into_vec().as_ref(), + [att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![1, 2] }] ); } @@ -108,29 +92,19 @@ mod test { let mut accumulator = PayloadAccumulator::new(5); // 3 + 2 bytes = the size, so both should push correctly - let first_ok = accumulator.push(AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([1, 2].into()), - }); - let second_ok = accumulator.push(AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([3].into()), - }); + let first_ok = accumulator + .push(att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![1, 2] }); + let second_ok = + accumulator.push(att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![3] }); // assert: both are pushed and output correctly assert!(first_ok); assert!(second_ok); assert_eq!( - accumulator.into_boxed_slice().as_ref(), + accumulator.into_vec().as_ref(), [ - AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([1, 2].into()), - }, - AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttChild::RawData([3].into()), - } + att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![1, 2] }, + att::Att { opcode: att::AttOpcode::WriteResponse, payload: vec![3] } ] ); } diff --git a/system/rust/src/gatt/server/transactions/read_by_group_type_request.rs b/system/rust/src/gatt/server/transactions/read_by_group_type_request.rs index 762b8ce196..319d5e6c45 100644 --- a/system/rust/src/gatt/server/transactions/read_by_group_type_request.rs +++ b/system/rust/src/gatt/server/transactions/read_by_group_type_request.rs @@ -1,18 +1,12 @@ use crate::{ core::uuid::Uuid, - gatt::{ - ids::AttHandle, - server::{ - att_database::StableAttDatabase, - gatt_database::{PRIMARY_SERVICE_DECLARATION_UUID, SECONDARY_SERVICE_DECLARATION_UUID}, - }, - }, - packets::{ - AttChild, AttErrorCode, AttErrorResponseBuilder, AttOpcode, - AttReadByGroupTypeDataElementBuilder, AttReadByGroupTypeRequestView, - AttReadByGroupTypeResponseBuilder, ParseError, + gatt::server::{ + att_database::StableAttDatabase, + gatt_database::{PRIMARY_SERVICE_DECLARATION_UUID, SECONDARY_SERVICE_DECLARATION_UUID}, }, + packets::att::{self, AttErrorCode}, }; +use pdl_runtime::EncodeError; use super::helpers::{ att_filter_by_size_type::{filter_read_attributes_by_size_type, AttributeWithValue}, @@ -22,12 +16,10 @@ use super::helpers::{ }; pub async fn handle_read_by_group_type_request( - request: AttReadByGroupTypeRequestView<'_>, + request: att::AttReadByGroupTypeRequest, mtu: usize, db: &impl StableAttDatabase, -) -> Result<AttChild, ParseError> { - let group_type: Uuid = request.get_attribute_group_type().try_into()?; - +) -> Result<att::Att, EncodeError> { // As per spec (5.3 Vol 3F 3.4.4.9) // > If an attribute in the set of requested attributes would cause an // > ATT_ERROR_RSP PDU then this attribute cannot be included in an @@ -36,20 +28,25 @@ pub async fn handle_read_by_group_type_request( // // Thus, we populate this response on failure, but only return it if no prior // matches were accumulated. - let mut failure_response = AttErrorResponseBuilder { - opcode_in_error: AttOpcode::READ_BY_GROUP_TYPE_REQUEST, - handle_in_error: AttHandle::from(request.get_starting_handle()).into(), + let mut failure_response = att::AttErrorResponse { + opcode_in_error: request.opcode(), + handle_in_error: request.starting_handle.clone(), // the default error code if we just fail to find anything - error_code: AttErrorCode::ATTRIBUTE_NOT_FOUND, + error_code: AttErrorCode::AttributeNotFound, + }; + + let Ok(group_type): Result<Uuid, _> = request.attribute_group_type.try_into() else { + failure_response.error_code = AttErrorCode::InvalidPdu; + return failure_response.try_into(); }; let Some(attrs) = filter_to_range( - request.get_starting_handle().into(), - request.get_ending_handle().into(), + request.starting_handle.into(), + request.ending_handle.into(), db.list_attributes().into_iter(), ) else { - failure_response.error_code = AttErrorCode::INVALID_HANDLE; - return Ok(failure_response.into()); + failure_response.error_code = AttErrorCode::InvalidHandle; + return failure_response.try_into(); }; // As per Core Spec 5.3 Vol 3G 2.5.3 Attribute Grouping, only these UUIDs are @@ -57,8 +54,8 @@ pub async fn handle_read_by_group_type_request( // UUIDs do exist) if !matches!(group_type, PRIMARY_SERVICE_DECLARATION_UUID | SECONDARY_SERVICE_DECLARATION_UUID) { - failure_response.error_code = AttErrorCode::UNSUPPORTED_GROUP_TYPE; - return Ok(failure_response.into()); + failure_response.error_code = AttErrorCode::UnsupportedGroupType; + return failure_response.try_into(); } // MTU-2 limit comes from Core Spec 5.3 Vol 3F 3.4.4.9 @@ -68,13 +65,13 @@ pub async fn handle_read_by_group_type_request( match filter_read_attributes_by_size_type(db, attrs, group_type, mtu - 6).await { Ok(attrs) => { for AttributeWithValue { attr, value } in attrs { - if !matches.push(AttReadByGroupTypeDataElementBuilder { + if !matches.push(att::AttReadByGroupTypeDataElement { handle: attr.handle.into(), end_group_handle: find_group_end(db, attr) .expect("should never be None, since grouping UUID was validated earlier") .handle .into(), - value: value.into(), + value, }) { break; } @@ -82,15 +79,15 @@ pub async fn handle_read_by_group_type_request( } Err(err) => { failure_response.error_code = err; - return Ok(failure_response.into()); + return failure_response.try_into(); } } - Ok(if matches.is_empty() { - failure_response.into() + if matches.is_empty() { + failure_response.try_into() } else { - AttReadByGroupTypeResponseBuilder { data: matches.into_boxed_slice() }.into() - }) + att::AttReadByGroupTypeResponse { data: matches.into_vec() }.try_into() + } } #[cfg(test)] @@ -104,8 +101,7 @@ mod test { test::test_att_db::TestAttDatabase, }, }, - packets::AttReadByGroupTypeRequestBuilder, - utils::packet::build_view_or_crash, + packets::att, }; use super::*; @@ -141,36 +137,31 @@ mod test { ]); // act - let att_view = build_view_or_crash(AttReadByGroupTypeRequestBuilder { + let att_view = att::AttReadByGroupTypeRequest { starting_handle: AttHandle(2).into(), ending_handle: AttHandle(6).into(), attribute_group_type: PRIMARY_SERVICE_DECLARATION_UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_group_type_request(att_view.view(), 31, &db)) - .unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_group_type_request(att_view, 31, &db)); // assert: we identified both service groups - let AttChild::AttReadByGroupTypeResponse(response) = response else { - unreachable!("{:?}", response) - }; assert_eq!( response, - AttReadByGroupTypeResponseBuilder { - data: [ - AttReadByGroupTypeDataElementBuilder { + att::AttReadByGroupTypeResponse { + data: vec![ + att::AttReadByGroupTypeDataElement { handle: AttHandle(3).into(), end_group_handle: AttHandle(4).into(), - value: [4, 5].into(), + value: vec![4, 5], }, - AttReadByGroupTypeDataElementBuilder { + att::AttReadByGroupTypeDataElement { handle: AttHandle(5).into(), end_group_handle: AttHandle(5).into(), - value: [6, 7].into(), + value: vec![6, 7], }, ] - .into() } + .try_into() ); } @@ -180,24 +171,22 @@ mod test { let db = TestAttDatabase::new(vec![]); // act: try using an unsupported group type - let att_view = build_view_or_crash(AttReadByGroupTypeRequestBuilder { + let att_view = att::AttReadByGroupTypeRequest { starting_handle: AttHandle(2).into(), ending_handle: AttHandle(6).into(), attribute_group_type: CHARACTERISTIC_UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_group_type_request(att_view.view(), 31, &db)) - .unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_group_type_request(att_view, 31, &db)); // assert: got UNSUPPORTED_GROUP_TYPE - let AttChild::AttErrorResponse(response) = response else { unreachable!("{:?}", response) }; assert_eq!( response, - AttErrorResponseBuilder { + att::AttErrorResponse { handle_in_error: AttHandle(2).into(), - opcode_in_error: AttOpcode::READ_BY_GROUP_TYPE_REQUEST, - error_code: AttErrorCode::UNSUPPORTED_GROUP_TYPE, + opcode_in_error: att::AttOpcode::ReadByGroupTypeRequest, + error_code: AttErrorCode::UnsupportedGroupType, } + .try_into() ); } @@ -207,24 +196,22 @@ mod test { let db = TestAttDatabase::new(vec![]); // act: query with an invalid attribute range - let att_view = build_view_or_crash(AttReadByGroupTypeRequestBuilder { + let att_view = att::AttReadByGroupTypeRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(2).into(), attribute_group_type: PRIMARY_SERVICE_DECLARATION_UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_group_type_request(att_view.view(), 31, &db)) - .unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_group_type_request(att_view, 31, &db)); // assert: we return an INVALID_HANDLE error - let AttChild::AttErrorResponse(response) = response else { unreachable!("{:?}", response) }; assert_eq!( response, - AttErrorResponseBuilder { + att::AttErrorResponse { handle_in_error: AttHandle(3).into(), - opcode_in_error: AttOpcode::READ_BY_GROUP_TYPE_REQUEST, - error_code: AttErrorCode::INVALID_HANDLE, + opcode_in_error: att::AttOpcode::ReadByGroupTypeRequest, + error_code: AttErrorCode::InvalidHandle, } + .try_into() ) } @@ -242,29 +229,24 @@ mod test { // act: read the service value with MTU = 7, so the value is truncated to MTU-6 // = 1 - let att_view = build_view_or_crash(AttReadByGroupTypeRequestBuilder { + let att_view = att::AttReadByGroupTypeRequest { starting_handle: AttHandle(2).into(), ending_handle: AttHandle(6).into(), attribute_group_type: PRIMARY_SERVICE_DECLARATION_UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_group_type_request(att_view.view(), 7, &db)) - .unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_group_type_request(att_view, 7, &db)); // assert: we identified both service groups - let AttChild::AttReadByGroupTypeResponse(response) = response else { - unreachable!("{:?}", response) - }; assert_eq!( response, - AttReadByGroupTypeResponseBuilder { - data: [AttReadByGroupTypeDataElementBuilder { + att::AttReadByGroupTypeResponse { + data: vec![att::AttReadByGroupTypeDataElement { handle: AttHandle(3).into(), end_group_handle: AttHandle(3).into(), - value: [1].into(), + value: vec![1], },] - .into() } + .try_into() ); } @@ -291,29 +273,24 @@ mod test { ]); // act: read with MTU = 9, so we can only fit the first attribute (untruncated) - let att_view = build_view_or_crash(AttReadByGroupTypeRequestBuilder { + let att_view = att::AttReadByGroupTypeRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(6).into(), attribute_group_type: PRIMARY_SERVICE_DECLARATION_UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_group_type_request(att_view.view(), 9, &db)) - .unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_group_type_request(att_view, 9, &db)); // assert: we return only the first attribute - let AttChild::AttReadByGroupTypeResponse(response) = response else { - unreachable!("{:?}", response) - }; assert_eq!( response, - AttReadByGroupTypeResponseBuilder { - data: [AttReadByGroupTypeDataElementBuilder { + att::AttReadByGroupTypeResponse { + data: vec![att::AttReadByGroupTypeDataElement { handle: AttHandle(3).into(), end_group_handle: AttHandle(3).into(), - value: [4, 5, 6].into() + value: vec![4, 5, 6], },] - .into() } + .try_into() ) } @@ -340,30 +317,25 @@ mod test { ]); // act: search in an interval that includes the service but not its child - let att_view = build_view_or_crash(AttReadByGroupTypeRequestBuilder { + let att_view = att::AttReadByGroupTypeRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(3).into(), attribute_group_type: PRIMARY_SERVICE_DECLARATION_UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_group_type_request(att_view.view(), 31, &db)) - .unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_group_type_request(att_view, 31, &db)); // assert: the end_group_handle is correct, even though it exceeds the query // interval - let AttChild::AttReadByGroupTypeResponse(response) = response else { - unreachable!("{:?}", response) - }; assert_eq!( response, - AttReadByGroupTypeResponseBuilder { - data: [AttReadByGroupTypeDataElementBuilder { + att::AttReadByGroupTypeResponse { + data: vec![att::AttReadByGroupTypeDataElement { handle: AttHandle(3).into(), end_group_handle: AttHandle(4).into(), - value: [4, 5].into(), + value: vec![4, 5], },] - .into() } + .try_into() ); } @@ -390,24 +362,22 @@ mod test { ]); // act - let att_view = build_view_or_crash(AttReadByGroupTypeRequestBuilder { + let att_view = att::AttReadByGroupTypeRequest { starting_handle: AttHandle(5).into(), ending_handle: AttHandle(6).into(), attribute_group_type: PRIMARY_SERVICE_DECLARATION_UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_group_type_request(att_view.view(), 31, &db)) - .unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_group_type_request(att_view, 31, &db)); // assert: we return ATTRIBUTE_NOT_FOUND - let AttChild::AttErrorResponse(response) = response else { unreachable!("{:?}", response) }; assert_eq!( response, - AttErrorResponseBuilder { + att::AttErrorResponse { handle_in_error: AttHandle(5).into(), - opcode_in_error: AttOpcode::READ_BY_GROUP_TYPE_REQUEST, - error_code: AttErrorCode::ATTRIBUTE_NOT_FOUND, + opcode_in_error: att::AttOpcode::ReadByGroupTypeRequest, + error_code: AttErrorCode::AttributeNotFound, } + .try_into() ) } } diff --git a/system/rust/src/gatt/server/transactions/read_by_type_request.rs b/system/rust/src/gatt/server/transactions/read_by_type_request.rs index 83a1b0862a..3e7f3b44c4 100644 --- a/system/rust/src/gatt/server/transactions/read_by_type_request.rs +++ b/system/rust/src/gatt/server/transactions/read_by_type_request.rs @@ -1,12 +1,9 @@ use crate::{ core::uuid::Uuid, - gatt::{ids::AttHandle, server::att_database::StableAttDatabase}, - packets::{ - AttChild, AttErrorCode, AttErrorResponseBuilder, AttOpcode, - AttReadByTypeDataElementBuilder, AttReadByTypeRequestView, AttReadByTypeResponseBuilder, - ParseError, - }, + gatt::server::att_database::StableAttDatabase, + packets::att::{self, AttErrorCode}, }; +use pdl_runtime::EncodeError; use super::helpers::{ att_filter_by_size_type::{filter_read_attributes_by_size_type, AttributeWithValue}, @@ -15,12 +12,10 @@ use super::helpers::{ }; pub async fn handle_read_by_type_request( - request: AttReadByTypeRequestView<'_>, + request: att::AttReadByTypeRequest, mtu: usize, db: &impl StableAttDatabase, -) -> Result<AttChild, ParseError> { - let request_type: Uuid = request.get_attribute_type().try_into()?; - +) -> Result<att::Att, EncodeError> { // As per spec (5.3 Vol 3F 3.4.4.1) // > If an attribute in the set of requested attributes would cause an // > ATT_ERROR_RSP PDU then this attribute cannot be included in an @@ -29,20 +24,25 @@ pub async fn handle_read_by_type_request( // // Thus, we populate this response on failure, but only return it if no prior // matches were accumulated. - let mut failure_response = AttErrorResponseBuilder { - opcode_in_error: AttOpcode::READ_BY_TYPE_REQUEST, - handle_in_error: AttHandle::from(request.get_starting_handle()).into(), + let mut failure_response = att::AttErrorResponse { + opcode_in_error: att::AttOpcode::ReadByTypeRequest, + handle_in_error: request.starting_handle.clone(), // the default error code if we just fail to find anything - error_code: AttErrorCode::ATTRIBUTE_NOT_FOUND, + error_code: AttErrorCode::AttributeNotFound, + }; + + let Ok(request_type): Result<Uuid, _> = request.attribute_type.try_into() else { + failure_response.error_code = AttErrorCode::InvalidPdu; + return failure_response.try_into(); }; let Some(attrs) = filter_to_range( - request.get_starting_handle().into(), - request.get_ending_handle().into(), + request.starting_handle.into(), + request.ending_handle.into(), db.list_attributes().into_iter(), ) else { - failure_response.error_code = AttErrorCode::INVALID_HANDLE; - return Ok(failure_response.into()); + failure_response.error_code = AttErrorCode::InvalidHandle; + return failure_response.try_into(); }; // MTU-2 limit comes from Core Spec 5.3 Vol 3F 3.4.4.1 @@ -52,25 +52,22 @@ pub async fn handle_read_by_type_request( match filter_read_attributes_by_size_type(db, attrs, request_type, mtu - 4).await { Ok(attrs) => { for AttributeWithValue { attr, value } in attrs { - if !out.push(AttReadByTypeDataElementBuilder { - handle: attr.handle.into(), - value: value.into_boxed_slice(), - }) { + if !out.push(att::AttReadByTypeDataElement { handle: attr.handle.into(), value }) { break; } } } Err(err) => { failure_response.error_code = err; - return Ok(failure_response.into()); + return failure_response.try_into(); } } - Ok(if out.is_empty() { - failure_response.into() + if out.is_empty() { + failure_response.try_into() } else { - AttReadByTypeResponseBuilder { data: out.into_boxed_slice() }.into() - }) + att::AttReadByTypeResponse { data: out.into_vec() }.try_into() + } } #[cfg(test)] @@ -86,8 +83,7 @@ mod test { test::test_att_db::TestAttDatabase, }, }, - packets::AttReadByTypeRequestBuilder, - utils::packet::build_view_or_crash, + packets::att, }; const UUID: Uuid = Uuid::new(1234); @@ -106,27 +102,23 @@ mod test { )]); // act - let att_view = build_view_or_crash(AttReadByTypeRequestBuilder { + let att_view = att::AttReadByTypeRequest { starting_handle: AttHandle(2).into(), ending_handle: AttHandle(6).into(), attribute_type: UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_type_request(att_view.view(), 31, &db)).unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_type_request(att_view, 31, &db)); // assert - let AttChild::AttReadByTypeResponse(response) = response else { - unreachable!("{:?}", response) - }; assert_eq!( response, - AttReadByTypeResponseBuilder { - data: [AttReadByTypeDataElementBuilder { + att::AttReadByTypeResponse { + data: vec![att::AttReadByTypeDataElement { handle: AttHandle(3).into(), - value: [4, 5].into() - }] - .into() + value: vec![4, 5], + },] } + .try_into() ) } @@ -161,34 +153,30 @@ mod test { ]); // act - let att_view = build_view_or_crash(AttReadByTypeRequestBuilder { + let att_view = att::AttReadByTypeRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(6).into(), attribute_type: UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_type_request(att_view.view(), 31, &db)).unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_type_request(att_view, 31, &db)); // assert: we correctly filtered by type (so we are using the filter_by_type // utility) - let AttChild::AttReadByTypeResponse(response) = response else { - unreachable!("{:?}", response) - }; assert_eq!( response, - AttReadByTypeResponseBuilder { - data: [ - AttReadByTypeDataElementBuilder { + att::AttReadByTypeResponse { + data: vec![ + att::AttReadByTypeDataElement { handle: AttHandle(3).into(), - value: [4, 5].into() + value: vec![4, 5], }, - AttReadByTypeDataElementBuilder { + att::AttReadByTypeDataElement { handle: AttHandle(6).into(), - value: [6, 7].into() - } + value: vec![6, 7], + }, ] - .into() } + .try_into() ) } @@ -215,27 +203,23 @@ mod test { ]); // act: read with MTU = 8, so we can only fit the first attribute (untruncated) - let att_view = build_view_or_crash(AttReadByTypeRequestBuilder { + let att_view = att::AttReadByTypeRequest { starting_handle: AttHandle(3).into(), ending_handle: AttHandle(6).into(), attribute_type: UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_type_request(att_view.view(), 8, &db)).unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_type_request(att_view, 8, &db)); // assert: we return only the first attribute - let AttChild::AttReadByTypeResponse(response) = response else { - unreachable!("{:?}", response) - }; assert_eq!( response, - AttReadByTypeResponseBuilder { - data: [AttReadByTypeDataElementBuilder { + att::AttReadByTypeResponse { + data: vec![att::AttReadByTypeDataElement { handle: AttHandle(3).into(), - value: [4, 5, 6].into() + value: vec![4, 5, 6], },] - .into() } + .try_into() ) } @@ -262,23 +246,22 @@ mod test { ]); // act - let att_view = build_view_or_crash(AttReadByTypeRequestBuilder { + let att_view = att::AttReadByTypeRequest { starting_handle: AttHandle(4).into(), ending_handle: AttHandle(6).into(), attribute_type: UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_type_request(att_view.view(), 31, &db)).unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_type_request(att_view, 31, &db)); // assert: we return ATTRIBUTE_NOT_FOUND - let AttChild::AttErrorResponse(response) = response else { unreachable!("{:?}", response) }; assert_eq!( response, - AttErrorResponseBuilder { + att::AttErrorResponse { handle_in_error: AttHandle(4).into(), - opcode_in_error: AttOpcode::READ_BY_TYPE_REQUEST, - error_code: AttErrorCode::ATTRIBUTE_NOT_FOUND, + opcode_in_error: att::AttOpcode::ReadByTypeRequest, + error_code: AttErrorCode::AttributeNotFound, } + .try_into() ) } @@ -288,23 +271,22 @@ mod test { let db = TestAttDatabase::new(vec![]); // act - let att_view = build_view_or_crash(AttReadByTypeRequestBuilder { + let att_view = att::AttReadByTypeRequest { starting_handle: AttHandle(0).into(), ending_handle: AttHandle(6).into(), attribute_type: UUID.into(), - }); - let response = - tokio_test::block_on(handle_read_by_type_request(att_view.view(), 31, &db)).unwrap(); + }; + let response = tokio_test::block_on(handle_read_by_type_request(att_view, 31, &db)); // assert: we return an INVALID_HANDLE error - let AttChild::AttErrorResponse(response) = response else { unreachable!("{:?}", response) }; assert_eq!( response, - AttErrorResponseBuilder { + att::AttErrorResponse { handle_in_error: AttHandle(0).into(), - opcode_in_error: AttOpcode::READ_BY_TYPE_REQUEST, - error_code: AttErrorCode::INVALID_HANDLE, + opcode_in_error: att::AttOpcode::ReadByTypeRequest, + error_code: AttErrorCode::InvalidHandle, } + .try_into() ) } } diff --git a/system/rust/src/gatt/server/transactions/read_request.rs b/system/rust/src/gatt/server/transactions/read_request.rs index ba4ea3a3fe..b5827deeb5 100644 --- a/system/rust/src/gatt/server/transactions/read_request.rs +++ b/system/rust/src/gatt/server/transactions/read_request.rs @@ -1,29 +1,25 @@ -use crate::{ - gatt::server::att_database::AttDatabase, - packets::{ - AttChild, AttErrorResponseBuilder, AttOpcode, AttReadRequestView, AttReadResponseBuilder, - }, -}; +use crate::{gatt::server::att_database::AttDatabase, packets::att}; +use pdl_runtime::EncodeError; pub async fn handle_read_request<T: AttDatabase>( - request: AttReadRequestView<'_>, + request: att::AttReadRequest, mtu: usize, db: &T, -) -> AttChild { - let handle = request.get_attribute_handle().into(); +) -> Result<att::Att, EncodeError> { + let handle = request.attribute_handle.into(); match db.read_attribute(handle).await { Ok(mut data) => { // as per 5.3 3F 3.4.4.4 ATT_READ_RSP, we truncate to MTU - 1 data.truncate(mtu - 1); - AttReadResponseBuilder { value: data.into_boxed_slice() }.into() + att::AttReadResponse { value: data }.try_into() } - Err(error_code) => AttErrorResponseBuilder { - opcode_in_error: AttOpcode::READ_REQUEST, + Err(error_code) => att::AttErrorResponse { + opcode_in_error: att::AttOpcode::ReadRequest, handle_in_error: handle.into(), error_code, } - .into(), + .try_into(), } } @@ -40,8 +36,7 @@ mod test { test::test_att_db::TestAttDatabase, }, }, - packets::{AttErrorCode, AttReadRequestBuilder, Serializable}, - utils::packet::build_view_or_crash, + packets::att, }; fn make_db_with_handle_and_value(handle: u16, value: Vec<u8>) -> TestAttDatabase { @@ -59,11 +54,9 @@ mod test { handle: u16, mtu: usize, db: &TestAttDatabase, - ) -> AttChild { - let att_view = build_view_or_crash(AttReadRequestBuilder { - attribute_handle: AttHandle(handle).into(), - }); - tokio_test::block_on(handle_read_request(att_view.view(), mtu, db)) + ) -> Result<att::Att, EncodeError> { + let att_view = att::AttReadRequest { attribute_handle: AttHandle(handle).into() }; + tokio_test::block_on(handle_read_request(att_view, mtu, db)) } #[test] @@ -72,11 +65,7 @@ mod test { let response = do_read_request_with_handle_and_mtu(3, 31, &db); - response.to_vec().unwrap(); // check it serializes - assert_eq!( - response, - AttChild::AttReadResponse(AttReadResponseBuilder { value: [4, 5].into() }) - ) + assert_eq!(response, att::AttReadResponse { value: vec![4, 5] }.try_into()); } #[test] @@ -87,7 +76,7 @@ mod test { let response = do_read_request_with_handle_and_mtu(3, 2, &db); // assert - assert_eq!(response.to_vec().unwrap(), vec![4]); + assert_eq!(response, att::AttReadResponse { value: vec![4] }.try_into()); } #[test] @@ -100,11 +89,12 @@ mod test { // assert assert_eq!( response, - AttChild::AttErrorResponse(AttErrorResponseBuilder { - opcode_in_error: AttOpcode::READ_REQUEST, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::ReadRequest, handle_in_error: AttHandle(4).into(), - error_code: AttErrorCode::INVALID_HANDLE, - }) + error_code: att::AttErrorCode::InvalidHandle, + } + .try_into() ); } @@ -129,11 +119,12 @@ mod test { // assert assert_eq!( response, - AttChild::AttErrorResponse(AttErrorResponseBuilder { - opcode_in_error: AttOpcode::READ_REQUEST, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::ReadRequest, handle_in_error: AttHandle(3).into(), - error_code: AttErrorCode::READ_NOT_PERMITTED, - }) + error_code: att::AttErrorCode::ReadNotPermitted, + } + .try_into() ); } } diff --git a/system/rust/src/gatt/server/transactions/write_request.rs b/system/rust/src/gatt/server/transactions/write_request.rs index b777c3fd82..226d57fc98 100644 --- a/system/rust/src/gatt/server/transactions/write_request.rs +++ b/system/rust/src/gatt/server/transactions/write_request.rs @@ -1,24 +1,20 @@ -use crate::{ - gatt::server::att_database::AttDatabase, - packets::{ - AttChild, AttErrorResponseBuilder, AttOpcode, AttWriteRequestView, AttWriteResponseBuilder, - }, -}; +use crate::{gatt::server::att_database::AttDatabase, packets::att}; +use pdl_runtime::EncodeError; pub async fn handle_write_request<T: AttDatabase>( - request: AttWriteRequestView<'_>, + request: att::AttWriteRequest, db: &T, -) -> AttChild { - let handle = request.get_handle().into(); - let value = request.get_value_iter().collect::<Vec<_>>(); +) -> Result<att::Att, EncodeError> { + let handle = request.handle.into(); + let value = request.value; match db.write_attribute(handle, &value).await { - Ok(()) => AttWriteResponseBuilder {}.into(), - Err(error_code) => AttErrorResponseBuilder { - opcode_in_error: AttOpcode::WRITE_REQUEST, + Ok(()) => att::AttWriteResponse {}.try_into(), + Err(error_code) => att::AttErrorResponse { + opcode_in_error: att::AttOpcode::WriteRequest, handle_in_error: handle.into(), error_code, } - .into(), + .try_into(), } } @@ -38,11 +34,7 @@ mod test { test::test_att_db::TestAttDatabase, }, }, - packets::{ - AttChild, AttErrorCode, AttErrorResponseBuilder, AttWriteRequestBuilder, - AttWriteResponseBuilder, - }, - utils::packet::build_view_or_crash, + packets::att, }; #[test] @@ -59,14 +51,11 @@ mod test { let data = vec![1, 2]; // act: write to the attribute - let att_view = build_view_or_crash(AttWriteRequestBuilder { - handle: AttHandle(1).into(), - value: data.clone().into_boxed_slice(), - }); - let resp = block_on(handle_write_request(att_view.view(), &db)); + let att_view = att::AttWriteRequest { handle: AttHandle(1).into(), value: data.clone() }; + let resp = block_on(handle_write_request(att_view, &db)); // assert: that the write succeeded - assert_eq!(resp, AttChild::from(AttWriteResponseBuilder {})); + assert_eq!(resp, att::AttWriteResponse {}.try_into()); assert_eq!(block_on(db.read_attribute(AttHandle(1))).unwrap(), data); } @@ -82,20 +71,18 @@ mod test { vec![], )]); // act: write to the attribute - let att_view = build_view_or_crash(AttWriteRequestBuilder { - handle: AttHandle(1).into(), - value: [1, 2].into(), - }); - let resp = block_on(handle_write_request(att_view.view(), &db)); + let att_view = att::AttWriteRequest { handle: AttHandle(1).into(), value: vec![1, 2] }; + let resp = block_on(handle_write_request(att_view, &db)); // assert: that the write failed assert_eq!( resp, - AttChild::from(AttErrorResponseBuilder { - opcode_in_error: AttOpcode::WRITE_REQUEST, + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::WriteRequest, handle_in_error: AttHandle(1).into(), - error_code: AttErrorCode::WRITE_NOT_PERMITTED - }) + error_code: att::AttErrorCode::WriteNotPermitted + } + .try_into() ); } } diff --git a/system/rust/src/packets.pdl b/system/rust/src/packets.pdl index 77fc970322..bb5f3de72e 100644 --- a/system/rust/src/packets.pdl +++ b/system/rust/src/packets.pdl @@ -133,32 +133,28 @@ struct GattCharacteristicProperties { extended_properties: 1, } -struct AttAttributeData { - _payload_ -} - -struct GattCharacteristicDeclarationValue : AttAttributeData { +struct GattCharacteristicDeclarationValue { properties: GattCharacteristicProperties, handle: AttHandle, uuid: Uuid, } -struct GattServiceDeclarationValue : AttAttributeData { +struct GattServiceDeclarationValue { uuid: Uuid, } -struct GattClientCharacteristicConfiguration : AttAttributeData { +struct GattClientCharacteristicConfiguration { notification: 1, indication: 1, _reserved_: 14, } -struct GattServiceChanged : AttAttributeData { +struct GattServiceChanged { start_handle: AttHandle, end_handle: AttHandle, } -struct UuidAsAttData : AttAttributeData { +struct UuidAsAttData { uuid: Uuid, } diff --git a/system/rust/src/packets.rs b/system/rust/src/packets.rs index a14103f1f7..55094c7f44 100644 --- a/system/rust/src/packets.rs +++ b/system/rust/src/packets.rs @@ -5,11 +5,6 @@ #![allow(clippy::all)] // this is now stable #![feature(mixed_integer_ops)] - -include!(concat!(env!("OUT_DIR"), "/_packets.rs")); - -impl std::cmp::PartialEq for SerializeError { - fn eq(&self, rhs: &Self) -> bool { - std::mem::discriminant(self) == std::mem::discriminant(rhs) - } +pub mod att { + include!(concat!(env!("OUT_DIR"), "/_packets.rs")); } diff --git a/system/rust/src/utils.rs b/system/rust/src/utils.rs index 242ff80399..ed6331a7af 100644 --- a/system/rust/src/utils.rs +++ b/system/rust/src/utils.rs @@ -1,7 +1,6 @@ //! Utilities that are not specific to a particular module pub mod owned_handle; -pub mod packet; #[cfg(test)] pub mod task; diff --git a/system/rust/src/utils/packet.rs b/system/rust/src/utils/packet.rs deleted file mode 100644 index 86039636b8..0000000000 --- a/system/rust/src/utils/packet.rs +++ /dev/null @@ -1,46 +0,0 @@ -//! Utility for packet manipulation on top of the codegen from PDL - -use crate::packets::{ - AttBuilder, AttChild, AttOpcode, Builder, OwnedAttView, OwnedPacket, Serializable, -}; - -/// Convert an ATT builder child into an owned AttView, for use in test -pub fn build_att_view_or_crash(child: impl Into<AttChild>) -> OwnedAttView { - let child = child.into(); - let opcode = HACK_child_to_opcode(&child); - let serialized = AttBuilder { _child_: child, opcode }.to_vec().unwrap(); - OwnedAttView::try_parse(serialized.into_boxed_slice()).unwrap() -} - -/// Convert an arbitrary packet builder into an OwnedView, for use in test -pub fn build_view_or_crash<T: Builder>(builder: T) -> T::OwnedPacket { - let buf = builder.to_vec().unwrap(); - T::OwnedPacket::try_parse(buf.into_boxed_slice()).unwrap() -} - -/// Hack to workaround PDL limitations where constraints are ignored in builders -/// TODO(aryarahul) - get rid of this, PDL should deal with it! -#[allow(non_snake_case)] -pub fn HACK_child_to_opcode(child: &AttChild) -> AttOpcode { - match child { - AttChild::RawData(_vec) => unreachable!(), - AttChild::AttFindInformationRequest(_) => AttOpcode::FIND_INFORMATION_REQUEST, - AttChild::AttReadByGroupTypeRequest(_) => AttOpcode::READ_BY_GROUP_TYPE_REQUEST, - AttChild::AttReadByTypeRequest(_) => AttOpcode::READ_BY_TYPE_REQUEST, - AttChild::AttReadRequest(_) => AttOpcode::READ_REQUEST, - AttChild::AttReadResponse(_) => AttOpcode::READ_RESPONSE, - AttChild::AttErrorResponse(_) => AttOpcode::ERROR_RESPONSE, - AttChild::AttReadByGroupTypeResponse(_) => AttOpcode::READ_BY_GROUP_TYPE_RESPONSE, - AttChild::AttReadByTypeResponse(_) => AttOpcode::READ_BY_TYPE_RESPONSE, - AttChild::AttFindInformationResponse(_) => AttOpcode::FIND_INFORMATION_RESPONSE, - AttChild::AttFindByTypeValueRequest(_) => AttOpcode::FIND_BY_TYPE_VALUE_REQUEST, - AttChild::AttFindByTypeValueResponse(_) => AttOpcode::FIND_BY_TYPE_VALUE_RESPONSE, - AttChild::AttWriteRequest(_) => AttOpcode::WRITE_REQUEST, - AttChild::AttWriteResponse(_) => AttOpcode::WRITE_RESPONSE, - AttChild::AttHandleValueIndication(_) => AttOpcode::HANDLE_VALUE_INDICATION, - AttChild::AttHandleValueConfirmation(_) => AttOpcode::HANDLE_VALUE_CONFIRMATION, - AttChild::AttExchangeMtuRequest(_) => AttOpcode::EXCHANGE_MTU_REQUEST, - AttChild::AttExchangeMtuResponse(_) => AttOpcode::EXCHANGE_MTU_RESPONSE, - AttChild::AttWriteCommand(_) => AttOpcode::WRITE_COMMAND, - } -} diff --git a/system/rust/tests/gatt_callbacks_test.rs b/system/rust/tests/gatt_callbacks_test.rs index 1a462da113..84409f376e 100644 --- a/system/rust/tests/gatt_callbacks_test.rs +++ b/system/rust/tests/gatt_callbacks_test.rs @@ -12,7 +12,7 @@ use bluetooth_core::{ ids::{AttHandle, ConnectionId, ServerId, TransactionId, TransportIndex}, mocks::mock_callbacks::{MockCallbackEvents, MockCallbacks}, }, - packets::AttErrorCode, + packets::att::AttErrorCode, }; use tokio::{sync::mpsc::UnboundedReceiver, task::spawn_local, time::Instant}; use utils::start_test; @@ -272,11 +272,11 @@ fn test_write_characteristic_response() { // provide a response with some error code let trans_id = pull_trans_id(&mut callbacks_rx).await; callback_manager - .send_response(CONN_ID, trans_id, Err(AttErrorCode::WRITE_NOT_PERMITTED)) + .send_response(CONN_ID, trans_id, Err(AttErrorCode::WriteNotPermitted)) .unwrap(); // assert: that the error code was received - assert_eq!(pending_write.await.unwrap(), Err(AttErrorCode::WRITE_NOT_PERMITTED)); + assert_eq!(pending_write.await.unwrap(), Err(AttErrorCode::WriteNotPermitted)); }); } @@ -295,7 +295,7 @@ fn test_response_timeout() { ); // assert: that we time-out after 15s - assert_eq!(pending_write.await.unwrap(), Err(AttErrorCode::UNLIKELY_ERROR)); + assert_eq!(pending_write.await.unwrap(), Err(AttErrorCode::UnlikelyError)); let time_slept = Instant::now().duration_since(time_sent); assert!(time_slept > Duration::from_secs(14)); assert!(time_slept < Duration::from_secs(16)); @@ -316,10 +316,10 @@ fn test_transaction_cleanup_after_timeout() { ); let trans_id = pull_trans_id(&mut callbacks_rx).await; // let it time out - assert_eq!(pending.await.unwrap(), Err(AttErrorCode::UNLIKELY_ERROR)); + assert_eq!(pending.await.unwrap(), Err(AttErrorCode::UnlikelyError)); // try responding to it now let resp = - callback_manager.send_response(CONN_ID, trans_id, Err(AttErrorCode::INVALID_HANDLE)); + callback_manager.send_response(CONN_ID, trans_id, Err(AttErrorCode::InvalidHandle)); // assert: the response failed assert_eq!(resp, Err(CallbackResponseError::NonExistentTransaction(trans_id))); @@ -344,7 +344,7 @@ fn test_listener_hang_up() { pending.await.unwrap_err(); // try responding to it now let resp = - callback_manager.send_response(CONN_ID, trans_id, Err(AttErrorCode::INVALID_HANDLE)); + callback_manager.send_response(CONN_ID, trans_id, Err(AttErrorCode::InvalidHandle)); // assert: we get the expected error assert_eq!(resp, Err(CallbackResponseError::ListenerHungUp(trans_id))); @@ -422,10 +422,10 @@ fn test_execute_characteristic_response() { // provide a response with some error code let trans_id = pull_trans_id(&mut callbacks_rx).await; callback_manager - .send_response(CONN_ID, trans_id, Err(AttErrorCode::WRITE_NOT_PERMITTED)) + .send_response(CONN_ID, trans_id, Err(AttErrorCode::WriteNotPermitted)) .unwrap(); // assert: that the error code was received - assert_eq!(pending_execute.await.unwrap(), Err(AttErrorCode::WRITE_NOT_PERMITTED)); + assert_eq!(pending_execute.await.unwrap(), Err(AttErrorCode::WriteNotPermitted)); }); } diff --git a/system/rust/tests/gatt_server_test.rs b/system/rust/tests/gatt_server_test.rs index 9cccd51bc3..cf91d35ad8 100644 --- a/system/rust/tests/gatt_server_test.rs +++ b/system/rust/tests/gatt_server_test.rs @@ -1,3 +1,4 @@ +use pdl_runtime::Packet; use std::{ rc::Rc, sync::{Arc, Mutex}, @@ -29,17 +30,7 @@ use bluetooth_core::{ GattModule, IndicationError, }, }, - packets::{ - AttBuilder, AttChild, AttErrorCode, AttErrorResponseBuilder, - AttFindByTypeValueRequestBuilder, AttFindInformationRequestBuilder, - AttFindInformationResponseChild, AttHandleValueConfirmationBuilder, - AttHandleValueIndicationBuilder, AttOpcode, AttReadByTypeRequestBuilder, - AttReadRequestBuilder, AttReadResponseBuilder, AttWriteRequestBuilder, - AttWriteResponseBuilder, GattCharacteristicDeclarationValueView, - GattClientCharacteristicConfigurationBuilder, GattServiceChangedBuilder, - GattServiceDeclarationValueBuilder, Packet, Serializable, UuidAsAttDataBuilder, - }, - utils::packet::build_att_view_or_crash, + packets::att::{self, AttErrorCode}, }; use tokio::{ @@ -69,7 +60,7 @@ const DESCRIPTOR_TYPE: Uuid = Uuid::new(0x0104); const DATA: [u8; 4] = [1, 2, 3, 4]; const ANOTHER_DATA: [u8; 4] = [5, 6, 7, 8]; -fn start_gatt_module() -> (gatt::server::GattModule, UnboundedReceiver<(TransportIndex, AttBuilder)>) +fn start_gatt_module() -> (gatt::server::GattModule, UnboundedReceiver<(TransportIndex, att::Att)>) { let (transport, transport_rx) = MockAttTransport::new(); let arbiter = IsolationManager::new(); @@ -119,28 +110,20 @@ fn test_service_read() { // act gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: SERVICE_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: SERVICE_HANDLE.into() }.try_into().unwrap(), ); let (tcb_idx, resp) = transport_rx.recv().await.unwrap(); // assert assert_eq!(tcb_idx, TCB_IDX); assert_eq!( - resp.to_vec(), - AttBuilder { - opcode: AttOpcode::READ_RESPONSE, - _child_: AttReadResponseBuilder { - value: GattServiceDeclarationValueBuilder { uuid: SERVICE_TYPE.into() } - .to_vec() - .unwrap() - .into(), - } - .into() + Ok(resp), + att::AttReadResponse { + value: att::GattServiceDeclarationValue { uuid: SERVICE_TYPE.into() } + .encode_to_vec() + .unwrap(), } - .to_vec() + .try_into() ); }) } @@ -157,23 +140,19 @@ fn test_server_closed_while_connected() { // act: read from the closed server gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: SERVICE_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: SERVICE_HANDLE.into() }.try_into().unwrap(), ); let (_, resp) = transport_rx.recv().await.unwrap(); // assert that the read failed, but that a response was provided - assert_eq!(resp.opcode, AttOpcode::ERROR_RESPONSE); assert_eq!( - resp._child_, - AttErrorResponseBuilder { - opcode_in_error: AttOpcode::READ_REQUEST, + Ok(resp), + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::ReadRequest, handle_in_error: SERVICE_HANDLE.into(), - error_code: AttErrorCode::INVALID_HANDLE + error_code: AttErrorCode::InvalidHandle } - .into() + .try_into() ) }); } @@ -187,10 +166,9 @@ fn test_characteristic_read() { // act gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: CHARACTERISTIC_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: CHARACTERISTIC_HANDLE.into() } + .try_into() + .unwrap(), ); let tx = if let MockDatastoreEvents::Read( TCB_IDX, @@ -208,13 +186,7 @@ fn test_characteristic_read() { // assert assert_eq!(tcb_idx, TCB_IDX); - assert_eq!( - resp, - AttBuilder { - opcode: AttOpcode::READ_RESPONSE, - _child_: AttReadResponseBuilder { value: DATA.into() }.into() - } - ); + assert_eq!(Ok(resp), att::AttReadResponse { value: DATA.into() }.try_into()); }) } @@ -227,11 +199,9 @@ fn test_characteristic_write() { // act gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttWriteRequestBuilder { - handle: CHARACTERISTIC_HANDLE.into(), - value: DATA.into(), - }) - .view(), + att::AttWriteRequest { handle: CHARACTERISTIC_HANDLE.into(), value: DATA.into() } + .try_into() + .unwrap(), ); let (tx, written_data) = if let MockDatastoreEvents::Write( TCB_IDX, @@ -250,13 +220,7 @@ fn test_characteristic_write() { // assert assert_eq!(tcb_idx, TCB_IDX); - assert_eq!( - resp, - AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttWriteResponseBuilder {}.into() - } - ); + assert_eq!(Ok(resp), att::AttWriteResponse {}.try_into()); assert_eq!(&DATA, written_data.as_slice()); }) } @@ -277,21 +241,18 @@ fn test_send_indication() { gatt.get_bearer(TCB_IDX) .unwrap() - .handle_packet(build_att_view_or_crash(AttHandleValueConfirmationBuilder {}).view()); + .handle_packet(att::AttHandleValueConfirmation {}.try_into().unwrap()); // assert assert!(matches!(pending_indication.await.unwrap(), Ok(()))); assert_eq!(tcb_idx, TCB_IDX); assert_eq!( - resp, - AttBuilder { - opcode: AttOpcode::HANDLE_VALUE_INDICATION, - _child_: AttHandleValueIndicationBuilder { - handle: CHARACTERISTIC_HANDLE.into(), - value: DATA.into(), - } - .into() + Ok(resp), + att::AttHandleValueIndication { + handle: CHARACTERISTIC_HANDLE.into(), + value: DATA.into(), } + .try_into() ); }) } @@ -330,11 +291,9 @@ fn test_write_to_descriptor() { // act gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttWriteRequestBuilder { - handle: DESCRIPTOR_HANDLE.into(), - value: DATA.into(), - }) - .view(), + att::AttWriteRequest { handle: DESCRIPTOR_HANDLE.into(), value: DATA.into() } + .try_into() + .unwrap(), ); let (tx, written_data) = if let MockDatastoreEvents::Write( TCB_IDX, @@ -353,13 +312,7 @@ fn test_write_to_descriptor() { // assert assert_eq!(tcb_idx, TCB_IDX); - assert_eq!( - resp, - AttBuilder { - opcode: AttOpcode::WRITE_RESPONSE, - _child_: AttWriteResponseBuilder {}.into() - } - ); + assert_eq!(Ok(resp), att::AttWriteResponse {}.try_into()); assert_eq!(&DATA, written_data.as_slice()); }) } @@ -395,16 +348,14 @@ fn test_multiple_servers() { // act: read from both connections gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: CHARACTERISTIC_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: CHARACTERISTIC_HANDLE.into() } + .try_into() + .unwrap(), ); gatt.get_bearer(ANOTHER_TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttReadRequestBuilder { - attribute_handle: CHARACTERISTIC_HANDLE.into(), - }) - .view(), + att::AttReadRequest { attribute_handle: CHARACTERISTIC_HANDLE.into() } + .try_into() + .unwrap(), ); // service the first read with `data` let MockDatastoreEvents::Read(TCB_IDX, _, _, tx) = data_rx_1.recv().await.unwrap() else { @@ -424,9 +375,9 @@ fn test_multiple_servers() { // assert: the responses were routed to the correct connections assert_eq!(tcb_idx_1, TCB_IDX); - assert_eq!(resp_1._child_.to_vec().unwrap(), DATA); + assert_eq!(Ok(resp_1), att::AttReadResponse { value: DATA.to_vec() }.try_into()); assert_eq!(tcb_idx_2, ANOTHER_TCB_IDX); - assert_eq!(resp_2._child_.to_vec().unwrap(), ANOTHER_DATA); + assert_eq!(Ok(resp_2), att::AttReadResponse { value: ANOTHER_DATA.to_vec() }.try_into()); }) } @@ -439,21 +390,27 @@ fn test_read_device_name() { // act: try to read the device name gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttReadByTypeRequestBuilder { + att::AttReadByTypeRequest { starting_handle: AttHandle(1).into(), ending_handle: AttHandle(0xFFFF).into(), attribute_type: DEVICE_NAME_UUID.into(), - }) - .view(), + } + .try_into() + .unwrap(), ); let (tcb_idx, resp) = transport_rx.recv().await.unwrap(); // assert: the name should not be readable assert_eq!(tcb_idx, TCB_IDX); - let AttChild::AttErrorResponse(resp) = resp._child_ else { - unreachable!("{resp:?}"); - }; - assert_eq!(resp.error_code, AttErrorCode::INSUFFICIENT_AUTHENTICATION); + assert_eq!( + Ok(resp), + att::AttErrorResponse { + opcode_in_error: att::AttOpcode::ReadByTypeRequest, + handle_in_error: AttHandle(1).into(), + error_code: AttErrorCode::InsufficientAuthentication, + } + .try_into() + ); }); } @@ -491,19 +448,19 @@ fn test_service_change_indication() { // act: discover the GATT server gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttFindByTypeValueRequestBuilder { + att::AttFindByTypeValueRequest { starting_handle: AttHandle::MIN.into(), ending_handle: AttHandle::MAX.into(), attribute_type: PRIMARY_SERVICE_DECLARATION_UUID.try_into().unwrap(), - attribute_value: UuidAsAttDataBuilder { uuid: GATT_SERVICE_UUID.into() } - .to_vec() - .unwrap() - .into(), - }) - .view(), + attribute_value: att::UuidAsAttData { uuid: GATT_SERVICE_UUID.into() } + .encode_to_vec() + .unwrap(), + } + .try_into() + .unwrap(), ); - let AttChild::AttFindByTypeValueResponse(resp) = - transport_rx.recv().await.unwrap().1._child_ + let Ok(resp): Result<att::AttFindByTypeValueResponse, _> = + transport_rx.recv().await.unwrap().1.try_into() else { unreachable!() }; @@ -513,29 +470,30 @@ fn test_service_change_indication() { ); // act: discover the service changed characteristic gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttReadByTypeRequestBuilder { + att::AttReadByTypeRequest { starting_handle, ending_handle, attribute_type: CHARACTERISTIC_UUID.into(), - }) - .view(), + } + .try_into() + .unwrap(), ); - let AttChild::AttReadByTypeResponse(resp) = transport_rx.recv().await.unwrap().1._child_ + + let Ok(resp): Result<att::AttReadByTypeResponse, _> = + transport_rx.recv().await.unwrap().1.try_into() else { unreachable!() }; let service_change_char_handle: AttHandle = resp .data - .into_vec() .into_iter() .find_map(|characteristic| { let value = characteristic.value.to_vec(); let decl = - GattCharacteristicDeclarationValueView::try_parse_from_buffer(value.as_slice()) - .unwrap(); + att::GattCharacteristicDeclarationValue::decode_full(value.as_slice()).unwrap(); - if SERVICE_CHANGE_UUID == decl.get_uuid().try_into().unwrap() { - Some(decl.get_handle().into()) + if SERVICE_CHANGE_UUID == decl.uuid.try_into().unwrap() { + Some(decl.handle.into()) } else { None } @@ -543,24 +501,23 @@ fn test_service_change_indication() { .unwrap(); // act: find the CCC descriptor for the service changed characteristic gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttFindInformationRequestBuilder { + att::AttFindInformationRequest { starting_handle: service_change_char_handle.into(), ending_handle: AttHandle::MAX.into(), - }) - .view(), + } + .try_into() + .unwrap(), ); - let AttChild::AttFindInformationResponse(resp) = - transport_rx.recv().await.unwrap().1._child_ + let Ok(resp): Result<att::AttFindInformationResponse, _> = + transport_rx.recv().await.unwrap().1.try_into() else { unreachable!() }; - let AttFindInformationResponseChild::AttFindInformationShortResponse(resp) = resp._child_ - else { + let Ok(resp): Result<att::AttFindInformationShortResponse, _> = resp.try_into() else { unreachable!() }; let service_change_descriptor_handle = resp .data - .into_vec() .into_iter() .find_map(|attr| { if attr.uuid == CLIENT_CHARACTERISTIC_CONFIGURATION_UUID.try_into().unwrap() { @@ -572,19 +529,21 @@ fn test_service_change_indication() { .unwrap(); // act: register for indications on this handle gatt.get_bearer(TCB_IDX).unwrap().handle_packet( - build_att_view_or_crash(AttWriteRequestBuilder { + att::AttWriteRequest { handle: service_change_descriptor_handle, - value: GattClientCharacteristicConfigurationBuilder { + value: att::GattClientCharacteristicConfiguration { notification: 0, indication: 1, } - .to_vec() - .unwrap() - .into(), - }) - .view(), + .encode_to_vec() + .unwrap(), + } + .try_into() + .unwrap(), ); - let AttChild::AttWriteResponse(_) = transport_rx.recv().await.unwrap().1._child_ else { + let Ok(_): Result<att::AttWriteResponse, _> = + transport_rx.recv().await.unwrap().1.try_into() + else { unreachable!() }; // act: add a new service @@ -601,19 +560,19 @@ fn test_service_change_indication() { .unwrap(); // assert: we got an indication - let AttChild::AttHandleValueIndication(indication) = - transport_rx.recv().await.unwrap().1._child_ + let Ok(indication): Result<att::AttHandleValueIndication, _> = + transport_rx.recv().await.unwrap().1.try_into() else { unreachable!() }; assert_eq!(indication.handle, service_change_char_handle.into()); assert_eq!( Ok(indication.value.into()), - GattServiceChangedBuilder { + att::GattServiceChanged { start_handle: AttHandle(30).into(), end_handle: AttHandle(30).into(), } - .to_vec() + .encode_to_vec() ); }); } |