Reduce GAP & DIS service boilerplate

[jamessizeland]

Proposal that the Generic Access Profile (GAP) and Device Information Service (DIS) should be built into the boilerplate reduction efforts as part of trouble-host-macro crate.

Removing the need for all of the following code:

    let mut table: AttributeTable<'_, NoopRawMutex, MAX_ATTRIBUTES> = AttributeTable::new();

    // Generic Access Service (mandatory)
    let id = b"Trouble";
    let appearance = [0x80, 0x07];
    let mut svc = table.add_service(Service::new(0x1800));
    let _ = svc.add_characteristic_ro(0x2a00, id);
    let _ = svc.add_characteristic_ro(0x2a01, &appearance[..]);
    svc.build();

    // Generic attribute service (mandatory)
    table.add_service(Service::new(0x1801));

The application developer should still be able to configure the appropriate fields, but that they should be made mandatory and checked at compile time.

This could either be with more macro args or as a builder function on-top of Service::new().

Such as

let service = Service::new_default(stack, "trouBLE"); 
// Or
let service = Service::new_with_config(stack, gap_config, Some(dis_config));

This could also include enums with the standard uuids for i.e. generic ble device, generic mobile, etc.

[jamessizeland]

This would actually take out any requirement for table to be external to the Service entirely.

[plaes]

In nrf-softdevice, this was handled via "Config" struct:
https://github.com/embassy-rs/nrf-softdevice/blob/991d28e3c61687908acc53e748191eb59d00e27a/nrf-softdevice/src/softdevice.rs#L49-L64

[alexmoon]

I'm inclined to think that the Generic Access and Generic Attribute services should be automatically created when you construct the GattServer object, since a GATT server is not valid without those services. Handling requests to those services should be handled by the host so it can ensure correct behavior for qualification, with appropriate methods for the app to get/set the information it needs.

I think the Device Information Service is separate, since it is optional by the spec. It could be useful for trouble to provide a ready-to-use implementation of the DIS service, but that could also be provided by an external crate.

[jamessizeland]

ok I've got a kinda working PoC, I'm just wondering how to structure the GapConfig best. There are a few things that differ between Central and Peripheral that might be worth having as an enum? Also I can't find the reference for the default appearance here: https://bitbucket.org/bluetooth-SIG/public/src/main/assigned_numbers/core/appearance_values.yaml am I missing something? That would be best to be an enum with a custom option I reckon.

use crate::prelude::*;
use embassy_sync::blocking_mutex::raw::RawMutex;

pub enum GapConfig {
    Peripheral(PeripheralConfig),
    Central(CentralConfig),
}
pub struct PeripheralConfig {
    pub name: &'static str,
    pub appearance: &'static [u8; 2],
    // pub preferred_connection_parameters: Option<ConnectionParameters>,
}

pub struct CentralConfig {
    pub name: &'static str,
    pub appearance: &'static [u8; 2],
}

impl GapConfig {
    pub fn default(name: &'static str) -> Self {
        GapConfig::Peripheral(PeripheralConfig {
            name,
            appearance: &[0x80, 0x07],
        })
    }
    pub fn build<M: RawMutex, const MAX: usize>(self, table: &mut AttributeTable<'_, M, MAX>) {
        let mut gap = table.add_service(Service::new(0x1800)); // GAP UUID (mandatory)
        match self {
            GapConfig::Peripheral(config) => {
                let id = config.name.as_bytes();
                let _ = gap.add_characteristic_ro(0x2a00, id);
                let _ = gap.add_characteristic_ro(0x2a01, &config.appearance[..]);
            }
            GapConfig::Central(config) => {
                let id = config.name.as_bytes();
                let _ = gap.add_characteristic_ro(0x2a00, id);
                let _ = gap.add_characteristic_ro(0x2a01, &config.appearance[..]);
            }
        };
        gap.build();

        // Generic attribute service (mandatory)
        table.add_service(Service::new(0x1801)); // GATT UUID (mandatory)
    }
}

this gives an api that changes from:

// before
let mut table: AttributeTable<'_, NoopRawMutex, MAX_ATTRIBUTES> = AttributeTable::new();

// Generic Access Service (mandatory)
let id = b"Trouble";
let appearance = [0x80, 0x07];
let mut svc = table.add_service(Service::new(0x1800));
let _ = svc.add_characteristic_ro(0x2a00, id);
let _ = svc.add_characteristic_ro(0x2a01, &appearance[..]);
svc.build();

// Generic attribute service (mandatory)
table.add_service(Service::new(0x1801));

let server = Server::new(stack, &mut table);

to:

// after
let server = Server::new_with_config(
    stack,
    GapConfig::Peripheral(PeripheralConfig {
        name: "TrouBLE",
        appearance: &[0x80, 0x07],
    }),
);

or this:

let mut table: AttributeTable<'_, NoopRawMutex, 10> = AttributeTable::new();
GapConfig::default("trouBLE").build(&mut table);
let server: Server<common::Controller> = Server::new(stack, &mut table);

Lots of this should simplify when this proposal gets finished too I think.

[plaes]

I guess default appearance would be just 0;
Also, appearance is calculated like this: let app = (category << 6 | subcategory).to_le_bytes();

Link to spec: https://www.bluetooth.com/wp-content/uploads/Files/Specification/Assigned_Numbers.html#bookmark49