Merge pull request #35 from OpenBluetoothToolbox/feature/simplebluez_0.1

Complete replacement of underlying Linux implementation.

.gitignore

@@ -1,4 +1,2 @@
-.build
-build
 .vscode
-*.binlog
+*.binlog

CHANGELOG.md

@@ -4,13 +4,15 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.0.3] - XXXX-XX-XX
+## [0.1.0] - 2021-12-28
 
 ### Changed
 - Referenced specific version of SimpleBluez to avoid breaking changes as those libraries evolve.
+- (Linux) When `scan_stop` is called, it is now guaranteed that no more scan results will be received.
+- Updated Linux implementation to use SimpleBluez v0.1.1.
 
 ### Fixed
-- Scan will never stop sleeping on Linux.
+- (Linux) Scan will never stop sleeping.
 
 ## [0.0.2] - 2021-10-09
 

CMakeLists.txt

@@ -15,6 +15,9 @@ if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR)
     set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
     set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
     set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
+
+    # Nice hack to automatically ignore the build directory
+    file(WRITE ${CMAKE_BINARY_DIR}/.gitignore "*")
 else()
     set(STANDALONE false)
 endif()
@@ -24,7 +27,7 @@ if(NOT SIMPLEBLUEZ_GIT_REPOSITORY)
     set(SIMPLEBLUEZ_GIT_REPOSITORY "https://github.com/OpenBluetoothToolbox/SimpleBluez.git")
 endif()
 if(NOT SIMPLEBLUEZ_GIT_TAG)
-    set(SIMPLEBLUEZ_GIT_TAG "262e1bf91a463548a5e47cadedadf21d6c0beb66") # References the latest stable commit that works.
+    set(SIMPLEBLUEZ_GIT_TAG "v0.1.1")
 endif()
 
 # Perform some overall CMake configurations
