Add Windows support for requestConnectionPriority and onConnectionParametersChange#244
Conversation
…ce.RequestPreferredConnectionParameters Map BleConnectionPriority values to WinRT equivalents: - balanced -> Balanced() - highPerformance -> ThroughputOptimized() - lowPower -> PowerOptimized() Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Windows has no event-driven API for connection parameter changes, so implement a background polling thread that checks GetConnectionParameters() every 2 seconds for all connected devices and fires the callback when values change. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request adds support for requesting connection priority and tracking connection parameter updates on Windows, aligning its capabilities with Android. The implementation introduces a background polling thread to monitor connection parameters every 2 seconds. However, several critical issues were identified in the C++ implementation: a potential use-after-free crash on exit due to UI-thread lambdas capturing this by value, destructor blocking caused by std::this_thread::sleep_for, and potential data races on connected_devices_. Additionally, an inefficient copy of BluetoothDeviceAgent by value was found when requesting connection priority, which should be replaced with a reference.
| void UniversalBlePlugin::StartConnectionParameterPolling() { | ||
| if (polling_active_.exchange(true)) return; | ||
| polling_thread_ = std::thread([this] { | ||
| while (polling_active_) { | ||
| std::this_thread::sleep_for(std::chrono::seconds(2)); | ||
| if (!polling_active_) break; | ||
| ui_thread_handler_.Post([this] { | ||
| for (const auto &[addr, agent] : connected_devices_) { | ||
| try { | ||
| if (agent->device.ConnectionStatus() != | ||
| BluetoothConnectionStatus::Connected) | ||
| continue; | ||
| auto params = agent->device.GetConnectionParameters(); | ||
| BleConnectionParametersUpdated update( | ||
| mac_address_to_str(addr), | ||
| static_cast<int64_t>(params.ConnectionInterval()), | ||
| static_cast<int64_t>(params.ConnectionLatency()), | ||
| static_cast<int64_t>(params.LinkTimeout()), | ||
| 0); | ||
| auto it = last_connection_params_.find(addr); | ||
| if (it == last_connection_params_.end() || | ||
| !it->second.has_value() || | ||
| it->second->interval() != update.interval() || | ||
| it->second->latency() != update.latency() || | ||
| it->second->supervision_timeout() != | ||
| update.supervision_timeout()) { | ||
| last_connection_params_[addr] = update; | ||
| callback_channel->OnConnectionParametersUpdated( | ||
| update, SuccessCallback, ErrorCallback); | ||
| } | ||
| } catch (...) { | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| void UniversalBlePlugin::StopConnectionParameterPolling() { | ||
| polling_active_ = false; | ||
| if (polling_thread_.joinable()) { | ||
| polling_thread_.join(); | ||
| } | ||
| } |
There was a problem hiding this comment.
There are two major issues with the polling implementation:
- Use-After-Free Crash on Exit: The polling thread posts lambdas to the UI thread capturing
thisby value. If the plugin is destroyed, any pending lambdas in the UI thread queue will execute afterthishas been deleted, leading to a crash. To prevent this, we can use a shared lifetime token (std::shared_ptr<bool>) that is invalidated in the destructor and checked inside the posted lambda before accessingthis. - Destructor Thread Blocking: Calling
std::this_thread::sleep_for(std::chrono::seconds(2))inside the polling loop can block the destructor (viajoin()) for up to 2 seconds when the plugin is destroyed. Using astd::condition_variablewithwait_forallows waking up the thread immediately upon destruction.
Additionally, please note that connected_devices_ is accessed on the UI thread here but modified on background threads (e.g., in ConnectAsync and CleanConnection), which is a data race. Consider protecting connected_devices_ with a mutex or ensuring all modifications are dispatched to the UI thread.
void UniversalBlePlugin::StartConnectionParameterPolling() {
if (polling_active_.exchange(true)) return;
polling_thread_ = std::thread([this] {
std::unique_lock<std::mutex> lock(polling_mutex_);
while (polling_active_) {
polling_cv_.wait_for(lock, std::chrono::seconds(2), [this] { return !polling_active_; });
if (!polling_active_) break;
auto alive = plugin_alive_;
ui_thread_handler_.Post([this, alive] {
if (!*alive) return;
for (const auto &[addr, agent] : connected_devices_) {
try {
if (agent->device.ConnectionStatus() !=
BluetoothConnectionStatus::Connected)
continue;
auto params = agent->device.GetConnectionParameters();
BleConnectionParametersUpdated update(
mac_address_to_str(addr),
static_cast<int64_t>(params.ConnectionInterval()),
static_cast<int64_t>(params.ConnectionLatency()),
static_cast<int64_t>(params.LinkTimeout()),
0);
auto it = last_connection_params_.find(addr);
if (it == last_connection_params_.end() ||
!it->second.has_value() ||
it->second->interval() != update.interval() ||
it->second->latency() != update.latency() ||
it->second->supervision_timeout() !=
update.supervision_timeout()) {
last_connection_params_[addr] = update;
callback_channel->OnConnectionParametersUpdated(
update, SuccessCallback, ErrorCallback);
}
} catch (...) {
}
}
});
}
});
}
void UniversalBlePlugin::StopConnectionParameterPolling() {
polling_active_ = false;
polling_cv_.notify_all();
if (polling_thread_.joinable()) {
polling_thread_.join();
}
}| UniversalBlePlugin::~UniversalBlePlugin() { | ||
| StopConnectionParameterPolling(); | ||
| ClearServices(); | ||
| peripheral_callback_channel_.reset(); | ||
| } |
There was a problem hiding this comment.
Update the destructor to invalidate the plugin_alive_ token before stopping the polling thread and cleaning up services. This ensures any pending lambdas posted to the UI thread will safely discard themselves instead of accessing deleted memory.
UniversalBlePlugin::~UniversalBlePlugin() {
if (plugin_alive_) {
*plugin_alive_ = false;
}
StopConnectionParameterPolling();
ClearServices();
peripheral_callback_channel_.reset();
}| std::thread polling_thread_; | ||
| std::atomic<bool> polling_active_{false}; | ||
| std::unordered_map<uint64_t, std::optional<BleConnectionParametersUpdated>> | ||
| last_connection_params_; |
There was a problem hiding this comment.
Add the std::condition_variable, std::mutex, and std::shared_ptr<bool> lifetime token to the class members to support the safe and responsive polling thread implementation.
std::thread polling_thread_;
std::atomic<bool> polling_active_{false};
std::condition_variable polling_cv_;
std::mutex polling_mutex_;
std::unordered_map<uint64_t, std::optional<BleConnectionParametersUpdated>>
last_connection_params_;
std::shared_ptr<bool> plugin_alive_ = std::make_shared<bool>(true);| #include <atomic> | ||
| #include <memory> | ||
| #include <optional> | ||
| #include <thread> | ||
| #include <vector> |
| "Unknown devicesId:" + device_id)); | ||
| return; | ||
| } | ||
| auto bluetooth_agent = *it->second; |
There was a problem hiding this comment.
The BluetoothDeviceAgent object is copied here by value (auto bluetooth_agent = *it->second;). Since BluetoothDeviceAgent contains a potentially large std::unordered_map (gatt_map) of services and characteristics, copying it on every connection priority request is inefficient. Consider using a reference instead.
| auto bluetooth_agent = *it->second; | |
| const auto &bluetooth_agent = *it->second; |
|
Thanks for the PR and for taking the time to implement this. The polling thread starts and keeps checking all connected devices, I'm not sure how expensive Windows BLE is already one of the more complex backends in the plugin, so I'm generally hesitant to add background polling unless there's a very strong reason. Even if we try to optimize this and only poll when needed, we'd likely need additional Windows-specific APIs on the Dart side to start and stop polling. That adds more complexity, and things like Flutter hot reload become tricky since Dart loses its state while native code may still be running. Overall, this feels like a fairly niche feature that would add quite a bit of Windows-specific complexity and maintenance for the future, That said, I'm open to suggestions |

Related: #220, #221
Implements two connection parameter APIs on Windows that were previously
Android-only.
requestConnectionPriority
Replaces the
otSupported stub with
BluetoothLEDevice::RequestPreferredConnectionParameters.
Maps BleConnectionPriority values to
BluetoothLEPreferredConnectionParameters
(Balanced / ThroughputOptimized / PowerOptimized).
onConnectionParametersChange
Windows has no event-driven API for connection parameter changes
(unlike Android's BluetoothGattCallback.onConnectionUpdated), so this
falls back to a background polling thread that calls
BluetoothLEDevice::GetConnectionParameters
every 2 seconds for connected devices. Callback fires only when values change
(deduplicated in both C++ and Dart).
This is the first polling-based callback in this library — every other
callback relies on native events. Feedback on the polling interval and
whether there is a better approach would be appreciated.