add 128 and 16 bit conversions to Uuid

[jamessizeland]

allow more ways to express Uuids in Gatt Service creation

pub const MATTER_BLE_SERVICE_UUID16: u16 = 0xFFF6;
pub const C1_CHARACTERISTIC_UUID: [u8; 16] = [0x18, 0xEE, 0x2E, 0xF5, 0x26, 0x3D, 0x45, 0x59, 0x95, 0x9F, 0x4F, 0x9C, 0x42, 0x9F, 0x9D, 0x11];
pub const C2_CHARACTERISTIC_UUID: u128 = 0x18EE2EF5263D4559959F4F9C429F9D12;

/// Matter service
#[gatt_service(uuid = MATTER_BLE_SERVICE_UUID16)]
struct MatterService {
    #[characteristic(uuid = C1_CHARACTERISTIC_UUID, write)]
    c1: u8,
    #[characteristic(uuid = C2_CHARACTERISTIC_UUID, write, indicate)]
    c2: u8,
    #[characteristic(uuid = "408813df-5dd4-1f87-ec11-cdb001100000")]
    status: bool,
}

[lure23]

Appreciated.

Observed in my testing:

If I use this:

const BB_SERVICE_UUID: u128 = 0x_92996405_8C0E_4FA1_A417_67D36995B563;      // "92996405-8C0E-4FA1-A417-67D36995B563";

..nRF Connect shows the service as "UUID: 63b59569-...-...-...-0e8c05649992", ie. in reverse order.

[lure23]

Should be .to_le_bytes(); see https://github.com/embassy-rs/trouble/blob/main/host/src/types/uuid.rs#L76

[jamessizeland]

I thought so, but then I was following @ivmarkov's type definition here: #248

[ivmarkov]

I thought so, but then I was following @ivmarkov's type definition here: #248

Mine was wrong, I corrected it in the meantime.

[jamessizeland]

hmm when I add in

impl From<u128> for Uuid {
    fn from(val: u128) -> Self {
        Uuid::Uuid128(val.to_be_bytes())
    }
}

I'm getting an ambiguity in type conversions elsewhere. This might need to be a NewType like BluetoothUuid16(u16) that we defined in https://github.com/embassy-rs/bt-hci/blob/main/src/uuid.rs

[jamessizeland]

I keep wondering if we need our own Uuid type here anyway or should fall back onto using https://crates.io/crates/uuid. At the moment we use this in the macro to allow smart conversion from &str to Uuid, but we don't pull it into the main host dependencies. I'm considering making it optional to allow things like:

use uuid::{uuid, Uuid};

pub const C2_CHARACTERISTIC_UUID: Uuid = Uuid::from_u128(0x18EE2EF5263D4559959F4F9C429F9D12);
pub const C4_CHARACTERISTIC_UUID: Uuid = uuid!("408813df-5dd4-1f87-ec11-cdb001100000");
/// Matter service
#[gatt_service(uuid = MATTER_BLE_SERVICE_UUID16)]
struct MatterService {
    #[characteristic(uuid = C2_CHARACTERISTIC_UUID, write, indicate)]
    c2: u8,
    #[characteristic(uuid = C4_CHARACTERISTIC_UUID, write, indicate)]
    status: bool,
}

But then at some point maybe we should just use that Uuid directly. I can't remember if we've discussed this in the past.

[lure23]

hmm when I add in

impl From<u128> for Uuid {
    fn from(val: u128) -> Self {
        Uuid::Uuid128(val.to_be_bytes())
    }
}

I'm getting an ambiguity in type conversions elsewhere. This might need to be a NewType like BluetoothUuid16(u16) that we defined in https://github.com/embassy-rs/bt-hci/blob/main/src/uuid.rs

In my similar code (not pushed), I used:

    fn from(data: u128) -> Self {
        Self::Uuid128(data.to_le_bytes())
    }

Imho, all those Uuid:: could be Self:: - but at least here it likely helps you. :)

[ivmarkov]

hmm when I add in

impl From<u128> for Uuid {
    fn from(val: u128) -> Self {
        Uuid::Uuid128(val.to_be_bytes())
    }
}

I'm getting an ambiguity in type conversions elsewhere. This might need to be a NewType like BluetoothUuid16(u16) that we defined in https://github.com/embassy-rs/bt-hci/blob/main/src/uuid.rs

