WIP Refactor ble advertising

Refactor ble advertising based on ble standards and conventions.
Changes are based on the bleprph example code, bluetooth docs, and nimble docs.

src/CMakeLists.txt

@@ -92,6 +92,9 @@ set(SDK_SOURCE_FILES
 set(TINYCRYPT_SRC
         libs/mynewt-nimble/ext/tinycrypt/src/aes_encrypt.c
         libs/mynewt-nimble/ext/tinycrypt/src/utils.c
+        libs/mynewt-nimble/ext/tinycrypt/src/cmac_mode.c
+        libs/mynewt-nimble/ext/tinycrypt/src/ecc.c
+        libs/mynewt-nimble/ext/tinycrypt/src/ecc_dh.c
         )
 
 set(NIMBLE_SRC
@@ -104,6 +107,10 @@ set(NIMBLE_SRC
         libs/mynewt-nimble/nimble/host/src/ble_l2cap.c
         libs/mynewt-nimble/nimble/host/src/ble_hs_mbuf.c
         libs/mynewt-nimble/nimble/host/src/ble_sm.c
+        libs/mynewt-nimble/nimble/host/src/ble_sm_cmd.c
+        libs/mynewt-nimble/nimble/host/src/ble_sm_lgcy.c
+        libs/mynewt-nimble/nimble/host/src/ble_sm_alg.c
+        libs/mynewt-nimble/nimble/host/src/ble_sm_sc.c
         libs/mynewt-nimble/nimble/host/src/ble_gap.c
         libs/mynewt-nimble/nimble/host/src/ble_gatts.c
         libs/mynewt-nimble/nimble/host/src/ble_gattc.c
@@ -127,10 +134,6 @@ set(NIMBLE_SRC
         libs/mynewt-nimble/nimble/host/src/ble_hs_atomic.c
         libs/mynewt-nimble/nimble/host/src/ble_hs_adv.c
         libs/mynewt-nimble/nimble/host/src/ble_hs_flow.c
-        libs/mynewt-nimble/nimble/host/src/ble_sm.c
-        libs/mynewt-nimble/nimble/host/src/ble_sm_cmd.c
-        libs/mynewt-nimble/nimble/host/src/ble_sm_lgcy.c
-        libs/mynewt-nimble/nimble/host/src/ble_sm_alg.c
         libs/mynewt-nimble/nimble/host/src/ble_hs_mqueue.c
         libs/mynewt-nimble/nimble/host/src/ble_hs_stop.c
         libs/mynewt-nimble/nimble/host/src/ble_hs_startup.c

src/components/ble/NimbleController.cpp

@@ -42,6 +42,19 @@ NimbleController::NimbleController(Pinetime::System::SystemTask& systemTask,
     serviceDiscovery({&currentTimeClient, &alertNotificationClient}) {
 }
 
+void nimble_on_reset(int reason) {
+  NRF_LOG_INFO("Resetting state; reason=%d\n", reason);
+}
+
+void nimble_on_sync(void) {
+    int rc;
+
+    rc = ble_hs_util_ensure_addr(0);
+    ASSERT(rc == 0);
+
+    nptr->StartAdvertising();
+}
+
 int GAPEventCallback(struct ble_gap_event* event, void* arg) {
   auto nimbleController = static_cast<NimbleController*>(arg);
   return nimbleController->OnGAPEvent(event);
@@ -51,6 +64,10 @@ void NimbleController::Init() {
   while (!ble_hs_synced()) {
   }
 
+  nptr = this;
+  ble_hs_cfg.reset_cb = nimble_on_reset;
+  ble_hs_cfg.sync_cb = nimble_on_sync;
+
   ble_svc_gap_init();
   ble_svc_gatt_init();
 
@@ -64,28 +81,31 @@ void NimbleController::Init() {
   batteryInformationService.Init();
   immediateAlertService.Init();
   heartRateService.Init();
-  int res;
+
-  res = ble_hs_util_ensure_addr(0);
+  int rc;
-  ASSERT(res == 0);
+  rc = ble_hs_util_ensure_addr(0);
-  res = ble_hs_id_infer_auto(0, &addrType);
+  ASSERT(rc == 0);
-  ASSERT(res == 0);
+  rc = ble_hs_id_infer_auto(0, &addrType);
-  res = ble_svc_gap_device_name_set(deviceName);
+  ASSERT(rc == 0);
-  ASSERT(res == 0);
+  rc = ble_svc_gap_device_name_set(deviceName);
+  ASSERT(rc == 0);
+  rc = ble_svc_gap_device_appearance_set(0xC2);
+  ASSERT(rc == 0);
   Pinetime::Controllers::Ble::BleAddress address;
-  res = ble_hs_id_copy_addr(addrType, address.data(), nullptr);
+  rc = ble_hs_id_copy_addr(addrType, address.data(), nullptr);
-  ASSERT(res == 0);
+  ASSERT(rc == 0);
   bleController.AddressType((addrType == 0) ? Ble::AddressTypes::Public : Ble::AddressTypes::Random);
   bleController.Address(std::move(address));
 
-  res = ble_gatts_start();
+  rc = ble_gatts_start();
-  ASSERT(res == 0);
+  ASSERT(rc == 0);
+
+  if (!ble_gap_adv_active() && !bleController.IsConnected())
+    StartAdvertising();
 }
 
 void NimbleController::StartAdvertising() {
-  if (bleController.IsConnected() || ble_gap_conn_active() || ble_gap_adv_active())
+  int rc;
-    return;
-
-  ble_svc_gap_device_name_set(deviceName);
 
   /* set adv parameters */
   struct ble_gap_adv_params adv_params;
@@ -104,9 +124,6 @@ void NimbleController::StartAdvertising() {
   adv_params.disc_mode = BLE_GAP_DISC_MODE_GEN;
 
   fields.flags = BLE_HS_ADV_F_DISC_GEN | BLE_HS_ADV_F_BREDR_UNSUP;
-  //  fields.uuids128 = BLE_UUID128(BLE_UUID128_DECLARE(
-  //          0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
-  //          0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff));
   fields.uuids128 = &dfuServiceUuid;
   fields.num_uuids128 = 1;
   fields.uuids128_is_complete = 1;
@@ -116,27 +133,24 @@ void NimbleController::StartAdvertising() {
   rsp_fields.name_len = strlen(deviceName);
   rsp_fields.name_is_complete = 1;
 
-  ble_gap_adv_set_fields(&fields);
+  rc = ble_gap_adv_set_fields(&fields);
-  //  ASSERT(res == 0); // TODO this one sometimes fails with error 22 (notsync)
+  ASSERT(rc == 0);
 
-  ble_gap_adv_rsp_set_fields(&rsp_fields);
+  rc = ble_gap_adv_rsp_set_fields(&rsp_fields);
-  //  ASSERT(res == 0);
+  ASSERT(rc == 0);
 
-  ble_gap_adv_start(addrType, NULL, 180000, &adv_params, GAPEventCallback, this);
+  rc = ble_gap_adv_start(addrType, NULL, 5000, &adv_params, GAPEventCallback, this);
-  //  ASSERT(res == 0);// TODO I've disabled these ASSERT as they sometime asserts and reset the mcu.
+  ASSERT(rc == 0);
-  // For now, the advertising is restarted as soon as it ends. There may be a race condition
-  // that prevent the advertising from restarting reliably.
-  // I remove the assert to prevent this uncesseray crash, but in the long term, the management of
-  // the advertising should be improve (better error handling, and advertise for 3 minutes after
-  // the application has been woken up, for example.
 }
 
 int NimbleController::OnGAPEvent(ble_gap_event* event) {
   switch (event->type) {
     case BLE_GAP_EVENT_ADV_COMPLETE:
       NRF_LOG_INFO("Advertising event : BLE_GAP_EVENT_ADV_COMPLETE");
-      NRF_LOG_INFO("advertise complete; reason=%dn status=%d", event->adv_complete.reason, event->connect.status);
+      NRF_LOG_INFO("reason=%d; status=%d", event->adv_complete.reason, event->connect.status);
+      StartAdvertising();
       break;
+
     case BLE_GAP_EVENT_CONNECT: {
       NRF_LOG_INFO("Advertising event : BLE_GAP_EVENT_CONNECT");
 
@@ -145,18 +159,19 @@ int NimbleController::OnGAPEvent(ble_gap_event* event) {
 
       if (event->connect.status != 0) {
         /* Connection failed; resume advertising. */
-        StartAdvertising();
         bleController.Disconnect();
+        StartAdvertising();
       } else {
+        connectionHandle = event->connect.conn_handle;
         bleController.Connect();
         systemTask.PushMessage(Pinetime::System::Messages::BleConnected);
-        connectionHandle = event->connect.conn_handle;
+        // Service discovery is deferred via systemtask
-        // Service discovery is deffered via systemtask
       }
     } break;
+
     case BLE_GAP_EVENT_DISCONNECT:
       NRF_LOG_INFO("Advertising event : BLE_GAP_EVENT_DISCONNECT");
-      NRF_LOG_INFO("disconnect; reason=%d", event->disconnect.reason);
+      NRF_LOG_INFO("disconnect reason=%d", event->disconnect.reason);
 
       /* Connection terminated; resume advertising. */
       currentTimeClient.Reset();
@@ -165,15 +180,18 @@ int NimbleController::OnGAPEvent(ble_gap_event* event) {
       bleController.Disconnect();
       StartAdvertising();
       break;
+
     case BLE_GAP_EVENT_CONN_UPDATE:
       NRF_LOG_INFO("Advertising event : BLE_GAP_EVENT_CONN_UPDATE");
       /* The central has updated the connection parameters. */
-      NRF_LOG_INFO("connection updated; status=%d ", event->conn_update.status);
+      NRF_LOG_INFO("update status=%d ", event->conn_update.status);
       break;
+
     case BLE_GAP_EVENT_ENC_CHANGE:
       /* Encryption has been enabled or disabled for this connection. */
       NRF_LOG_INFO("encryption change event; status=%d ", event->enc_change.status);
-      return 0;
+      break;
+
     case BLE_GAP_EVENT_SUBSCRIBE:
       NRF_LOG_INFO("subscribe event; conn_handle=%d attr_handle=%d "
                    "reason=%d prevn=%d curn=%d previ=%d curi=???\n",
@@ -183,10 +201,11 @@ int NimbleController::OnGAPEvent(ble_gap_event* event) {
                    event->subscribe.prev_notify,
                    event->subscribe.cur_notify,
                    event->subscribe.prev_indicate);
-      return 0;
+      break;
+
     case BLE_GAP_EVENT_MTU:
       NRF_LOG_INFO("mtu update event; conn_handle=%d cid=%d mtu=%d\n", event->mtu.conn_handle, event->mtu.channel_id, event->mtu.value);
-      return 0;
+      break;
 
     case BLE_GAP_EVENT_REPEAT_PAIRING: {
       /* We already have a bond with the peer, but it is attempting to
@@ -217,8 +236,7 @@ int NimbleController::OnGAPEvent(ble_gap_event* event) {
                    notifSize);
 
       alertNotificationClient.OnNotification(event);
-      return 0;
+    } break;
-    }
       /* Attribute data is contained in event->notify_rx.attr_data. */
 
     default:

src/components/ble/NimbleController.h

@@ -101,5 +101,7 @@ namespace Pinetime {
 
       ServiceDiscovery serviceDiscovery;
     };
+
+  static NimbleController* nptr;
   }
 }

src/systemtask/SystemTask.cpp

@@ -120,7 +120,6 @@ void SystemTask::Work() {
   fs.Init();
 
   nimbleController.Init();
-  nimbleController.StartAdvertising();
   lcd.Init();
 
   twiMaster.Init();
@@ -226,7 +225,6 @@ void SystemTask::Work() {
             touchPanel.Wakeup();
           }
 
-          nimbleController.StartAdvertising();
           xTimerStart(dimTimer, 0);
           spiNorFlash.Wakeup();
           lcd.Wakeup();