@@ -56,6 +59,19 @@ set(
 if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
     message("-- [INFO] Linux Host Detected")
 
+    # Append additional flags for address and thread sanitization
+    if(SIMPLEBLE_SANITIZE MATCHES "Address")
+        set(SIMPLEBLUEZ_SANITIZE ${SIMPLEBLE_SANITIZE})
+        set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} -fsanitize=address -fsanitize-recover=address -fno-omit-frame-pointer -fno-common -g)
+        set(EXTRA_LINK_OPTIONS ${EXTRA_LINK_OPTIONS} -fsanitize=address)
+    endif()
+
+    if(SIMPLEBLE_SANITIZE MATCHES "Thread")
+        set(SIMPLEBLUEZ_SANITIZE ${SIMPLEBLE_SANITIZE})
+        set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} -fsanitize=thread -fno-omit-frame-pointer -fno-common -g)
+        set(EXTRA_LINK_OPTIONS ${EXTRA_LINK_OPTIONS} -fsanitize=thread)
+    endif()
+
     if(NOT SIMPLEBLUEZ_LOCAL_PATH)
         FetchContent_Declare(
             simplebluez
@@ -171,6 +187,16 @@ target_link_libraries(simpleble-c PUBLIC simpleble-static)
 
 # Append any compilation flags to each specific target
 if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
+    target_compile_options(simpleble-static PRIVATE ${EXTRA_COMPILE_OPTIONS})
+    target_compile_options(simpleble PRIVATE ${EXTRA_COMPILE_OPTIONS})
+    target_compile_options(simpleble-c-static PRIVATE ${EXTRA_COMPILE_OPTIONS})
+    target_compile_options(simpleble-c PRIVATE ${EXTRA_COMPILE_OPTIONS})
+
+    target_link_options(simpleble-static PRIVATE ${EXTRA_LINK_OPTIONS})
+    target_link_options(simpleble PRIVATE ${EXTRA_LINK_OPTIONS})
+    target_link_options(simpleble-c-static PRIVATE ${EXTRA_LINK_OPTIONS})
+    target_link_options(simpleble-c PRIVATE ${EXTRA_LINK_OPTIONS})
+
 elseif (CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows")
 
     set(WINVERSION_CODE 0x0A00) # Selected Windows 10 based on https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt

README.md

@@ -132,6 +132,18 @@ specifying the additional command line arguments to the `cmake` command:
     Call CMake with `-DSIMPLEDBUS_LOCAL_PATH=<path>` to override the
     default location of the SimpleDBus repository.
 
+### Tests
+Testing is currently not available for the library as a whole, yet there are some
+build settings that can be used to find issues with the library.
+
+#### (Linux) Address Sanitizer
+In order to run tests with Address Sanitizer, CMake needs to be called with
+the following option: `-DSIMPLEBLE_SANITIZE=Address`.
+
+#### (Linux) Thread Sanitizer
+In order to run tests with Thread Sanitizer, CMake needs to be called with
+the following option: `-DSIMPLEBLE_SANITIZE=Thread`.
+
 ## Collaborating
 
 ### Coding & Naming Conventions
@@ -142,44 +154,6 @@ specifying the additional command line arguments to the `cmake` command:
     - Class protected and private property names must end with an underscore (`_`).
     - Class protected and private method names must start with an underscore (`_`).
 
-### API per OS
-
-The following tables describe the state of each available API per operating system.
-Any field that is not specified as supported will throw a compilation error if used
-or just be ignored.
-
-| API                                                  | Linux | Windows | MacOS |
-| ---------------------------------------------------- | ----- | ------- | ----- |
-| `SimpleBLE::Adapter::identifier`                     | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::address`                        | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::scan_start`                     | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::scan_stop`                      | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::scan_for`                       | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::scan_is_active`                 | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::scan_get_results`               | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::set_callback_on_scan_start`     | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::set_callback_on_scan_stop`      | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::set_callback_on_scan_updated`   | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::set_callback_on_scan_found`     | Yes   | Yes     | Yes   |
-| `SimpleBLE::Adapter::get_adapters`                   | Yes   | Yes     | Yes   |
-| ---------------------------------------------------- | ----- | ------- | ----- |
-| `SimpleBLE::Peripheral::identifier`                  | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::address`                     | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::connect`                     | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::disconnect`                  | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::is_connected`                | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::is_connectable`              | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::services`                    | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::manufacturer_data`           | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::read`                        | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::write_request`               | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::write_command`               | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::notify`                      | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::indicate`                    | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::unsubscribe`                 | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::set_callback_on_connected`   | Yes   | Yes     | Yes   |
-| `SimpleBLE::Peripheral::set_callback_on_disconnected`| Yes   | Yes     | Yes   |
-
 ## Known Issues / To-Do's
 - [Linux] Fork safety is not guaranteed.
 - [MacOS] Only the main system adapter can be detected.
@@ -190,15 +164,10 @@ or just be ignored.
 - [Windows] Unclear if multiple adapters can be detected.
 - [All] Run callbacks in separate threads to prevent blocking internal threads.
 - [All] Add a signal handler to ensure all objects are disconnected when the program exits.
-- [All] Add safe version of the library that won't trigger any exceptions.
 - [All] Replace C-style casts with C++ style casts.
 - [All] Add Python bindings for the library.
 - [All] Add Javascript bindings for the library.
 - [All] Add Kotlin bindings for the library.
 
-## Ideas
-- Explore if callbacks can be wrapped in shared pointers to prevent them from being prematurely deleted.
-- Explore if callbacks can be handled in a separate thread to prevent blocking the main thread.
-
 ## License
 All components within this project that have not been bundled from external creators, are licensed under the terms of the [MIT Licence](LICENCE.md).

examples/CMakeLists.txt

@@ -6,6 +6,9 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 
+# Nice hack to automatically ignore the build directory
+file(WRITE ${CMAKE_BINARY_DIR}/.gitignore "*")
+
 set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_EXTENSIONS OFF)
 

include/simpleble/Adapter.h

@@ -16,7 +16,7 @@ class AdapterBase;
 class Adapter {
   public:
     Adapter();
-    ~Adapter();
+    virtual ~Adapter();
 
     std::string identifier();
     BluetoothAddress address();

include/simpleble/AdapterSafe.h

@@ -10,7 +10,7 @@ namespace Safe {
 class Adapter : public SimpleBLE::Adapter {
   public:
     Adapter(SimpleBLE::Adapter& adapter);
-    ~Adapter();
+    virtual ~Adapter();
 
     std::optional<std::string> identifier() noexcept;
     std::optional<BluetoothAddress> address() noexcept;

include/simpleble/Peripheral.h

@@ -16,7 +16,7 @@ class PeripheralBase;
 class Peripheral {
   public:
     Peripheral();
-    ~Peripheral();
+    virtual ~Peripheral();
 
     std::string identifier();
     BluetoothAddress address();

include/simpleble/PeripheralSafe.h

@@ -11,7 +11,7 @@ namespace Safe {
 class Peripheral : public SimpleBLE::Peripheral {
   public:
     Peripheral(SimpleBLE::Peripheral& peripheral);
-    ~Peripheral();
+    virtual ~Peripheral();
 
     std::optional<std::string> identifier() noexcept;
     std::optional<BluetoothAddress> address() noexcept;

scripts/linux/build_examples.sh

@@ -22,5 +22,5 @@ fi
 # Compile!
 mkdir -p $BUILD_PATH
 cd $BUILD_PATH
-cmake -H$SOURCE_ROOT
+cmake -H$SOURCE_ROOT -DCMAKE_BUILD_TYPE=Debug
 make -j

src/Adapter.cpp

@@ -3,8 +3,6 @@
 #include "AdapterBase.h"
 #include "AdapterBuilder.h"
 
-#include <iostream>
-
 using namespace SimpleBLE;
 
 std::vector<Adapter> Adapter::get_adapters() {

src/AdapterBuilder.h

@@ -16,6 +16,7 @@ namespace SimpleBLE {
 class AdapterBuilder : public Adapter {
   public:
     AdapterBuilder(std::shared_ptr<AdapterBase> internal);
+    virtual ~AdapterBuilder() = default;
 };
 
-}  // namespace SimpleBLE
+}  // namespace SimpleBLE

src/Peripheral.cpp

@@ -1,7 +1,6 @@
 #include <simpleble/Peripheral.h>
 
 #include <simpleble/Exceptions.h>
-#include <iostream>
 #include "PeripheralBase.h"
 
 using namespace SimpleBLE;

src/PeripheralBuilder.h

@@ -16,6 +16,7 @@ namespace SimpleBLE {
 class PeripheralBuilder : public Peripheral {
   public:
     PeripheralBuilder(std::shared_ptr<PeripheralBase> internal);
+    virtual ~PeripheralBuilder() = default;
 };
 
-}  // namespace SimpleBLE
+}  // namespace SimpleBLE

src/linux/AdapterBase.cpp

@@ -3,87 +3,66 @@
 #include "PeripheralBase.h"
 #include "PeripheralBuilder.h"
 
-#include <iostream>
-
 using namespace SimpleBLE;
 
 std::vector<std::shared_ptr<AdapterBase>> AdapterBase::get_adapters() {
     std::vector<std::shared_ptr<AdapterBase>> adapter_list;
-    auto internal_adapters = Bluez::get()->bluez_service.get_all_adapters();
+    auto internal_adapters = Bluez::get()->bluez.get_adapters();
     for (auto& adapter : internal_adapters) {
         adapter_list.push_back(std::make_shared<AdapterBase>(adapter));
     }
     return adapter_list;
 }
 
-AdapterBase::AdapterBase(std::shared_ptr<BluezAdapter> adapter) { adapter_ = adapter; }
+AdapterBase::AdapterBase(std::shared_ptr<SimpleBluez::Adapter> adapter) : adapter_(adapter) {}
 
-AdapterBase::~AdapterBase() {}
+AdapterBase::~AdapterBase() { adapter_->clear_on_device_updated(); }
 
-std::string AdapterBase::identifier() {
-    auto adapter = adapter_.lock();
-    if (adapter) {
-        return adapter->get_identifier();
-    } else {
-        throw Exception::InvalidReference();
-    }
-}
+std::string AdapterBase::identifier() { return adapter_->identifier(); }
 
-BluetoothAddress AdapterBase::address() {
-    auto adapter = adapter_.lock();
-    if (adapter) {
-        return adapter->Address();
-    } else {
-        throw Exception::InvalidReference();
-    }
-}
+BluetoothAddress AdapterBase::address() { return adapter_->address(); }
 
 void AdapterBase::scan_start() {
-    auto adapter = adapter_.lock();
-    if (adapter) {
-        adapter->discovery_filter_transport_set("le");
-        this->seen_devices_.clear();
-        adapter->OnDeviceUpdated = [&](std::shared_ptr<BluezDevice> device) {
-            PeripheralBuilder peripheral_builder(std::make_shared<PeripheralBase>(device));
-
-            if (this->seen_devices_.count(peripheral_builder.address()) == 0) {
-                this->seen_devices_.insert(std::make_pair<>(peripheral_builder.address(), peripheral_builder));
-                if (this->callback_on_scan_found_) {
-                    this->callback_on_scan_found_(peripheral_builder);
-                }
-            } else {
-                if (this->callback_on_scan_updated_) {
-                    this->callback_on_scan_updated_(peripheral_builder);
-                }
-            }
-        };
+    adapter_->discovery_filter(SimpleBluez::Adapter::DiscoveryFilter::LE);
 
-        // Start scanning and notify the user.
-        adapter->StartDiscovery();
-        if (callback_on_scan_start_) {
-            callback_on_scan_start_();
+    seen_devices_.clear();
+    adapter_->set_on_device_updated([this](std::shared_ptr<SimpleBluez::Device> device) {
+        if (!this->is_scanning_) {
+            return;
         }
-    } else {
-        throw Exception::InvalidReference();
+
+        PeripheralBuilder peripheral_builder(std::make_shared<PeripheralBase>(device));
+
+        if (this->seen_devices_.count(peripheral_builder.address()) == 0) {
+            this->seen_devices_.insert(std::make_pair<>(peripheral_builder.address(), peripheral_builder));
+            if (this->callback_on_scan_found_) {
+                this->callback_on_scan_found_(peripheral_builder);
+            }
+        } else {
+            if (this->callback_on_scan_updated_) {
+                this->callback_on_scan_updated_(peripheral_builder);
+            }
+        }
+    });
+
+    // Start scanning and notify the user.
+    adapter_->discovery_start();
+    if (callback_on_scan_start_) {
+        callback_on_scan_start_();
     }
+    is_scanning_ = true;
 }
 
 void AdapterBase::scan_stop() {
-    auto adapter = adapter_.lock();
-    if (adapter) {
-        adapter->StopDiscovery();
-        // Due to the fact that Bluez takes some time to process the command
-        // and for SimpleDBus to flush the queue, wait until the driver
-        // has acknowledged that it is no longer discovering.
-        while (adapter->Property_Discovering()) {
-            std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        }
-        if (callback_on_scan_stop_) {
-            callback_on_scan_stop_();
-        }
-    } else {
-        throw Exception::InvalidReference();
+    adapter_->discovery_stop();
+    is_scanning_ = false;
+    if (callback_on_scan_stop_) {
+        callback_on_scan_stop_();
     }
+
+    // Important: Bluez might continue scanning if another process is also requesting
+    // scanning from the adapter. The use of the is_scanning_ flag is to prevent
+    // any scan updates to reach the user when not expected.
 }
 
 void AdapterBase::scan_for(int timeout_ms) {
@@ -92,14 +71,7 @@ void AdapterBase::scan_for(int timeout_ms) {
     scan_stop();
 }
 
-bool AdapterBase::scan_is_active() {
-    auto adapter = adapter_.lock();
-    if (adapter) {
-        return adapter->Property_Discovering();
-    } else {
-        throw Exception::InvalidReference();
-    }
-}
+bool AdapterBase::scan_is_active() { return is_scanning_ && adapter_->discovering(); }
 
 std::vector<Peripheral> AdapterBase::scan_get_results() {
     std::vector<Peripheral> peripherals;