Skip to content
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.

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

Support ble extended advertising on Windows #114

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

Conversation

rohitsangwan01
Copy link
Contributor

@rohitsangwan01 rohitsangwan01 commented Dec 9, 2024

PR Type

enhancement


Description

  • Added support for Bluetooth Low Energy (BLE) extended advertisements on Windows by modifying the BluetoothLEAdvertisementWatcher to allow extended advertisements.
  • This enhancement enables the use of extended advertising features in BLE applications on Windows.

Changes walkthrough 📝

Relevant files
Enhancement
universal_ble_plugin.cpp
Enable BLE extended advertisements on Windows                       

windows/src/universal_ble_plugin.cpp

  • Added support for allowing extended advertisements in Bluetooth LE.
  • Modified the BluetoothLEAdvertisementWatcher to enable extended
    advertisements.
  • +1/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling
    The return value from AllowExtendedAdvertisements() is explicitly discarded using (void). Consider handling potential errors that may occur when enabling extended advertisements.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Handle potential failures when enabling a feature instead of silently ignoring them

    Check the return value of AllowExtendedAdvertisements() to handle potential failures
    in enabling extended advertisements. Ignoring the return value with a void cast is
    not recommended.

    windows/src/universal_ble_plugin.cpp [110]

    -(void)bluetoothLEWatcher.AllowExtendedAdvertisements();
    +if (!bluetoothLEWatcher.AllowExtendedAdvertisements()) {
    +    // Handle the failure case appropriately
    +    throw std::runtime_error("Failed to enable extended advertisements");
    +}
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies a potential reliability issue where failures in enabling extended advertisements are being silently ignored. Adding error handling is crucial for robust Bluetooth functionality.

    8

    💡 Need additional feedback ? start a PR chat

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant