Skip to content

Fix crash when unpairing device while scanning #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
fotiDim wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fix crash when unpairing device while scanning #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 0.21.1
* Fix device name resolution on Windows
* Add `exclusionFilters` to filter out devices from scan results
* Fix crash when unpairing device while scanning

## 0.21.0
* BREAKING CHANGE: `connectionTimeout` argument from `connect`, `isPaired` and `pair` API renamed to `timeout`
Expand Down
128 changes: 75 additions & 53 deletions windows/src/universal_ble_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,39 +830,46 @@ namespace universal_ble

void UniversalBlePlugin::OnDeviceInfoReceived(const DeviceInformation& device_info)
{
const auto properties = device_info.Properties();
try
{
const auto properties = device_info.Properties();

// Avoid devices if not connectable or if deviceAddressKey is not present
if (!(properties.HasKey(is_connectable_key) && (properties.Lookup(is_connectable_key).as<IPropertyValue>()).GetBoolean()) || !properties.HasKey(device_address_key))
return;
// Avoid devices if not connectable or if deviceAddressKey is not present
if (!(properties.HasKey(is_connectable_key) && (properties.Lookup(is_connectable_key).as<IPropertyValue>()).GetBoolean()) || !properties.HasKey(device_address_key))
return;

const auto bluetooth_address_property_value = properties.Lookup(device_address_key).as<IPropertyValue>();
const std::string device_address = to_string(bluetooth_address_property_value.GetString());
const auto bluetooth_address_property_value = properties.Lookup(device_address_key).as<IPropertyValue>();
const std::string device_address = to_string(bluetooth_address_property_value.GetString());

// Update device info if already discovered in advertisementWatcher
if (scan_results_.get(device_address).has_value())
{
bool is_paired = device_info.Pairing().IsPaired();
if (properties.HasKey(is_paired_key))
// Update device info if already discovered in advertisementWatcher
if (scan_results_.get(device_address).has_value())
{
const auto is_paired_property_value = properties.Lookup(is_paired_key).as<IPropertyValue>();
is_paired = is_paired_property_value.GetBoolean();
}
bool is_paired = device_info.Pairing().IsPaired();
if (properties.HasKey(is_paired_key))
{
const auto is_paired_property_value = properties.Lookup(is_paired_key).as<IPropertyValue>();
is_paired = is_paired_property_value.GetBoolean();
}

UniversalBleScanResult universal_scan_result(device_address);
universal_scan_result.set_is_paired(is_paired);

UniversalBleScanResult universal_scan_result(device_address);
universal_scan_result.set_is_paired(is_paired);
if (!device_info.Name().empty())
universal_scan_result.set_name(to_string(device_info.Name()));

if (!device_info.Name().empty())
universal_scan_result.set_name(to_string(device_info.Name()));
if (properties.HasKey(signal_strength_key))
{
const auto rssi_property_value = properties.Lookup(signal_strength_key).as<IPropertyValue>();
const int16_t rssi = rssi_property_value.GetInt16();
universal_scan_result.set_rssi(rssi);
}

if (properties.HasKey(signal_strength_key))
{
const auto rssi_property_value = properties.Lookup(signal_strength_key).as<IPropertyValue>();
const int16_t rssi = rssi_property_value.GetInt16();
universal_scan_result.set_rssi(rssi);
PushUniversalScanResult(universal_scan_result, true);
}

PushUniversalScanResult(universal_scan_result, true);
}
catch (...)
{
std::cout << "DeviceWatcherErrorInParsing" << std::endl;
}
Comment on lines +870 to 873
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While adding a try-catch block is a good way to prevent the crash, catching all exceptions with ... can hide other potential issues. It would be better to catch the specific exception type you expect, which is likely winrt::hresult_error when dealing with stale WinRT objects. This makes the code safer and easier to debug. Also, the log message could be more descriptive.

    catch (const winrt::hresult_error& e)
    {
      std::cout << "Failed to process device info, likely due to unpairing: " << to_string(e.message()) << std::endl;
    }

}

Expand All @@ -873,7 +880,9 @@ namespace universal_ble
{
auto device_id = mac_address_to_str(args.BluetoothAddress());
auto universal_scan_result = UniversalBleScanResult(device_id);
std::string name = to_string(args.Advertisement().LocalName());
std::string name = "";
if (args.Advertisement() != nullptr)
name = to_string(args.Advertisement().LocalName());
Comment on lines +883 to +885
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There are multiple if (args.Advertisement() != nullptr) checks in this function. To improve readability and avoid repeated calls to args.Advertisement(), you could cache the result in a local variable at the beginning of the function and use that variable for all subsequent checks and operations on the advertisement object. The ternary operator can also make this part more concise.

      auto advertisement = args.Advertisement();
      std::string name = advertisement ? to_string(advertisement.LocalName()) : "";
      // TODO: Use `advertisement` in subsequent null checks in this function.


auto manufacturer_data_encodable_list = flutter::EncodableList();
if (args.Advertisement() != nullptr)
Expand All @@ -885,19 +894,22 @@ namespace universal_ble
}
}

