Modernize the app and fix countless connection stability issues#229
Open
FlorentRevest wants to merge 22 commits into
Open
Modernize the app and fix countless connection stability issues#229FlorentRevest wants to merge 22 commits into
FlorentRevest wants to merge 22 commits into
Conversation
The MTU was pinned to 256 with a comment claiming a 244-byte payload (it is actually MTU - 3 = 253). That arbitrary value needlessly fragments every transfer. Now that the previous commit clamps each GATT write to the 512-byte attribute limit regardless of the negotiated MTU, it is safe to request the spec maximum (517): the connection negotiates the largest MTU both ends support, transfers use fewer/larger packets, and CAPPED_SPLITTER still guarantees no write exceeds 512 bytes. Peers that cannot do 517 negotiate down automatically.
createBond() was called on every connect attempt, even when the device was already bonded, immediately before connect(). This races the bonding state machine against GATT connection setup and is a well-known cause of error 133 / failed service discovery (the "forget and re-pair" symptom). Now bonding is only initiated when the device is BOND_NONE. Unexpected disconnects (link loss, timeout, peer-terminated) previously left the watch disconnected with no recovery until the app was restarted. A managed reconnect is now scheduled on such disconnects, gated by an explicit user-initiated-disconnect flag and the current connection state so it never fights a deliberate disconnect nor double-connects while the stack is already (re)connecting. mState is made volatile as it is touched from both BLE callback threads and the main handler.
Services are driven by broadcasts, content observers, alarms and network callbacks that fire independently of the BLE link. SynchronizationService .send() now drops writes unless the state is CONNECTED, and AsteroidBleManager.send() is null-safe instead of calling Objects.requireNonNull() on a characteristic that does not exist when disconnected. Together these remove a whole family of NullPointerException crashes (e.g. a notification arriving while the watch is briefly out of range).
A flaky reconnect can return a partial GATT table. The discovery code dereferenced gatt.getService()/getCharacteristic() without null checks, throwing a NullPointerException inside isRequiredServiceSupported() that aborted the whole connection and trapped reconnects in a failing retry loop. Missing services and characteristics are now skipped. Also removed the meaningless addCharacteristic() calls on already-discovered services.
onDestroy() called disconnect() without enqueue(), so the request never ran and the GATT connection was leaked on service teardown; it is now enqueued. updateNotification() now always calls startForeground() instead of only when a device is set, so a service launched via startForegroundService() cannot be killed for failing to go foreground in time. AutostartService starts the service with startForegroundService() on Android 8+, where starting a plain background service from the boot broadcast throws IllegalStateException and the watch silently never reconnects after a reboot.
On a "refresh" request before the listener was connected, NLService spun in a while(!listenerConnected) loop on the main thread, freezing the UI and guaranteeing an ANR if the listener never connected. The refresh is now deferred via a flag and fired from onListenerConnected(); when already connected it posts the push after a short settle delay. Also guards against a null broadcast command and wraps getActiveNotifications in a try/catch.
The MEDIA_COMMANDS_CHAR callback indexed data[0]/data[1] without verifying the payload length, crashing with ArrayIndexOutOfBoundsException on a short/empty packet from the watch. onMetadataChanged and the volume content observer dereferenced getPlaybackInfo() (which can be null) and divided by getMaxVolume() (which can be 0). All of these are now guarded, and the content observer uses the main looper explicitly instead of a bare new Handler().
The download path advanced progress by the full chunk length even when the bounds-checked arraycopy was skipped, so a desynced or oversized stream could push progress past size; the size == progress completion check then never fired, leaving the per-second progress executor running forever and the buffer copy potentially out of bounds. A data chunk arriving with no preceding header dereferenced a null buffer. The announced size from the header was used unchecked for new byte[size] (NegativeArraySize / OOM). Now the header size is validated against a sane ceiling, chunks are clamped to the remaining buffer, completion triggers on progress >= size, and the executor is shut down on completion and on unsync (disconnect mid-download).
onReceive() registered a fresh PhoneStateListener via telephony.listen() on each ACTION_PHONE_STATE_CHANGED broadcast and never unregistered it. Since that broadcast fires multiple times per call and the receiver instance is discarded after onReceive() returns, listeners accumulated, leaking and sending duplicate ring notifications to the watch. The call state is now read directly from the broadcast extras (EXTRA_STATE / EXTRA_INCOMING_NUMBER), so no listener is registered at all. Also wraps the contact cursor in try/finally and guards a null number.
If silence-on-connect was enabled and a session was killed before unsync() could restore the ringer, the next sync() saved RINGER_MODE_SILENT as the user's "original" mode, leaving the phone permanently muted. sync() now falls back to NORMAL when it would otherwise record SILENT as the original.
GPSTracker registers a location listener via requestSingleUpdate() but it was only stopped when weather sync was toggled off, not on disconnect, so a pending update outlived the connection. unsync() now stops and clears the GPS tracker.
The app uses the Nordic BLE library (no.nordicsemi.android:ble); SweetBlue is not referenced by any source file, nor by app/build.gradle.kts sourceSets, nor included as a module in settings.gradle.kts (only :app is). The vendored tree was dead weight, so drop it.
com.github.MagneFire:EasyWeather:1.3 is no longer resolvable: JitPack has evicted the artifacts and can no longer rebuild them (every version reports an "Error" build status), so the build failed at checkDebugAarMetadata with "Could not find com.github.MagneFire:EasyWeather:1.3". Vendor the library's 18 source files under src/main/lib/easyweather and add its runtime dependencies (Retrofit, converter-gson, OkHttp logging interceptor) directly from Maven Central, mirroring how the project already vendors its other libs. The single android.support.annotation.NonNull import is migrated to androidx. Resources/manifest are not vendored (unused, and the library's app_name string would collide). The now-unused JitPack repository is removed so the build no longer depends on it at all. EasyWeather is Apache-2.0 (Copyright 2016 Vatsal Bajpai); see the vendored README for attribution.
- Android Gradle Plugin 7.4.2 -> 8.5.2, Gradle 7.5 -> 8.7. Builds now run
on JDK 17-21 (the old AGP failed jlink's JdkImageTransform on JDK 21).
- Move to the plugins {} DSL and centralize repositories in
settings.gradle.kts via dependencyResolutionManagement
(FAIL_ON_PROJECT_REPOS), replacing the legacy buildscript/allprojects
blocks. Drop the unused Kotlin Gradle plugin classpath.
- compileSdk 33 -> 34; drop the hard-pinned buildToolsVersion (AGP selects
it). targetSdk stays 34-pending and is bumped separately with its
required runtime adaptations.
- gradle.properties: drop android.enableJetifier (no support-library
dependencies remain after EasyWeather was migrated to AndroidX) and set
android.nonTransitiveRClass=true (AGP 8 default).
- Bump low-risk dependencies: appcompat 1.7.0, material 1.12.0, gson
2.11.0, retrofit 2.11.0, osmdroid 6.1.20. The Nordic BLE libraries are
intentionally kept pinned so the tuned connection lifecycle is undisturbed.
- Declare android:foregroundServiceType="connectedDevice" on SynchronizationService and add the FOREGROUND_SERVICE_CONNECTED_DEVICE permission, as required for foreground services on Android 14. - Pass an explicit export flag to every context-registered receiver via ContextCompat.registerReceiver(..., RECEIVER_NOT_EXPORTED). Android 14 requires this for apps targeting 34, otherwise registerReceiver throws at runtime. All five receivers (notification listener bridge, notification, time, weather, screenshot) only handle app-internal or system-protected broadcasts, so NOT_EXPORTED is correct.
Functional fixes (these silently misbehaved on modern Android): - BluetoothAdapter.enable() has been a no-op since Android 13, so the app could no longer turn Bluetooth on; it now launches ACTION_REQUEST_ENABLE to ask the user. - ActivityCompat.requestPermissions(getParent(), ...) passed getParent(), which is null for a normal activity, so those BLUETOOTH_CONNECT permission requests no-op'd / could NPE. Pass the activity itself. - Remove the bogus android.permission.ACCESS_LOCATION manifest entry (not a real permission; the real ACCESS_FINE/COARSE_LOCATION are already declared). Deprecation replacements: - BluetoothAdapter.getDefaultAdapter() -> BluetoothManager.getAdapter(). - Override onBackPressed() -> OnBackPressedDispatcher + OnBackPressedCallback (predictive-back ready); the toolbar home button routes through the dispatcher too. - Replace the finish()/overridePendingTransition()/startActivity() theme restart with recreate(), which keeps the task and back stack intact.
Replace the deprecated BluetoothAdapter.getDefaultAdapter() calls with BluetoothManager.getAdapter() via a small helper, and null-check the adapter before use.
Nothing in the app references legacy-support-v4: the androidx.core / androidx.fragment classes used come transitively from appcompat and material, and the vendored material-intro-screen's ViewPager resolves through material. Both debug and release (incl. lintVitalRelease) build without it.
Turn on org.gradle.caching, org.gradle.parallel and org.gradle.configuration-cache. All three were verified against this project on both the debug and release paths (configuration cache stores and reuses with no reported problems), giving faster incremental and clean builds.
Move all plugin and library versions into gradle/libs.versions.toml and reference them via the libs.* accessors in the build scripts. This is the modern, tooling-friendly way to manage versions (and plays nicely with automated dependency updates). No version changes; build output is unchanged.
Targeting SDK 34 (commit "Target SDK 34 with Android 14 runtime adaptations") registered the context receivers with RECEIVER_NOT_EXPORTED, but the matching broadcasts are sent as implicit intents (action only, no package). On Android 14 a fully implicit broadcast is not delivered to a NOT_EXPORTED receiver, which silently broke every receiver-driven feature: screenshot requests, the "find my watch" buzz, notification forwarding and the call-state notifications — while internal sends (media, weather, time) kept working. Set the package on every internal broadcast (UI cards, NLService, PhoneStateReceiver, the notification-refresh trigger and the time/weather alarm intents) so they are delivered to the app's own private receivers without exporting them.
Flip PREFS_SYNC_WEATHER_DEFAULT to true so weather tracks the device location out of the box (the location-picker already exposes the toggle to turn it off). Also seed the GPS cold start from the last known location so the first sync shows weather immediately instead of waiting up to two minutes for the single-update fix. Degrades gracefully when no location or permission is available.
MagneFire
reviewed
Jun 21, 2026
MagneFire
left a comment
MagneFire
left a comment
Member
There was a problem hiding this comment.
Only had a quick look through the changes. Looks good, but have not tested any of this.
| // safe because CAPPED_SPLITTER above clamps every write to the 512-byte | ||
| // attribute limit no matter what MTU the system negotiates; the peer always | ||
| // falls back to a smaller MTU if it can't support the maximum. | ||
| private static final int GATT_MAX_MTU = 517; |
Member
There was a problem hiding this comment.
This probably needs to be tested on various phones as the Bluetooth stack is known to be buggy for different vendors. Google Pixel seems to be one of the few to have a proper Bluetooth stack. Looking at the documentation, pre-Android 14 probably need to be checked (https://developer.android.com/about/versions/14/behavior-changes-all#mtu-set-to-517).
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
2 participants
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.
No description provided.