Adjust BLE/LL stacks, style, comments, refactoring

Increase BLE task stack +200 and decrease LL task stack -200
more braces!

src/components/ble/NimbleController.cpp

@@ -10,10 +10,10 @@
 #include <host/util/util.h>
 #include <controller/ble_ll.h>
 #include <controller/ble_hw.h>
-#undef max
-#undef min
 #include <services/gap/ble_svc_gap.h>
 #include <services/gatt/ble_svc_gatt.h>
+#undef max
+#undef min
 #include "components/ble/BleController.h"
 #include "components/ble/NotificationManager.h"
 #include "components/datetime/DateTimeController.h"
@@ -36,7 +36,9 @@ NimbleController::NimbleController(Pinetime::System::SystemTask& systemTask,
     dateTimeController {dateTimeController},
     notificationManager {notificationManager},
     spiNorFlash {spiNorFlash},
+    fs {fs},
     dfuService {systemTask, bleController, spiNorFlash},
+
     currentTimeClient {dateTimeController},
     anService {systemTask, notificationManager},
     alertNotificationClient {systemTask, notificationManager},
@@ -46,7 +48,6 @@ NimbleController::NimbleController(Pinetime::System::SystemTask& systemTask,
     batteryInformationService {batteryController},
     immediateAlertService {systemTask, notificationManager},
     heartRateService {systemTask, heartRateController},
-    fs {fs},
     motionService {systemTask, motionController},
     serviceDiscovery({&currentTimeClient, &alertNotificationClient}) {
 }
@@ -129,9 +130,10 @@ void NimbleController::Init() {
 
   RestoreBond();
 
-  if (!ble_gap_adv_active() && !bleController.IsConnected())
+  if (!ble_gap_adv_active() && !bleController.IsConnected()) {
     StartAdvertising();
   }
+}
 
 void NimbleController::StartAdvertising() {
   struct ble_gap_adv_params adv_params;
@@ -209,8 +211,9 @@ int NimbleController::OnGAPEvent(ble_gap_event* event) {
       NRF_LOG_INFO("Disconnect event : BLE_GAP_EVENT_DISCONNECT");
       NRF_LOG_INFO("disconnect reason=%d", event->disconnect.reason);
 
-      if (event->disconnect.conn.sec_state.bonded)
+      if (event->disconnect.conn.sec_state.bonded) {
         PersistBond(event->disconnect.conn);
+      }
 
       currentTimeClient.Reset();
       alertNotificationClient.Reset();
@@ -244,8 +247,9 @@ int NimbleController::OnGAPEvent(ble_gap_event* event) {
       if (event->enc_change.status == 0) {
         struct ble_gap_conn_desc desc;
         ble_gap_conn_find(event->enc_change.conn_handle, &desc);
-        if (desc.sec_state.bonded)
+        if (desc.sec_state.bonded) {
           PersistBond(desc);
+        }
 
         NRF_LOG_INFO("new state: encrypted=%d authenticated=%d bonded=%d key_size=%d",
                      desc.sec_state.encrypted,
@@ -257,8 +261,16 @@ int NimbleController::OnGAPEvent(ble_gap_event* event) {
 
     case BLE_GAP_EVENT_PASSKEY_ACTION:
       /* Authentication has been requested for this connection.
+       *
+       * BLE authentication is determined by the combination of I/O capabilities
+       * on the central and peripheral. When the peripheral is display only and
+       * the central has a keyboard and display then passkey auth is selected.
+       * When both the central and peripheral have displays and support yes/no
+       * buttons then numeric comparison is selected. We currently advertise
+       * display capability only so we only handle the "display" action here.
+       *
        * Standards insist that the rand() PRNG be deterministic.
-       * Use the nimble TRNG since rand() is predictable.
+       * Use the nimble TRNG here since rand() is predictable.
        */
       NRF_LOG_INFO("Security event : BLE_GAP_EVENT_PASSKEY_ACTION");
       if (event->passkey.params.action == BLE_SM_IOACT_DISP) {
@@ -372,8 +384,9 @@ void NimbleController::PersistBond(struct ble_gap_conn_desc& desc) {
   key.sec.peer_addr = desc.peer_id_addr;
   rc = ble_store_read_our_sec(&key.sec, &our_sec.sec);
 
-  if (memcmp(&our_sec.sec, &bondId, sizeof bondId) == 0)
+  if (memcmp(&our_sec.sec, &bondId, sizeof bondId) == 0) {
     return;
+  }
 
   memcpy(&bondId, &our_sec.sec, sizeof bondId);
 

src/components/ble/NimbleController.h

@@ -14,11 +14,11 @@
 #include "components/ble/CurrentTimeService.h"
 #include "components/ble/DeviceInformationService.h"
 #include "components/ble/DfuService.h"
+#include "components/ble/HeartRateService.h"
 #include "components/ble/ImmediateAlertService.h"
 #include "components/ble/MusicService.h"
 #include "components/ble/NavigationService.h"
 #include "components/ble/ServiceDiscovery.h"
-#include "components/ble/HeartRateService.h"
 #include "components/ble/MotionService.h"
 #include "components/fs/FS.h"
 
@@ -80,16 +80,17 @@ namespace Pinetime {
         fastAdvCount = 0;
       }
 
+    private:
       void PersistBond(struct ble_gap_conn_desc& desc);
       void RestoreBond();
 
-    private:
       static constexpr const char* deviceName = "InfiniTime";
       Pinetime::System::SystemTask& systemTask;
       Pinetime::Controllers::Ble& bleController;
       DateTime& dateTimeController;
       Pinetime::Controllers::NotificationManager& notificationManager;
       Pinetime::Drivers::SpiNorFlash& spiNorFlash;
+      Pinetime::Controllers::FS& fs;
       Pinetime::Controllers::DfuService dfuService;
 
       DeviceInformationService deviceInformationService;
@@ -103,9 +104,9 @@ namespace Pinetime {
       ImmediateAlertService immediateAlertService;
       HeartRateService heartRateService;
       MotionService motionService;
-      Pinetime::Controllers::FS& fs;
+      ServiceDiscovery serviceDiscovery;
 
-      uint8_t addrType; // 1 = Random, 0 = PUBLIC
+      uint8_t addrType;
       uint16_t connectionHandle = BLE_HS_CONN_HANDLE_NONE;
       uint8_t fastAdvCount = 0;
       uint8_t bondId[16] = {0};
@@ -113,8 +114,6 @@ namespace Pinetime {
       ble_uuid128_t dfuServiceUuid {
         .u {.type = BLE_UUID_TYPE_128},
         .value = {0x23, 0xD1, 0xBC, 0xEA, 0x5F, 0x78, 0x23, 0x15, 0xDE, 0xEF, 0x12, 0x12, 0x30, 0x15, 0x00, 0x00}};
-
-      ServiceDiscovery serviceDiscovery;
     };
 
     static NimbleController* nptr;

src/libs/mynewt-nimble/porting/npl/freertos/src/nimble_port_freertos.c

@@ -37,7 +37,7 @@ nimble_port_freertos_init(TaskFunction_t host_task_fn)
      * provided by NimBLE and in case of FreeRTOS it does not need to be wrapped
      * since it has compatible prototype.
      */
-    xTaskCreate(nimble_port_ll_task_func, "ll", configMINIMAL_STACK_SIZE + 400,
+    xTaskCreate(nimble_port_ll_task_func, "ll", configMINIMAL_STACK_SIZE + 200,
                 NULL, configMAX_PRIORITIES - 1, &ll_task_h);
 #endif
 
@@ -46,6 +46,6 @@ nimble_port_freertos_init(TaskFunction_t host_task_fn)
      * have separate task for NimBLE host, but since something needs to handle
      * default queue it is just easier to make separate task which does this.
      */
-    xTaskCreate(host_task_fn, "ble", configMINIMAL_STACK_SIZE + 400,
+    xTaskCreate(host_task_fn, "ble", configMINIMAL_STACK_SIZE + 600,
                 NULL, tskIDLE_PRIORITY + 1, &host_task_h);
 }

src/systemtask/SystemTask.cpp

@@ -259,8 +259,9 @@ void SystemTask::Work() {
           displayApp.PushMessage(Pinetime::Applications::Display::Messages::GoToRunning);
           heartRateApp.PushMessage(Pinetime::Applications::HeartRateTask::Messages::WakeUp);
 
-          if (!bleController.IsConnected())
+          if (!bleController.IsConnected()) {
             nimbleController.RestartFastAdv();
+          }
 
           isSleeping = false;
           isWakingUp = false;