auto data_section = args.Advertisement().DataSections();
for (auto &&data : data_section)
if (args.Advertisement() != nullptr)
{
auto data_bytes = to_bytevc(data.Data());
// Use CompleteName from dataType if localName is empty
if (name.empty() && data.DataType() == static_cast<uint8_t>(AdvertisementSectionType::CompleteLocalName))
auto data_section = args.Advertisement().DataSections();
for (auto &&data : data_section)
{
name = std::string(data_bytes.begin(), data_bytes.end());
}
// Use ShortenedLocalName from dataType if localName is empty
else if (name.empty() && data.DataType() == static_cast<uint8_t>(AdvertisementSectionType::ShortenedLocalName))
{
name = std::string(data_bytes.begin(), data_bytes.end());
auto data_bytes = to_bytevc(data.Data());
// Use CompleteName from dataType if localName is empty
if (name.empty() && data.DataType() == static_cast<uint8_t>(AdvertisementSectionType::CompleteLocalName))
{
name = std::string(data_bytes.begin(), data_bytes.end());
}
// Use ShortenedLocalName from dataType if localName is empty
else if (name.empty() && data.DataType() == static_cast<uint8_t>(AdvertisementSectionType::ShortenedLocalName))
{
name = std::string(data_bytes.begin(), data_bytes.end());
}
}
}

Expand All @@ -914,27 +926,37 @@ namespace universal_ble
universal_scan_result.set_rssi(args.RawSignalStrengthInDBm());

// Add services
auto services = flutter::EncodableList();
for (auto &&uuid : args.Advertisement().ServiceUuids())
services.push_back(guid_to_uuid(uuid));
universal_scan_result.set_services(services);
if (args.Advertisement() != nullptr)
{
auto services = flutter::EncodableList();
for (auto &&uuid : args.Advertisement().ServiceUuids())
services.push_back(guid_to_uuid(uuid));
universal_scan_result.set_services(services);
}

// check if this device already discovered in deviceWatcher
auto it = device_watcher_devices_.get(device_id);
if (it.has_value())
{
auto &device_info = it.value();
auto properties = device_info.Properties();

// Update Paired Status
bool is_paired = device_info.Pairing().IsPaired();
if (properties.HasKey(is_paired_key))
is_paired = (properties.Lookup(is_paired_key).as<IPropertyValue>()).GetBoolean();
universal_scan_result.set_is_paired(is_paired);

// Update Name
if (name.empty() && !device_info.Name().empty())
universal_scan_result.set_name(to_string(device_info.Name()));
try
{
auto &device_info = it.value();
auto properties = device_info.Properties();

// Update Paired Status
bool is_paired = device_info.Pairing().IsPaired();
if (properties.HasKey(is_paired_key))
is_paired = (properties.Lookup(is_paired_key).as<IPropertyValue>()).GetBoolean();
universal_scan_result.set_is_paired(is_paired);

// Update Name
if (name.empty() && !device_info.Name().empty())
universal_scan_result.set_name(to_string(device_info.Name()));
}
catch (...)
{
// Ignore errors from accessing stale DeviceInformation
}
Comment on lines +956 to +959
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using catch (...) is a bit broad and can hide unrelated errors. It's better to catch the specific exception you expect to ignore, which is likely winrt::hresult_error for stale WinRT objects. This makes the code more robust.

        catch (const winrt::hresult_error&)
        {
          // Ignore errors from accessing stale DeviceInformation
        }

}

// Filter Device
Expand Down