embassy-rs / nrf-softdevice

Apache License 2.0
254 stars 74 forks source link

examples: Consistently of constants, comments and services #216

Closed chrysn closed 4 months ago

chrysn commented 6 months ago

This is a follow-up to https://github.com/embassy-rs/nrf-softdevice/pull/215

It does a few things (sorry, it's mixed; if you insist I can pull it apart but I hope it's fine):

Testing: I've flashed all changed examples onto an nrf52dk, and they passed startup (on the l2cap, I had to change counts to fit in the RAM). If the descriptors were broken, softdevice would have refused startup. I've done actual integration testing on the ble_dis_bas_peripheral_builder and blebas* examples.

Dirbaio commented 6 months ago

oh, #214 changed examples to use the builder, perhaps this is no longer relevant then

chrysn commented 6 months ago

Good point; and indeed that makes things even more readable.

I'll keep this issue open until I've started a new one where I apply the changes to the advertised services: Even after #214, dis_bas_peripheral advertises as a thermometer, where it should advertise discovery and battery services.

To fix this once and for all on the long run, we might at some point annotating the service with how it should be advertised, and then create the builder out of the server:

#[nrf_softdevice::gatt_service(uuid = "180f", advertise_in = "scan")]
struct BatteryService { ... }

let adv_data = Server::adv_data().flags([Flag::GeneralDiscovery]).name(ShortName("HelloRust"));
alexmoon commented 6 months ago

I've updated the advertising packets in #217. I think that should address the issues you found. Let me know if I missed anything.

plaes commented 5 months ago

@chrysn This PR has conflicts, though did PR #217 actually obsolete this one?

alexmoon commented 4 months ago

I think this has been covered by other PRs. Feel free to reopen if there's still something that needs to be addressed.