embassy-rs / bt-hci

Apache License 2.0
19 stars 7 forks source link

add service and some appearance values #19

Closed jamessizeland closed 3 weeks ago

jamessizeland commented 1 month ago

Starting to implement this issue: https://github.com/embassy-rs/bt-hci/issues/17

alexmoon commented 1 month ago

I think we should have a newtype like BluetoothUuid16(u16) (with appropriate conversions) for this data.

Also, in the medium term I think it is important that we auto-generate this data from the Bluetooth SIG's bitbucket data.

jamessizeland commented 1 month ago

Yeah that sounds like a good idea

jamessizeland commented 1 month ago

making good progress on this, just have to deal with the variability in the naming conventions and fields.

jamessizeland commented 4 weeks ago

@alexmoon, @lulf here's a big bang of autogenerated UUIDs. I'm not sure why the CI is failing.

jamessizeland commented 4 weeks ago

Do we need the custom nightly formatting rules? I'm not sure what they are doing here.

lulf commented 4 weeks ago

@jamessizeland The nice thing is that they provide a consistent way to format imports. If you run cargo +nightly fmt (assuming you've added the nightly toolchain, that should automatically use those rules)

mrossington commented 4 weeks ago

Love this, great work :)

If the update_uuids is to run in CI. Should there be an associated github action?

jamessizeland commented 4 weeks ago

the formatter is suggeting a lot of the lifetimes in bt-hci can be elided, i.e.

error: the following explicit lifetimes could be elided: 'a
   --> src/transport.rs:144:6
    |
144 | impl<'a, T: HostToControllerPacket> WriteHci for WithIndicator<'a, T> {
    |      ^^                                                        ^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
help: elide the lifetimes
    |
144 - impl<'a, T: HostToControllerPacket> WriteHci for WithIndicator<'a, T> {
144 + impl<T: HostToControllerPacket> WriteHci for WithIndicator<'_, T> {
    |

looks like that's not stopping this from passing checks, but is it something to look at in future?

ScottGibb commented 4 weeks ago

Love this, great work :)

If the update_uuids is to run in CI. Should there be an associated github action?

I think this would be a really cool thing to add as well :)

jamessizeland commented 4 weeks ago

ok this is done now I think!

jamessizeland commented 4 weeks ago

Nice, all sounds good to me, I'll get that done. I shortened to BleUuid to make it less verbose but I agree that it's less specific, so I'll change it ☺️. This was fun to implement!

jamessizeland commented 3 weeks ago

Do you need any more changes to this @alexmoon?