Conversation
This depends on NetBirdSDK at least at commit 1024d4569, which exports lazy connection variables (whether if it's enabled and the inactivity threshold) both for Android and iOS
📝 WalkthroughWalkthroughThe PR adds lazy connection configuration to the iOS NetBird app by introducing a new UI toggle in AdvancedView, implementing persistence through UserDefaults, updating the ViewModel to handle the new setting, and exposing it via environment variables. The force-relay alert is replaced with a generic config-change alert. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Fix all issues with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@NetbirdKit/EnvVarPackager.swift`:
- Line 33: Replace the magic number 5 used in
envList.put(NetBirdSDKGetEnvKeyNBInactivityThreshold(), value: String(5)) with a
named constant defined in GlobalConstants (e.g., NB_INACTIVITY_THRESHOLD) and
document the unit (seconds/minutes) in that constant's declaration; update the
call to use String(GlobalConstants.NB_INACTIVITY_THRESHOLD) so the value is
centralized and clearer for maintainability.
🧹 Nitpick comments (1)
🤖 Fix all nitpicks with AI agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@NetbirdKit/EnvVarPackager.swift`: - Line 33: Replace the magic number 5 used in envList.put(NetBirdSDKGetEnvKeyNBInactivityThreshold(), value: String(5)) with a named constant defined in GlobalConstants (e.g., NB_INACTIVITY_THRESHOLD) and document the unit (seconds/minutes) in that constant's declaration; update the call to use String(GlobalConstants.NB_INACTIVITY_THRESHOLD) so the value is centralized and clearer for maintainability.NetbirdKit/EnvVarPackager.swift (1)
33-33: Magic number for inactivity threshold.The value
5is hardcoded without a named constant or documentation of its unit (seconds? minutes?). Consider extracting this to a constant inGlobalConstantsfor clarity and maintainability, similar to how other settings are managed.🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@NetbirdKit/EnvVarPackager.swift` at line 33, Replace the magic number 5 used in envList.put(NetBirdSDKGetEnvKeyNBInactivityThreshold(), value: String(5)) with a named constant defined in GlobalConstants (e.g., NB_INACTIVITY_THRESHOLD) and document the unit (seconds/minutes) in that constant's declaration; update the call to use String(GlobalConstants.NB_INACTIVITY_THRESHOLD) so the value is centralized and clearer for maintainability.
There was a problem hiding this comment.
Pull request overview
This PR adds a toggle in the iOS app's advanced settings to enable/disable lazy connection functionality, which automatically manages peer connections based on activity. The feature defaults to enabled (true) and applies an inactivity threshold of 5 seconds.
Changes:
- Added a new user preference key for lazy connection settings
- Implemented UI toggle in AdvancedView with configuration change alert
- Refactored ForceRelayAlert to ConfigChangeAlert for reusability across multiple settings
- Added comprehensive test coverage for the new preference
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| NetbirdKit/GlobalConstants.swift | Added new constant keyEnableLazyConnection for storing the lazy connection preference |
| NetbirdKit/EnvVarPackager.swift | Added lazy connection and inactivity threshold environment variables to the SDK configuration |
| NetBirdTests/SharedUserDefaultsTests.swift | Added test to verify lazy connection defaults to true and added teardown cleanup |
| NetBirdTests/GlobalConstantsTests.swift | Added test to verify the lazy connection key constant value |
| NetBird/Source/App/Views/AdvancedView.swift | Added lazy connection toggle UI and renamed ForceRelayAlert to ConfigChangeAlert for reusability |
| NetBird/Source/App/ViewModels/MainViewModel.swift | Added state management for lazy connection setting and updated alert property name |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| envList.put(NetBirdSDKGetEnvKeyNBForceRelay(), value: String(forceRelayConnection)) | ||
|
|
||
| envList.put(NetBirdSDKGetEnvKeyNBLazyConn(), value: String(isLazyConnectionEnabled)) | ||
| envList.put(NetBirdSDKGetEnvKeyNBInactivityThreshold(), value: String(5)) |
There was a problem hiding this comment.
The hardcoded inactivity threshold value of 5 should be extracted to a named constant in GlobalConstants.swift for better maintainability. This would follow the established pattern seen with other configuration values in the codebase and make it easier to adjust this value in the future without searching through the code.
| let userDefaults = UserDefaults(suiteName: GlobalConstants.userPreferencesSuiteName) | ||
| userDefaults?.register(defaults: [GlobalConstants.keyEnableLazyConnection: true]) | ||
| return userDefaults?.bool(forKey: GlobalConstants.keyEnableLazyConnection) ?? true |
There was a problem hiding this comment.
The getLazyConnectionEnabled function should follow the same pattern as getForcedRelayConnectionEnabled by including platform-specific defaults using #if os(iOS) guards. According to the PR description, lazy connection is for the iOS peer, suggesting it may need different defaults on tvOS similar to how forceRelayConnection has platform-specific handling.
| let userDefaults = UserDefaults(suiteName: GlobalConstants.userPreferencesSuiteName) | |
| userDefaults?.register(defaults: [GlobalConstants.keyEnableLazyConnection: true]) | |
| return userDefaults?.bool(forKey: GlobalConstants.keyEnableLazyConnection) ?? true | |
| let userDefaults = UserDefaults(suiteName: GlobalConstants.userPreferencesSuiteName) | |
| #if os(iOS) | |
| userDefaults?.register(defaults: [GlobalConstants.keyEnableLazyConnection: true]) | |
| return userDefaults?.bool(forKey: GlobalConstants.keyEnableLazyConnection) ?? true | |
| #else | |
| // lazy connection is for the iOS peer; disable by default on Apple TV | |
| userDefaults?.register(defaults: [GlobalConstants.keyEnableLazyConnection: false]) | |
| return userDefaults?.bool(forKey: GlobalConstants.keyEnableLazyConnection) ?? false | |
| #endif |
This PR adds a toggle in advanced settings for enabling lazy connection on the iOS peer.
Summary by CodeRabbit
New Features
Improvements