Skip to content

Refactor property value lookups in Windows to use a safe lookup function#241

Merged
navidecklabs merged 3 commits into
mainfrom
lookup_i_property_value
May 28, 2026
Merged

Refactor property value lookups in Windows to use a safe lookup function#241
navidecklabs merged 3 commits into
mainfrom
lookup_i_property_value

Conversation

@fotiDim

@fotiDim fotiDim commented May 25, 2026

Copy link
Copy Markdown
Contributor

This pull request refactors the way BLE device property values are accessed in the Windows plugin, improving error handling and code maintainability. The main change is the introduction of a helper function, lookup_i_property_value, which standardizes property access and logging for missing or invalid properties. This function is then used throughout the codebase to replace direct property lookups, resulting in safer and more readable code.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a helper function lookup_i_property_value to refactor and safely lookup properties from a WinRT property map, avoiding potential crashes from invalid casts. A critical issue was identified where using a template parameter for PropertyMap makes try_as a dependent template member function call, which requires the template keyword to compile under standard-compliant compilers. It is recommended to make lookup_i_property_value a non-template function with fully qualified WinRT types to prevent compilation failures and template bloat.

Comment thread windows/src/universal_ble_plugin.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors Windows DeviceInformation property retrieval to centralize IPropertyValue lookups behind a helper, aiming to avoid unsafe casts and improve resilience when property types don’t match expectations.

Changes:

  • Added a reusable lookup_i_property_value(...) helper for property lookups.
  • Updated device watcher “Added” handling to use the helper for DeviceAddress.
  • Updated OnDeviceInfoReceived and BluetoothLeWatcherReceived to use the helper for IsConnectable, DeviceAddress, IsPaired, and SignalStrength.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread windows/src/universal_ble_plugin.cpp Outdated
@fotiDim fotiDim force-pushed the lookup_i_property_value branch from 7a2916c to 63caf06 Compare May 26, 2026 09:40
…andling and ensure safe retrieval of IPropertyValue. This change replaces the previous template-based approach with a more robust implementation that catches exceptions and logs errors appropriately.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@navidecklabs navidecklabs merged commit 889a125 into main May 28, 2026
3 checks passed
@navidecklabs navidecklabs deleted the lookup_i_property_value branch May 28, 2026 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants