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

Enhance getSystemDevice API on macOS with Broader Default Services Fi… #121

Conversation

rohitsangwan01
Copy link
Contributor

@rohitsangwan01 rohitsangwan01 commented Jan 2, 2025

User description

…lter


PR Type

Enhancement


Description

  • Enhanced getSystemDevices API to include broader default services filter.

  • Added support for secure restorable state in macOS example app.

  • Minor formatting adjustments in UniversalBlePlugin.swift.


Changes walkthrough 📝

Relevant files
Enhancement
UniversalBlePlugin.swift
Broaden default services filter in `getSystemDevices` API

darwin/Classes/UniversalBlePlugin.swift

  • Broadened default services filter in getSystemDevices API.
  • Adjusted logic to handle empty service filters.
  • Minor formatting changes for better readability.
  • +8/-5     
    AppDelegate.swift
    Add secure restorable state support in macOS app                 

    example/macos/Runner/AppDelegate.swift

  • Added applicationSupportsSecureRestorableState method.
  • Enabled secure restorable state for macOS example app.
  • +4/-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: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Service Filter Validation

    The hardcoded list of default BLE service UUIDs should be validated to ensure they are all valid and relevant for the target use cases. Consider documenting why these specific services were chosen.

    servicesFilter = ["1800", "1801", "180A", "180D", "1810", "181B", "1808", "181D", "1816", "1814", "181A", "1802", "1803", "1804", "1815", "1805", "1807", "1806", "1848", "185E", "180F", "1812", "180E", "1813"]

    Copy link

    codiumai-pr-agent-free bot commented Jan 2, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Move hardcoded values to static constants to improve code maintainability and performance

    Consider using a static constant for the default service UUIDs to improve
    maintainability and avoid recreating the array on each function call.

    darwin/Classes/UniversalBlePlugin.swift [295-298]

    +private static let defaultServiceUUIDs = ["1800", "1801", "180A", "180D", "1810", "181B", "1808", "181D", "1816", "1814", "181A", "1802", "1803", "1804", "1815", "1805", "1807", "1806", "1848", "185E", "180F", "1812", "180E", "1813"]
    +
     if servicesFilter.isEmpty {
    -  // Add few general services as filter
    -  servicesFilter = ["1800", "1801", "180A", "180D", "1810", "181B", "1808", "181D", "1816", "1814", "181A", "1802", "1803", "1804", "1815", "1805", "1807", "1806", "1848", "185E", "180F", "1812", "180E", "1813"]
    +  // Use default services as filter
    +  servicesFilter = Self.defaultServiceUUIDs
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Moving the hardcoded service UUIDs to a static constant is a good practice that improves maintainability and slightly optimizes performance by avoiding array recreation on each function call.

    7

    @rohitsangwan01 rohitsangwan01 merged commit f341128 into main Jan 2, 2025
    1 check passed
    @rohitsangwan01 rohitsangwan01 deleted the Enhance-getSystemDevice-API-on-macOS-with-Broader-Default-Services-Filter branch January 2, 2025 10:28
    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.

    2 participants