From 73b9bb3ba48a9d2a5c92f1620d185225a603fb8d Mon Sep 17 00:00:00 2001 From: Pascal Sachs Date: Thu, 24 Oct 2024 15:10:09 +0200 Subject: [PATCH 1/3] Fix several clang-tidiy issues * Add unused annotations * Fix missing explicit casting issues * Remove "_" prefix from header guards and make them readable --- src/BleClient.h | 6 +++--- src/BleClientCallback.h | 11 +++++------ src/NimBleClient.cpp | 8 ++++---- src/NimBleClient.h | 14 ++++++++------ src/Sensirion_upt_ble_auto_detection.cpp | 14 ++++++-------- src/Sensirion_upt_ble_auto_detection.h | 22 +++++++++++----------- 6 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/BleClient.h b/src/BleClient.h index c7a8439..68f955a 100644 --- a/src/BleClient.h +++ b/src/BleClient.h @@ -1,5 +1,5 @@ -#ifndef _SENSIRION_UPT_BLE_AUTO_DETECTION_BLECLIENT_H -#define _SENSIRION_UPT_BLE_AUTO_DETECTION_BLECLIENT_H +#ifndef SENSIRION_UPT_BLE_AUTO_DETECTION_BLE_CLIENT_H +#define SENSIRION_UPT_BLE_AUTO_DETECTION_BLE_CLIENT_H #include "Arduino.h" #include "BleClientCallback.h" @@ -13,4 +13,4 @@ class BleClient { virtual void keepAlive() = 0; }; -#endif /* _SENSIRION_UPT_BLE_AUTO_DETECTION_BLECLIENT_H */ +#endif /* SENSIRION_UPT_BLE_AUTO_DETECTION_BLE_CLIENT_H */ diff --git a/src/BleClientCallback.h b/src/BleClientCallback.h index 3106190..e4216dd 100644 --- a/src/BleClientCallback.h +++ b/src/BleClientCallback.h @@ -1,13 +1,12 @@ -#ifndef _SENSIRION_UPT_BLE_AUTO_DETECTION_BLECLIENTCALLBACK_H -#define _SENSIRION_UPT_BLE_AUTO_DETECTION_BLECLIENTCALLBACK_H +#ifndef SENSIRION_UPT_BLE_AUTO_DETECTION_BLE_CLIENT_CALLBACK_H +#define SENSIRION_UPT_BLE_AUTO_DETECTION_BLE_CLIENT_CALLBACK_H class BleClientCallback { public: - virtual ~BleClientCallback() { - } + virtual ~BleClientCallback() = default; virtual void onAdvertisementReceived(std::string address, std::string name, - std::string data); + std::string data) = 0; }; -#endif /* _SENSIRION_UPT_BLE_AUTO_DETECTION_BLECLIENTCALLBACK_H */ +#endif /* SENSIRION_UPT_BLE_AUTO_DETECTION_BLE_CLIENT_CALLBACK_H */ diff --git a/src/NimBleClient.cpp b/src/NimBleClient.cpp index b5b01ef..6e2ffdc 100644 --- a/src/NimBleClient.cpp +++ b/src/NimBleClient.cpp @@ -3,11 +3,11 @@ void NimBleClient::begin(BleClientCallback* callback) { _callback = callback; setupAndStartBleScans(); -}; +} void NimBleClient::keepAlive() { // If an error occurs that stops the scan, it will be restarted here. - if (_bleScan->isScanning() == false) { + if (!_bleScan->isScanning()) { // Start scan with: duration = 0 seconds(forever), no scan end callback, // not a continuation of a previous scan. _bleScan->start(0, nullptr, false); @@ -47,7 +47,7 @@ void NimBleClient::onResult(NimBLEAdvertisedDevice* advertisedDevice) { const uint8_t* bleMACAddress = advertisedDevice->getAddress().getNative(); std::string address; for (int i = 5; i >= 0; i--) { - address.push_back(bleMACAddress[i]); + address.push_back(static_cast(bleMACAddress[i])); } std::string name = advertisedDevice->haveName() @@ -56,4 +56,4 @@ void NimBleClient::onResult(NimBLEAdvertisedDevice* advertisedDevice) { std::string manufacturerData = advertisedDevice->getManufacturerData(); _callback->onAdvertisementReceived(address, name, manufacturerData); -}; +} diff --git a/src/NimBleClient.h b/src/NimBleClient.h index 23918b3..e03bb59 100644 --- a/src/NimBleClient.h +++ b/src/NimBleClient.h @@ -1,12 +1,14 @@ -#ifndef _SENSIRION_UPT_BLE_AUTO_DETECTION_NIMBLECLIENT_H -#define _SENSIRION_UPT_BLE_AUTO_DETECTION_NIMBLECLIENT_H +#ifndef SENSIRION_UPT_BLE_AUTO_DETECTION_NIMBLE_CLIENT_H +#define SENSIRION_UPT_BLE_AUTO_DETECTION_NIMBLE_CLIENT_H #include "BleClient.h" #include "NimBLEDevice.h" -class NimBleClient: public BleClient, public NimBLEAdvertisedDeviceCallbacks { +class __attribute__((unused)) NimBleClient + : public BleClient, + public NimBLEAdvertisedDeviceCallbacks { public: - NimBleClient() : _bleScan(nullptr), _callback(nullptr){}; + NimBleClient() : _bleScan(nullptr), _callback(nullptr) {}; void begin(BleClientCallback* callback) override; void keepAlive() override; @@ -14,7 +16,7 @@ class NimBleClient: public BleClient, public NimBLEAdvertisedDeviceCallbacks { NimBLEScan* _bleScan; BleClientCallback* _callback; void setupAndStartBleScans(); - void onResult(NimBLEAdvertisedDevice* advertisedDevice); + void onResult(NimBLEAdvertisedDevice* advertisedDevice) override; }; -#endif /* _SENSIRION_UPT_BLE_AUTO_DETECTION_NIMBLECLIENT_H */ +#endif /* SENSIRION_UPT_BLE_AUTO_DETECTION_NIMBLE_CLIENT_H */ diff --git a/src/Sensirion_upt_ble_auto_detection.cpp b/src/Sensirion_upt_ble_auto_detection.cpp index 9a57a89..e868557 100644 --- a/src/Sensirion_upt_ble_auto_detection.cpp +++ b/src/Sensirion_upt_ble_auto_detection.cpp @@ -9,15 +9,15 @@ void SensiScan::begin() { _bleClient->keepAlive(); } -void SensiScan::getScanResults( +__attribute__((unused)) void SensiScan::getScanResults( std::map>& scanResults) { for (const auto& cachedSample : _sampleCache) { - scanResults[cachedSample.first] = _sampleCache[cachedSample.first]; + scanResults[cachedSample.first] = cachedSample.second; } _sampleCache.clear(); } -void SensiScan::keepAlive() { +__attribute__((unused)) void SensiScan::keepAlive() { _bleClient->keepAlive(); } @@ -45,7 +45,7 @@ void SensiScan::onAdvertisementReceived(const std::string address, return; } - // Last two digits of MAC addr. suffice to uniquely ID a BLE device + // Last two digits of MAC addr. Suffice to uniquely ID a BLE device _sampleCache[getDeviceId(data)] = samples; } @@ -55,7 +55,7 @@ void SensiScan::onAdvertisementReceived(const std::string address, */ uint64_t SensiScan::squashMACAddress(const std::string& macAddressString) { - uint64_t deviceID = macAddressString[0]; + uint64_t deviceID = static_cast(macAddressString[0]); for (size_t i = 1; i < 6; i++) { deviceID = (deviceID << 8) | macAddressString[i]; } @@ -71,8 +71,6 @@ uint16_t SensiScan::getDeviceId(const std::string& data) { return deviceId; } -extern std::map sampleConfigSelector; - /** * @brief decode chunk of Advertisement containing encoded samples * @@ -81,7 +79,7 @@ extern std::map sampleConfigSelector; */ uint8_t SensiScan::decodeData(const MetaData& metaData, const std::string& data, std::vector& samples) { - uint8_t sampleType = static_cast(data[3]); + auto sampleType = static_cast(data[3]); DataType dataType = getDataTypeFromSampleType(sampleType); SampleConfig sampleConfig = sampleConfigSelector[dataType]; diff --git a/src/Sensirion_upt_ble_auto_detection.h b/src/Sensirion_upt_ble_auto_detection.h index e94dc2a..054f125 100644 --- a/src/Sensirion_upt_ble_auto_detection.h +++ b/src/Sensirion_upt_ble_auto_detection.h @@ -1,5 +1,5 @@ -#ifndef _SENSIRION_UPT_BLE_AUTO_DETECTION_H_ -#define _SENSIRION_UPT_BLE_AUTO_DETECTION_H_ +#ifndef SENSIRION_UPT_BLE_AUTO_DETECTION_H +#define SENSIRION_UPT_BLE_AUTO_DETECTION_H #include "Arduino.h" #include "BleClient.h" @@ -8,24 +8,24 @@ #include #include -class SensiScan: public BleClientCallback { +class __attribute__((unused)) SensiScan: public BleClientCallback { public: - explicit SensiScan() : _bleClient(nullptr){}; + explicit SensiScan() : _bleClient(nullptr) {}; void begin(); - void + __attribute__((unused)) void getScanResults(std::map>& scanResults); - void keepAlive(); + __attribute__((unused)) void keepAlive(); private: BleClient* _bleClient; std::map> _sampleCache; void onAdvertisementReceived(std::string address, std::string name, std::string data) override; - uint64_t squashMACAddress(const std::string& macAddressString); - uint16_t getDeviceId(const std::string& data); - uint8_t decodeData(const MetaData& metaData, const std::string& data, - std::vector& samples); + static uint64_t squashMACAddress(const std::string& macAddressString); + static uint16_t getDeviceId(const std::string& data); + static uint8_t decodeData(const MetaData& metaData, const std::string& data, + std::vector& samples); }; -#endif /* _SENSIRION_UPT_BLE_AUTO_DETECTION_H_ */ +#endif /* SENSIRION_UPT_BLE_AUTO_DETECTION_H */ From 099fb9b30e540cc71a23f76ca287a1196e032d24 Mon Sep 17 00:00:00 2001 From: Pascal Sachs Date: Thu, 24 Oct 2024 16:06:32 +0200 Subject: [PATCH 2/3] Improve initialization process for NimBLE client Rename setupAndStartBleScans to setupBleScans since it only configures the BLE scanning. Create new internal function to start BLE scans that gets called from the begin() method. keepAlive doesn't need to be called on init to start the scanning. It can be used to re-start in case of errors as documented. --- src/NimBleClient.cpp | 52 ++++++++++++++---------- src/NimBleClient.h | 3 +- src/Sensirion_upt_ble_auto_detection.cpp | 1 - 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/NimBleClient.cpp b/src/NimBleClient.cpp index 6e2ffdc..3c8856d 100644 --- a/src/NimBleClient.cpp +++ b/src/NimBleClient.cpp @@ -2,7 +2,8 @@ void NimBleClient::begin(BleClientCallback* callback) { _callback = callback; - setupAndStartBleScans(); + setupBleScans(); + startBleScans(); } void NimBleClient::keepAlive() { @@ -10,11 +11,33 @@ void NimBleClient::keepAlive() { if (!_bleScan->isScanning()) { // Start scan with: duration = 0 seconds(forever), no scan end callback, // not a continuation of a previous scan. - _bleScan->start(0, nullptr, false); + startBleScans(); } } -void NimBleClient::setupAndStartBleScans() { + +void NimBleClient::onResult(NimBLEAdvertisedDevice* advertisedDevice) { + if (!advertisedDevice->haveManufacturerData()) { + return; + } + + // MAC address contains 6 bytes of MAC address (in reversed order) + // (Note: advertisedDevice->getAddress().toString() seems broken) + const uint8_t* bleMACAddress = advertisedDevice->getAddress().getNative(); + std::string address; + for (int i = 5; i >= 0; i--) { + address.push_back(static_cast(bleMACAddress[i])); + } + + std::string name = advertisedDevice->haveName() + ? advertisedDevice->getName() + : "UNDEFINED"; + std::string manufacturerData = advertisedDevice->getManufacturerData(); + + _callback->onAdvertisementReceived(address, name, manufacturerData); +} + +void NimBleClient::setupBleScans() { // CONFIG_BTDM_SCAN_DUPL_TYPE_DATA_DEVICE (2) // Filter by address and data, advertisements from the same address will be // reported only once, except if the data in the advertisement has changed, @@ -37,23 +60,8 @@ void NimBleClient::setupAndStartBleScans() { _bleScan->setMaxResults(0); } -void NimBleClient::onResult(NimBLEAdvertisedDevice* advertisedDevice) { - if (!advertisedDevice->haveManufacturerData()) { - return; - } - - // MAC address contains 6 bytes of MAC address (in reversed order) - // (Note: advertisedDevice->getAddress().toString() seems broken) - const uint8_t* bleMACAddress = advertisedDevice->getAddress().getNative(); - std::string address; - for (int i = 5; i >= 0; i--) { - address.push_back(static_cast(bleMACAddress[i])); - } - - std::string name = advertisedDevice->haveName() - ? advertisedDevice->getName() - : "UNDEFINED"; - std::string manufacturerData = advertisedDevice->getManufacturerData(); - - _callback->onAdvertisementReceived(address, name, manufacturerData); +void NimBleClient::startBleScans() { + // Start scan with: duration = 0 seconds(forever), no scan end callback, + // not a continuation of a previous scan. + _bleScan->start(0, nullptr, false); } diff --git a/src/NimBleClient.h b/src/NimBleClient.h index e03bb59..61574c1 100644 --- a/src/NimBleClient.h +++ b/src/NimBleClient.h @@ -15,7 +15,8 @@ class __attribute__((unused)) NimBleClient private: NimBLEScan* _bleScan; BleClientCallback* _callback; - void setupAndStartBleScans(); + void setupBleScans(); + void startBleScans(); void onResult(NimBLEAdvertisedDevice* advertisedDevice) override; }; diff --git a/src/Sensirion_upt_ble_auto_detection.cpp b/src/Sensirion_upt_ble_auto_detection.cpp index e868557..2dc734a 100644 --- a/src/Sensirion_upt_ble_auto_detection.cpp +++ b/src/Sensirion_upt_ble_auto_detection.cpp @@ -6,7 +6,6 @@ const int COMPANY_ID_FILTER = 54534; void SensiScan::begin() { _bleClient = new NimBleClient(); _bleClient->begin(this); - _bleClient->keepAlive(); } __attribute__((unused)) void SensiScan::getScanResults( From ecfe84a9e4dcdcd6f734dd210f84da1e4228dc1a Mon Sep 17 00:00:00 2001 From: Pascal Sachs Date: Thu, 24 Oct 2024 17:49:39 +0200 Subject: [PATCH 3/3] Improve BLE interface Use uint64_t as interface between NimBLE client and SensiScan as we only convert it into a invalid string and back into an uint64_t. Add constructor and destructure to NimBLE client to better represent the life-cycle of this class. Remove squash method from SensiScan as it is not required anymore. --- src/BleClientCallback.h | 2 +- src/NimBleClient.cpp | 37 ++++++++++++++---------- src/NimBleClient.h | 3 +- src/Sensirion_upt_ble_auto_detection.cpp | 24 +++------------ src/Sensirion_upt_ble_auto_detection.h | 3 +- 5 files changed, 29 insertions(+), 40 deletions(-) diff --git a/src/BleClientCallback.h b/src/BleClientCallback.h index e4216dd..463c452 100644 --- a/src/BleClientCallback.h +++ b/src/BleClientCallback.h @@ -5,7 +5,7 @@ class BleClientCallback { public: virtual ~BleClientCallback() = default; - virtual void onAdvertisementReceived(std::string address, std::string name, + virtual void onAdvertisementReceived(uint64_t address, std::string name, std::string data) = 0; }; diff --git a/src/NimBleClient.cpp b/src/NimBleClient.cpp index 3c8856d..839776a 100644 --- a/src/NimBleClient.cpp +++ b/src/NimBleClient.cpp @@ -1,8 +1,23 @@ #include "NimBleClient.h" +NimBleClient::NimBleClient() : _callback(nullptr) { + // CONFIG_BTDM_SCAN_DUPL_TYPE_DATA_DEVICE (2) + // Filter by address and data, advertisements from the same address will be + // reported only once, except if the data in the advertisement has changed, + // then it will be reported again. + NimBLEDevice::setScanFilterMode(CONFIG_BTDM_SCAN_DUPL_TYPE_DATA_DEVICE); + NimBLEDevice::setScanDuplicateCacheSize(200); + NimBLEDevice::init(""); + _bleScan = NimBLEDevice::getScan(); + setupBleScans(); +} + +NimBleClient::~NimBleClient() { + _bleScan->stop(); +} + void NimBleClient::begin(BleClientCallback* callback) { _callback = callback; - setupBleScans(); startBleScans(); } @@ -15,18 +30,18 @@ void NimBleClient::keepAlive() { } } - void NimBleClient::onResult(NimBLEAdvertisedDevice* advertisedDevice) { if (!advertisedDevice->haveManufacturerData()) { return; } // MAC address contains 6 bytes of MAC address (in reversed order) - // (Note: advertisedDevice->getAddress().toString() seems broken) const uint8_t* bleMACAddress = advertisedDevice->getAddress().getNative(); - std::string address; - for (int i = 5; i >= 0; i--) { - address.push_back(static_cast(bleMACAddress[i])); + uint64_t address = 0; + size_t address_size = 6; + // reverse MAC address and store it as 64-bit unsigned int + for (int ix = 0; ix < address_size; ix++) { + address = (address << 8) | bleMACAddress[address_size - 1 - ix]; } std::string name = advertisedDevice->haveName() @@ -38,16 +53,6 @@ void NimBleClient::onResult(NimBLEAdvertisedDevice* advertisedDevice) { } void NimBleClient::setupBleScans() { - // CONFIG_BTDM_SCAN_DUPL_TYPE_DATA_DEVICE (2) - // Filter by address and data, advertisements from the same address will be - // reported only once, except if the data in the advertisement has changed, - // then it will be reported again. - NimBLEDevice::setScanFilterMode(CONFIG_BTDM_SCAN_DUPL_TYPE_DATA_DEVICE); - NimBLEDevice::setScanDuplicateCacheSize(200); - NimBLEDevice::init(""); - - // create new scan - _bleScan = NimBLEDevice::getScan(); // Activate callback on advertisement update _bleScan->setAdvertisedDeviceCallbacks(this, true); // Set active scanning, this will get more data from the advertiser. diff --git a/src/NimBleClient.h b/src/NimBleClient.h index 61574c1..0de06cc 100644 --- a/src/NimBleClient.h +++ b/src/NimBleClient.h @@ -8,7 +8,8 @@ class __attribute__((unused)) NimBleClient : public BleClient, public NimBLEAdvertisedDeviceCallbacks { public: - NimBleClient() : _bleScan(nullptr), _callback(nullptr) {}; + NimBleClient(); + ~NimBleClient(); void begin(BleClientCallback* callback) override; void keepAlive() override; diff --git a/src/Sensirion_upt_ble_auto_detection.cpp b/src/Sensirion_upt_ble_auto_detection.cpp index 2dc734a..6eb4d65 100644 --- a/src/Sensirion_upt_ble_auto_detection.cpp +++ b/src/Sensirion_upt_ble_auto_detection.cpp @@ -1,4 +1,5 @@ #include "Sensirion_upt_ble_auto_detection.h" +#include "Arduino.h" #include "NimBleClient.h" const int COMPANY_ID_FILTER = 54534; @@ -20,19 +21,16 @@ __attribute__((unused)) void SensiScan::keepAlive() { _bleClient->keepAlive(); } -void SensiScan::onAdvertisementReceived(const std::string address, - std::string name, std::string data) { +void SensiScan::onAdvertisementReceived(uint64_t address, std::string name, + std::string data) { uint16_t companyId = (uint16_t)data[0] << 8 | (uint8_t)data[1]; if (companyId != COMPANY_ID_FILTER) { return; } - // Get MAC address as uint64_t - uint64_t deviceID = squashMACAddress(address); - // Build MetaData MetaData metaData; - metaData.deviceID = deviceID; + metaData.deviceID = address; metaData.deviceType.bleGadgetType = bleGadgetTypeFromCompleteLocalName(name.c_str()); metaData.platform = DevicePlatform::BLE; @@ -48,20 +46,6 @@ void SensiScan::onAdvertisementReceived(const std::string address, _sampleCache[getDeviceId(data)] = samples; } -/** - * @brief squash std::string address to a uint64_t - * @note MAC address is comprised of 6 bytes - */ -uint64_t SensiScan::squashMACAddress(const std::string& macAddressString) { - - uint64_t deviceID = static_cast(macAddressString[0]); - for (size_t i = 1; i < 6; i++) { - deviceID = (deviceID << 8) | macAddressString[i]; - } - - return deviceID; -} - /** * @brief last two digits of MAC address uniquely define a device */ diff --git a/src/Sensirion_upt_ble_auto_detection.h b/src/Sensirion_upt_ble_auto_detection.h index 054f125..c36cde5 100644 --- a/src/Sensirion_upt_ble_auto_detection.h +++ b/src/Sensirion_upt_ble_auto_detection.h @@ -20,9 +20,8 @@ class __attribute__((unused)) SensiScan: public BleClientCallback { private: BleClient* _bleClient; std::map> _sampleCache; - void onAdvertisementReceived(std::string address, std::string name, + void onAdvertisementReceived(uint64_t address, std::string name, std::string data) override; - static uint64_t squashMACAddress(const std::string& macAddressString); static uint16_t getDeviceId(const std::string& data); static uint8_t decodeData(const MetaData& metaData, const std::string& data, std::vector& samples);