Add Quick Settings Tile service for VPN control#136
Open
Dan-Kingsley wants to merge 4 commits intonetbirdio:mainfrom
Open
Add Quick Settings Tile service for VPN control#136Dan-Kingsley wants to merge 4 commits intonetbirdio:mainfrom
Dan-Kingsley wants to merge 4 commits intonetbirdio:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a Quick Settings TileService, manifest entry, and string resource; TileService binds to the app VPNService, toggles or starts/stops the VPN based on binder state and permissions, and updates tile UI. Also exposes VPN running state via a new binder method. Changes
Sequence DiagramsequenceDiagram
actor User
participant QS as Quick Settings
participant Tile as NetbirdTileService
participant VPN as VPNService (binder)
participant Engine as EngineRunner
User->>QS: Tap Netbird tile
QS->>Tile: onClick()
alt Tile not bound
Tile->>VPN: bindToVpnService()
VPN-->>Tile: onServiceConnected (binder)
end
Tile->>VPN: isRunning()
VPN->>Engine: engineRunner.isRunning()
Engine-->>VPN: running? (true/false)
alt running == true
Tile->>VPN: request stop via binder
VPN->>Engine: stop()
else running == false
Tile->>Tile: check BIND_QUICK_SETTINGS_TILE / VPN permission
alt permission granted
Tile->>VPN: startAndRunVpnService()
VPN->>Engine: start()
else permission missing
Tile->>Tile: launch MainActivity for permission
end
end
Tile->>Tile: updateTile() (ACTIVE/INACTIVE)
Tile->>QS: reflect tile state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Author
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/io/netbird/client/NetbirdTileService.java`:
- Around line 104-118: bindToVpnService currently starts the bind but you never
track a pending bind, so if onStopListening/unbindFromVpnService runs before
onServiceConnected the ServiceConnection remains; add a "bindingInProgress"
boolean set true in bindToVpnService, clear it in
serviceConnection.onServiceConnected and onServiceDisconnected, and modify
unbindFromVpnService to unbind when either isBound or bindingInProgress is true
(removing the serviceStateListener only if mBinder != null), then call
unbindService(serviceConnection) inside a try/catch to safely handle cases where
the system rejected the unbind, and reset isBound, bindingInProgress, and
mBinder appropriately.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/io/netbird/client/NetbirdTileService.java`:
- Around line 72-77: Ensure pendingClick cannot persist across failed/aborted
binds: in NetbirdTileService.onStopListening() clear/reset the pendingClick flag
immediately (and any related state) so a subsequent successful bind won't act on
an old click; additionally, in the bind flow (the methods handling bindService
success/failure—e.g., your ServiceConnection callbacks and the code paths around
bindService / onServiceConnected / onServiceDisconnected used in the bind logic
between lines ~107–124) handle bind failures by clearing pendingClick and
aborting any pending toggle actions. Make sure both unbindFromVpnService() and
any bind-failure/error branches consistently reset pendingClick to a safe
default.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/src/main/java/io/netbird/client/NetbirdTileService.java`:
- Around line 94-105: The toggle handler should avoid calling
startAndRunVpnService() when a bind is already in progress; update
handleToggle() to check the isBinding flag (or equivalent) before initiating the
service: if mBinder is null and isBinding is true, just set pendingClick = true
and return without calling startAndRunVpnService(), otherwise (when not binding)
set pendingClick and call startAndRunVpnService(); reference the handleToggle
method, mBinder, isBinding, pendingClick and startAndRunVpnService to locate and
implement this guard.
- Around line 84-88: NetbirdTileService currently calls
startActivityAndCollapse(Intent) which is deprecated and throws on Android 14+;
replace that call by building a PendingIntent for MainActivity (use the same
Intent with FLAG_ACTIVITY_NEW_TASK | FLAG_ACTIVITY_CLEAR_TOP), create the
PendingIntent with FLAG_UPDATE_CURRENT | FLAG_IMMUTABLE, and pass that
PendingIntent to startActivityAndCollapse(PendingIntent) instead of the raw
Intent so it works on SDK 35+ and avoids UnsupportedOperationException.
🧹 Nitpick comments (1)
app/src/main/java/io/netbird/client/NetbirdTileService.java (1)
39-45: Consider removing the service state listener inonServiceDisconnected.When the service disconnects unexpectedly (e.g., process killed),
mBinderis set to null without removing theserviceStateListener. While the listener reference will become stale, explicitly removing it before nulling the binder ensures cleaner lifecycle management and mirrors the pattern inunbindFromVpnService().♻️ Suggested improvement
`@Override` public void onServiceDisconnected(ComponentName name) { + if (mBinder != null) { + mBinder.removeServiceStateListener(serviceStateListener); + } mBinder = null; isBound = false; isBinding = false; updateTile(); }
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have been using NetBird for some time now, but I find it annoying to need to into the app to turn the VPN connection on. I have created a Icon and button to exist in the Quick Settings notification panel. so that this can be easily accessed without needing to go into the NetBird app
Summary by CodeRabbit