Remove mutable ret pattern and test feature combinations

After adding test for feature combinations, I found a corner case where, when Transport is dropped and the process is terminated in a test, the `close` Python future is not awaited.
I don't know what other situations this issue may arise, so I have safe-guarded it via `block_on` instead of spawning a thread.

.github/workflows/python-build-test.yml

@@ -56,7 +56,7 @@ jobs:
         uses: actions/setup-python@v4
         with:
           python-version: ${{ matrix.python-version }}
-      - name: Install dependencies
+      - name: Install Python dependencies
         run: |
           python -m pip install --upgrade pip
           python -m pip install ".[build,test,development,documentation]"
@@ -65,15 +65,17 @@ jobs:
         with:
           components: clippy,rustfmt
           toolchain: ${{ matrix.rust-version }}
+      - name: Install Rust dependencies
+        run: cargo install cargo-all-features # allows building/testing combinations of features
       - name: Check License Headers
         run: cd rust && cargo run --features dev-tools --bin file-header check-all
       - name: Rust Build
-        run: cd rust && cargo build --all-targets && cargo build --all-features --all-targets
+        run: cd rust && cargo build --all-targets && cargo build-all-features --all-targets
       # Lints after build so what clippy needs is already built
       - name: Rust Lints
         run: cd rust && cargo fmt --check && cargo clippy --all-targets -- --deny warnings && cargo clippy --all-features --all-targets -- --deny warnings
       - name: Rust Tests
-        run: cd rust && cargo test
+        run: cd rust && cargo test-all-features
       # At some point, hook up publishing the binary. For now, just make sure it builds.
       # Once we're ready to publish binaries, this should be built with `--release`.
       - name: Build Bumble CLI

bumble/controller.py

@@ -1000,16 +1000,15 @@ def on_hci_le_set_scan_parameters_command(self, command):
         '''
         See Bluetooth spec Vol 4, Part E - 7.8.10 LE Set Scan Parameters Command
         '''
-        ret = HCI_SUCCESS
-        if not self.le_scan_enable:
-            self.le_scan_type = command.le_scan_type
-            self.le_scan_interval = command.le_scan_interval
-            self.le_scan_window = command.le_scan_window
-            self.le_scan_own_address_type = command.own_address_type
-            self.le_scanning_filter_policy = command.scanning_filter_policy
-        else:
-            ret = HCI_COMMAND_DISALLOWED_ERROR
-        return bytes([ret])
+        if self.le_scan_enable:
+            return bytes([HCI_COMMAND_DISALLOWED_ERROR])
+
+        self.le_scan_type = command.le_scan_type
+        self.le_scan_interval = command.le_scan_interval
+        self.le_scan_window = command.le_scan_window
+        self.le_scan_own_address_type = command.own_address_type
+        self.le_scanning_filter_policy = command.scanning_filter_policy
+        return bytes([HCI_SUCCESS])
 
     def on_hci_le_set_scan_enable_command(self, command):
         '''

rust/Cargo.toml

@@ -12,6 +12,13 @@ keywords = ["bluetooth", "ble"]
 categories = ["api-bindings", "network-programming"]
 rust-version = "1.70.0"
 
+# https://github.com/frewsxcv/cargo-all-features#options
+[package.metadata.cargo-all-features]
+# We are interested in testing subset combinations of this feature, so this is redundant
+denylist = ["unstable"]
+# To exercise combinations of any of these features, remove from `always_include_features`
+always_include_features = ["anyhow", "pyo3-asyncio-attributes", "dev-tools", "bumble-tools"]
+
 [dependencies]
 pyo3 = { version = "0.18.3", features = ["macros"] }
 pyo3-asyncio = { version = "0.18.0", features = ["tokio-runtime"] }
@@ -26,6 +33,7 @@ thiserror = "1.0.41"
 bytes = "1.5.0"
 pdl-derive = "0.2.0"
 pdl-runtime = "0.2.0"
+futures = "0.3.28"
 
 # Dev tools
 file-header = { version = "0.1.2", optional = true }
@@ -36,7 +44,6 @@ anyhow = { version = "1.0.71", optional = true }
 clap = { version = "4.3.3", features = ["derive"], optional = true }
 directories = { version = "5.0.1", optional = true }
 env_logger = { version = "0.10.0", optional = true }
-futures = { version = "0.3.28", optional = true }
 log = { version = "0.4.19", optional = true }
 owo-colors = { version = "3.5.0", optional = true }
 reqwest = { version = "0.11.20", features = ["blocking"], optional = true }
@@ -90,7 +97,7 @@ anyhow = ["pyo3/anyhow"]
 pyo3-asyncio-attributes = ["pyo3-asyncio/attributes"]
 dev-tools = ["dep:anyhow", "dep:clap", "dep:file-header", "dep:globset"]
 # separate feature for CLI so that dependencies don't spend time building these
-bumble-tools = ["dep:clap", "anyhow", "dep:anyhow", "dep:directories", "pyo3-asyncio-attributes", "dep:owo-colors", "dep:reqwest", "dep:rusb", "dep:log", "dep:env_logger", "dep:futures"]
+bumble-tools = ["dep:clap", "anyhow", "dep:anyhow", "dep:directories", "pyo3-asyncio-attributes", "dep:owo-colors", "dep:reqwest", "dep:rusb", "dep:log", "dep:env_logger"]
 
 # all the unstable features
 unstable = ["unstable_extended_adv"]

rust/src/wrapper/transport.rs

@@ -15,6 +15,7 @@
 //! HCI packet transport
 
 use crate::wrapper::controller::Controller;
+use futures::executor::block_on;
 use pyo3::{intern, types::PyModule, PyObject, PyResult, Python};
 
 /// A source/sink pair for HCI packet I/O.
@@ -58,9 +59,9 @@ impl Transport {
 
 impl Drop for Transport {
     fn drop(&mut self) {
-        // can't await in a Drop impl, but we can at least spawn a task to do it
-        let obj = self.0.clone();
-        tokio::spawn(async move { Self(obj).close().await });
+        // don't spawn a thread to handle closing, as it may get dropped at program termination,
+        // resulting in `RuntimeWarning: coroutine ... was never awaited` from Python
+        let _ = block_on(self.close());
     }
 }