Remove start() calls from BLE initialization functions#106
Remove start() calls from BLE initialization functions#106zjwhitehead merged 4 commits intomasterfrom
Conversation
Remove explicit service->start() calls from init functions in config_service.cpp, fastlink_service.cpp, and ota_service.cpp. Services are intended to be started elsewhere (e.g. by the server lifecycle), so dropping these calls prevents duplicate starts and potential BLE initialization/race issues.
There was a problem hiding this comment.
Pull request overview
This PR removes start() calls from BLE service initialization helpers (and deletes an unused diagnostics helper), aiming to make BLE initialization a no-op change.
Changes:
- Remove an unused
zeroCoreDump()helper from diagnostics. - Remove
NimBLEService::start()calls from OTA, FastLink, and Config/Device Info BLE service init functions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/sp140/diagnostics.cpp | Removes an unused helper function. |
| src/sp140/ble/ota_service.cpp | Removes start() call for the OTA BLE service. |
| src/sp140/ble/fastlink_service.cpp | Removes start() call for the FastLink BLE service. |
| src/sp140/ble/config_service.cpp | Removes start() calls for Config and Device Info BLE services. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -368,5 +368,4 @@ void initOtaBleService(NimBLEServer* pServer) { | |||
| NIMBLE_PROPERTY::NOTIFY | | |||
| NIMBLE_PROPERTY::INDICATE); | |||
|
|
|||
There was a problem hiding this comment.
initOtaBleService no longer starts the created OTA service. With NimBLE-Arduino (^2.3.9), omitting start() can prevent the OTA service/characteristics from being registered in the GATT server, which would break OTA discovery/transfer. Please restore the service start() call or ensure services are started elsewhere before advertising/accepting connections.
| // Start OTA service so characteristics are registered with the GATT server | |
| pService->start(); |
| @@ -370,8 +370,6 @@ void initConfigBleService(NimBLEServer* server, const std::string& uniqueId) { | |||
| NimBLEUUID(DEVICE_UNIQUE_ID_UUID), kReadSecure); | |||
| uniqueIdCharacteristic->setValue(uniqueId); // Already uppercase string | |||
|
|
|||
There was a problem hiding this comment.
initConfigBleService no longer calls start() on the created GATT services. With NimBLE-Arduino (h2zero/NimBLE-Arduino ^2.3.9 in this repo), services generally must be started to be registered/visible to clients; otherwise the Config/Device Info services (and their characteristics) may not be discoverable after connecting. Either restore the start() calls here or ensure there is a single, explicit place that starts all created services before advertising begins.
| // Start services so they are registered and discoverable to BLE clients. | |
| configService->start(); | |
| deviceInfoService->start(); |
| @@ -224,7 +224,6 @@ void initFastLinkBleService(NimBLEServer *pServer) { | |||
| kFastLinkCommandProperties); | |||
| pCommandCharacteristic->setCallbacks(&commandCallbacks); | |||
|
|
|||
There was a problem hiding this comment.
initFastLinkBleService no longer starts the created service. Under NimBLE-Arduino (^2.3.9), a service typically needs start() to be added to the GATT server; without it, the FastLink telemetry/command characteristics may never be exposed to the app. Please either restore pService->start() here or start all services in a centralized spot after initialization.
| pService->start(); |
Call start() on BLE services after creating characteristics to ensure they become active and visible. Added service start calls in: - src/sp140/ble/config_service.cpp (configService and deviceInfoService) - src/sp140/ble/fastlink_service.cpp (pService) - src/sp140/ble/ota_service.cpp (pService) This fixes cases where characteristics were configured but the corresponding services were never started, preventing proper BLE exposure and notifications.
Should be no-op