From 4b46386dc5a7c1e310928ef1aa4e97a694728f14 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Mon, 7 Dec 2020 12:58:20 +0100 Subject: [PATCH 1/2] Use a trait for representing attribute values This makes it easier to use attributes in environments where using static lifetimes for attribute values is challenging by allowing the attribute service to use its own types that can choose how to store the attribute values. In addition, the usage of attribute in the AttributeProvider now takes a reference to the Attribute instead of a copy to allow the attribute service more freedom in implementing. --- rubble/src/att/mod.rs | 51 +++++++++++++++++++++++++++++----------- rubble/src/att/server.rs | 12 +++++----- rubble/src/gatt/mod.rs | 50 +++++++++------------------------------ 3 files changed, 54 insertions(+), 59 deletions(-) diff --git a/rubble/src/att/mod.rs b/rubble/src/att/mod.rs index b855f11eb..69d6f736c 100644 --- a/rubble/src/att/mod.rs +++ b/rubble/src/att/mod.rs @@ -35,47 +35,69 @@ mod server; mod uuid; use self::{handle::*, pdus::*}; -use crate::{utils::HexSlice, Error}; +use crate::Error; pub use self::handle::{Handle, HandleRange}; pub use self::server::{AttributeServer, AttributeServerTx}; pub use self::uuid::AttUuid; +/// An attribute value that can be represented as a byte slice. +pub trait AttrValue { + fn as_slice(&self) -> &[u8]; +} + +impl AttrValue for &[u8] { + fn as_slice(&self) -> &[u8] { + self + } +} + +impl AttrValue for () { + fn as_slice(&self) -> &[u8] { + &[] + } +} + /// An ATT server attribute -pub struct Attribute<'a> { +pub struct Attribute +where + T: AttrValue, +{ /// The type of the attribute as a UUID16, EG "Primary Service" or "Anaerobic Heart Rate Lower Limit" pub att_type: AttUuid, /// Unique server-side identifer for attribute pub handle: Handle, /// Attribute values can be any fixed length or variable length octet array, which if too large /// can be sent across multiple PDUs - pub value: HexSlice<&'a [u8]>, + pub value: T, } -impl<'a> Attribute<'a> { +impl Attribute { /// Creates a new attribute. - pub fn new(att_type: AttUuid, handle: Handle, value: &'a [u8]) -> Self { + pub fn new(att_type: AttUuid, handle: Handle, value: T) -> Self { assert_ne!(handle, Handle::NULL); Attribute { att_type, handle, - value: HexSlice(value), + value: value, } } /// Retrieves the attribute's value as a slice. - pub fn value(&self) -> &'a [u8] { - self.value.as_ref() + pub fn value(&self) -> &[u8] { + self.value.as_slice() } /// Overrides the previously set attribute's value. - pub fn set_value(&mut self, value: &'a [u8]) { - self.value = HexSlice(value) + pub fn set_value(&mut self, value: T) { + self.value = value; } } /// Trait for attribute sets that can be hosted by an `AttributeServer`. pub trait AttributeProvider { + type ValueType: AttrValue; + /// Calls a closure `f` with every attribute whose handle is inside `range`, ascending. /// /// If `f` returns an error, this function will stop calling `f` and propagate the error @@ -87,7 +109,7 @@ pub trait AttributeProvider { fn for_attrs_in_range( &mut self, range: HandleRange, - f: impl FnMut(&Self, Attribute<'_>) -> Result<(), Error>, + f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, ) -> Result<(), Error>; /// Returns whether `uuid` is a valid grouping attribute type that can be used in *Read By @@ -109,7 +131,7 @@ pub trait AttributeProvider { /// last attribute contained within that service. /// /// TODO: document what the BLE spec has to say about grouping for characteristics. - fn group_end(&self, handle: Handle) -> Option<&Attribute<'_>>; + fn group_end(&self, handle: Handle) -> Option<&Attribute>; } /// An empty attribute set. @@ -118,10 +140,11 @@ pub trait AttributeProvider { pub struct NoAttributes; impl AttributeProvider for NoAttributes { + type ValueType = (); fn for_attrs_in_range( &mut self, _range: HandleRange, - _f: impl FnMut(&Self, Attribute<'_>) -> Result<(), Error>, + _f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, ) -> Result<(), Error> { Ok(()) } @@ -130,7 +153,7 @@ impl AttributeProvider for NoAttributes { false } - fn group_end(&self, _handle: Handle) -> Option<&Attribute<'_>> { + fn group_end(&self, _handle: Handle) -> Option<&Attribute> { None } } diff --git a/rubble/src/att/server.rs b/rubble/src/att/server.rs index a7acba0e8..edd2f53c4 100644 --- a/rubble/src/att/server.rs +++ b/rubble/src/att/server.rs @@ -2,7 +2,7 @@ use super::{ pdus::{AttPdu, ByGroupAttData, ByTypeAttData, ErrorCode, Opcode}, - AttError, AttributeProvider, Handle, HandleRange, + AttError, AttrValue, AttributeProvider, Handle, HandleRange, }; use crate::bytes::{ByteReader, FromBytes, ToBytes}; use crate::l2cap::{Protocol, ProtocolObj, Sender}; @@ -99,7 +99,7 @@ impl AttributeServer { .for_attrs_in_range(range, |_provider, attr| { if attr.att_type == *attribute_type { let data = - ByTypeAttData::new(att_mtu, attr.handle, attr.value.as_ref()); + ByTypeAttData::new(att_mtu, attr.handle, attr.value.as_slice()); if size == Some(data.encoded_size()) || size.is_none() { // Can try to encode `data`. If we run out of space, end the list. data.to_bytes(writer)?; @@ -156,7 +156,7 @@ impl AttributeServer { att_mtu, attr.handle, provider.group_end(attr.handle).unwrap().handle, - attr.value.as_ref(), + attr.value.as_slice(), ); if size == Some(data.encoded_size()) || size.is_none() { // Can try to encode `data`. If we run out of space, end the list. @@ -197,10 +197,10 @@ impl AttributeServer { self.attrs.for_attrs_in_range( HandleRange::new(*handle, *handle), |_provider, attr| { - let value = if writer.space_left() < attr.value.as_ref().len() { - &attr.value.as_ref()[..writer.space_left()] + let value = if writer.space_left() < attr.value.as_slice().len() { + &attr.value.as_slice()[..writer.space_left()] } else { - attr.value.as_ref() + attr.value.as_slice() }; writer.write_slice(value) }, diff --git a/rubble/src/gatt/mod.rs b/rubble/src/gatt/mod.rs index f11def283..1cd65cd46 100644 --- a/rubble/src/gatt/mod.rs +++ b/rubble/src/gatt/mod.rs @@ -8,11 +8,11 @@ pub mod characteristic; use crate::att::{AttUuid, Attribute, AttributeProvider, Handle, HandleRange}; use crate::uuid::{Uuid128, Uuid16}; use crate::Error; -use core::{cmp, slice}; +use core::cmp; /// A demo `AttributeProvider` that will enumerate as a *Battery Service*. pub struct BatteryServiceAttrs { - attributes: [Attribute<'static>; 3], + attributes: [Attribute<&'static [u8]>; 3], } impl BatteryServiceAttrs { @@ -45,10 +45,11 @@ impl BatteryServiceAttrs { } impl AttributeProvider for BatteryServiceAttrs { + type ValueType = &'static [u8]; fn for_attrs_in_range( &mut self, range: HandleRange, - mut f: impl FnMut(&Self, Attribute<'_>) -> Result<(), Error>, + mut f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, ) -> Result<(), Error> { let count = self.attributes.len(); let start = usize::from(range.start().as_u16() - 1); // handles start at 1, not 0 @@ -62,14 +63,7 @@ impl AttributeProvider for BatteryServiceAttrs { }; for attr in attrs { - f( - self, - Attribute { - att_type: attr.att_type, - handle: attr.handle, - value: attr.value, - }, - )?; + f(self, attr)?; } Ok(()) } @@ -78,7 +72,7 @@ impl AttributeProvider for BatteryServiceAttrs { uuid == Uuid16(0x2800) // FIXME not characteristics? } - fn group_end(&self, handle: Handle) -> Option<&Attribute<'_>> { + fn group_end(&self, handle: Handle) -> Option<&Attribute> { match handle.as_u16() { 0x0001 => Some(&self.attributes[2]), 0x0002 => Some(&self.attributes[2]), @@ -87,27 +81,11 @@ impl AttributeProvider for BatteryServiceAttrs { } } -pub struct Attributes<'a> { - to_yield: slice::Iter<'a, Attribute<'a>>, -} - -impl<'a> Iterator for Attributes<'a> { - type Item = Attribute<'a>; - - fn next(&mut self) -> Option> { - self.to_yield.next().map(|attr| Attribute { - att_type: attr.att_type, - handle: attr.handle, - value: attr.value, - }) - } -} - /// A demo `AttributeProvider` that will enumerate as a *Midi Service*. /// /// Also refer to https://www.midi.org/specifications-old/item/bluetooth-le-midi pub struct MidiServiceAttrs { - attributes: [Attribute<'static>; 4], + attributes: [Attribute<&'static [u8]>; 4], } // MIDI Service (UUID: 03B80E5A-EDE8-4B33-A751-6CE34EC4C700) @@ -178,10 +156,11 @@ impl MidiServiceAttrs { } impl AttributeProvider for MidiServiceAttrs { + type ValueType = &'static [u8]; fn for_attrs_in_range( &mut self, range: HandleRange, - mut f: impl FnMut(&Self, Attribute<'_>) -> Result<(), Error>, + mut f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, ) -> Result<(), Error> { let count = self.attributes.len(); let start = usize::from(range.start().as_u16() - 1); // handles start at 1, not 0 @@ -195,14 +174,7 @@ impl AttributeProvider for MidiServiceAttrs { }; for attr in attrs { - f( - self, - Attribute { - att_type: attr.att_type, - handle: attr.handle, - value: attr.value, - }, - )?; + f(self, attr)?; } Ok(()) } @@ -211,7 +183,7 @@ impl AttributeProvider for MidiServiceAttrs { uuid == Uuid16(0x2800) // FIXME not characteristics? } - fn group_end(&self, handle: Handle) -> Option<&Attribute<'_>> { + fn group_end(&self, handle: Handle) -> Option<&Attribute> { match handle.as_u16() { 0x0001 => Some(&self.attributes[3]), 0x0002 => Some(&self.attributes[3]), From 21ea16ee929a6978a4e564d2376d6a8b083c0f69 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Tue, 8 Dec 2020 11:45:06 +0100 Subject: [PATCH 2/2] Use trait objects (dyn AttrValue) --- rubble/src/att/mod.rs | 13 +++++-------- rubble/src/att/server.rs | 2 +- rubble/src/gatt/mod.rs | 12 +++++------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/rubble/src/att/mod.rs b/rubble/src/att/mod.rs index 69d6f736c..51cea7d1f 100644 --- a/rubble/src/att/mod.rs +++ b/rubble/src/att/mod.rs @@ -61,7 +61,7 @@ impl AttrValue for () { /// An ATT server attribute pub struct Attribute where - T: AttrValue, + T: ?Sized, { /// The type of the attribute as a UUID16, EG "Primary Service" or "Anaerobic Heart Rate Lower Limit" pub att_type: AttUuid, @@ -96,8 +96,6 @@ impl Attribute { /// Trait for attribute sets that can be hosted by an `AttributeServer`. pub trait AttributeProvider { - type ValueType: AttrValue; - /// Calls a closure `f` with every attribute whose handle is inside `range`, ascending. /// /// If `f` returns an error, this function will stop calling `f` and propagate the error @@ -109,7 +107,7 @@ pub trait AttributeProvider { fn for_attrs_in_range( &mut self, range: HandleRange, - f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, + f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, ) -> Result<(), Error>; /// Returns whether `uuid` is a valid grouping attribute type that can be used in *Read By @@ -131,7 +129,7 @@ pub trait AttributeProvider { /// last attribute contained within that service. /// /// TODO: document what the BLE spec has to say about grouping for characteristics. - fn group_end(&self, handle: Handle) -> Option<&Attribute>; + fn group_end(&self, handle: Handle) -> Option<&Attribute>; } /// An empty attribute set. @@ -140,11 +138,10 @@ pub trait AttributeProvider { pub struct NoAttributes; impl AttributeProvider for NoAttributes { - type ValueType = (); fn for_attrs_in_range( &mut self, _range: HandleRange, - _f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, + _f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, ) -> Result<(), Error> { Ok(()) } @@ -153,7 +150,7 @@ impl AttributeProvider for NoAttributes { false } - fn group_end(&self, _handle: Handle) -> Option<&Attribute> { + fn group_end(&self, _handle: Handle) -> Option<&Attribute> { None } } diff --git a/rubble/src/att/server.rs b/rubble/src/att/server.rs index edd2f53c4..6117a59d2 100644 --- a/rubble/src/att/server.rs +++ b/rubble/src/att/server.rs @@ -2,7 +2,7 @@ use super::{ pdus::{AttPdu, ByGroupAttData, ByTypeAttData, ErrorCode, Opcode}, - AttError, AttrValue, AttributeProvider, Handle, HandleRange, + AttError, AttributeProvider, Handle, HandleRange, }; use crate::bytes::{ByteReader, FromBytes, ToBytes}; use crate::l2cap::{Protocol, ProtocolObj, Sender}; diff --git a/rubble/src/gatt/mod.rs b/rubble/src/gatt/mod.rs index 1cd65cd46..acd6e12f3 100644 --- a/rubble/src/gatt/mod.rs +++ b/rubble/src/gatt/mod.rs @@ -5,7 +5,7 @@ pub mod characteristic; -use crate::att::{AttUuid, Attribute, AttributeProvider, Handle, HandleRange}; +use crate::att::{AttUuid, AttrValue, Attribute, AttributeProvider, Handle, HandleRange}; use crate::uuid::{Uuid128, Uuid16}; use crate::Error; use core::cmp; @@ -45,11 +45,10 @@ impl BatteryServiceAttrs { } impl AttributeProvider for BatteryServiceAttrs { - type ValueType = &'static [u8]; fn for_attrs_in_range( &mut self, range: HandleRange, - mut f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, + mut f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, ) -> Result<(), Error> { let count = self.attributes.len(); let start = usize::from(range.start().as_u16() - 1); // handles start at 1, not 0 @@ -72,7 +71,7 @@ impl AttributeProvider for BatteryServiceAttrs { uuid == Uuid16(0x2800) // FIXME not characteristics? } - fn group_end(&self, handle: Handle) -> Option<&Attribute> { + fn group_end(&self, handle: Handle) -> Option<&Attribute> { match handle.as_u16() { 0x0001 => Some(&self.attributes[2]), 0x0002 => Some(&self.attributes[2]), @@ -156,11 +155,10 @@ impl MidiServiceAttrs { } impl AttributeProvider for MidiServiceAttrs { - type ValueType = &'static [u8]; fn for_attrs_in_range( &mut self, range: HandleRange, - mut f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, + mut f: impl FnMut(&Self, &Attribute) -> Result<(), Error>, ) -> Result<(), Error> { let count = self.attributes.len(); let start = usize::from(range.start().as_u16() - 1); // handles start at 1, not 0 @@ -183,7 +181,7 @@ impl AttributeProvider for MidiServiceAttrs { uuid == Uuid16(0x2800) // FIXME not characteristics? } - fn group_end(&self, handle: Handle) -> Option<&Attribute> { + fn group_end(&self, handle: Handle) -> Option<&Attribute> { match handle.as_u16() { 0x0001 => Some(&self.attributes[3]), 0x0002 => Some(&self.attributes[3]),