The newtype kind of defeats the purpose. Given that Uuid is defined in trouble-host I wonder from where this ambiguity comes from and what exactly is the compiler error message?

[lulf]

Generally

hmm when I add in

impl From<u128> for Uuid {
    fn from(val: u128) -> Self {
        Uuid::Uuid128(val.to_be_bytes())
    }
}

I'm getting an ambiguity in type conversions elsewhere. This might need to be a NewType like BluetoothUuid16(u16) that we defined in https://github.com/embassy-rs/bt-hci/blob/main/src/uuid.rs

In my similar code (not pushed), I used:

    fn from(data: u128) -> Self {
        Self::Uuid128(data.to_le_bytes())
    }

I think it should be little endian, the spec uses that everywhere from what I've seen.

[jamessizeland]

You can see from the CI output the ambiguity adding the From adds in, it then stops assuming From is unambiguous. @lulf how do you feel about me pulling out trouble_host::Uuid and replacing with the Uuid crate? I've not dug deep enough to know if there's a good reason for not doing so.

error[E0277]: the trait bound `trouble_host::prelude::Uuid: From<i32>` is not satisfied
   --> tests/gatt.rs:51:54
    |
51  | ...dd_service(Service::new(0x1800));
    |               ------------ ^^^^^^ unsatisfied trait bound
    |               |
    |               required by a bound introduced by this call
    |
    = help: the trait `From<i32>` is not implemented for `trouble_host::prelude::Uuid`
    = help: the following other types implement trait `From<T>`:
              `trouble_host::prelude::Uuid` implements `From<BluetoothUuid16>`
              `trouble_host::prelude::Uuid` implements `From<[u8; 16]>`
              `trouble_host::prelude::Uuid` implements `From<[u8; 2]>`
              `trouble_host::prelude::Uuid` implements `From<u128>`
              `trouble_host::prelude::Uuid` implements `From<u16>`
    = note: required for `i32` to implement `Into<trouble_host::prelude::Uuid>`
note: required by a bound in `trouble_host::prelude::Service::new`

[lulf]

You can see from the CI output the ambiguity adding the From adds in, it then stops assuming From is unambiguous. @lulf how do you feel about me pulling out trouble_host::Uuid and replacing with the Uuid crate? I've not dug deep enough to know if there's a good reason for not doing so.

I'm not sure I understand, wouldn't just adding From to the type (casting to a u32 and then convert) fix this?
Edit: I forgot it's 16 bit! @ivmarkov's suggestion gets my vote.

The reason I added uuid was that it felt like adding a dependency on the uuid crate is a bit overkill when we don't need to support random generated UUIDs, just representing the bytes. Another point is that there are some restrictions on UUIDs in the BLE spec (i.e. they can't start with the prefixes that are used by spec UUIDs), that I think we should eventually encode into the Uuid type.

[ivmarkov]

You can see from the CI output the ambiguity adding the From adds in, it then stops assuming From is unambiguous. @lulf how do you feel about me pulling out trouble_host::Uuid and replacing with the Uuid crate? I've not dug deep enough to know if there's a good reason for not doing so.

error[E0277]: the trait bound `trouble_host::prelude::Uuid: From<i32>` is not satisfied
   --> tests/gatt.rs:51:54
    |
51  | ...dd_service(Service::new(0x1800));
    |               ------------ ^^^^^^ unsatisfied trait bound
    |               |
    |               required by a bound introduced by this call
    |
    = help: the trait `From<i32>` is not implemented for `trouble_host::prelude::Uuid`
    = help: the following other types implement trait `From<T>`:
              `trouble_host::prelude::Uuid` implements `From<BluetoothUuid16>`
              `trouble_host::prelude::Uuid` implements `From<[u8; 16]>`
              `trouble_host::prelude::Uuid` implements `From<[u8; 2]>`
              `trouble_host::prelude::Uuid` implements `From<u128>`
              `trouble_host::prelude::Uuid` implements `From<u16>`
    = note: required for `i32` to implement `Into<trouble_host::prelude::Uuid>`
note: required by a bound in `trouble_host::prelude::Service::new`

But that's normal as the system now does not know if you want to create a Uuid16 or a Uuid128.
This is easy to fix by just using:

dd_service(Service::new(0x1800u16));

[jamessizeland]

Cool, if we're happy with having to add the type suffix I can do that, I was trying to avoid doing that as the error could be confusing for users.

[jamessizeland]

/test