FullCharge: Silence the gesture notification, warn on failed restores - #33
Merged
Conversation
The reconnect-gesture channel drops from DEFAULT to LOW so it lands in the shade's silent section. DEFAULT bought nothing: all four foreground-service notifications share SESSION_ID and set onlyAlertOnce, so the reconnect countdown never re-alerted and the importance only produced one alert when the service went foreground. LOW keeps the status-bar icon that makes an armed gesture discoverable. The recovery channel goes the other way, DEFAULT to HIGH, and its notification gains CATEGORY_ERROR. It fires when the protective policy could not be restored, so the battery charges unprotected until the user intervenes — it was ranked below the charge alarm, a convenience reminder. It is rare, auto-cancels, and is withdrawn as soon as a restore succeeds. Neither importance change reaches an existing install: a channel's importance is fixed once created. That is deliberate — no migration channel ids while the app is pre-launch. The session channel is renamed "Temporary full charge" -> "Full charge and restore" because recovering() shares it with session(), so it covers both lifting the limit and putting it back. Its id stays temporary_full_charge; ids are permanent and changing one would reset the channel's settings. Names and descriptions, unlike importance, do update on an existing channel. Also extracts the two hardcoded English channel descriptions to strings.xml and adds the missing one for the recovery channel, which was the only channel describing itself with a bare name.
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.
What changed
The reconnect-gesture notification is now silent. It sits in the notification shade's less-important section instead of the default one, so it no longer makes a sound when it appears. It still shows a status-bar icon, so an armed gesture stays discoverable. Previously the only way to get this was to change it by hand in Android's notification settings.
"Charge limit needs attention" now alerts properly. This is the notification that appears when Amply could not put your protective charge limit back — the battery keeps charging unprotected until you step in. It was ranked below the charge alarm, so a convenience reminder was more prominent than the one failure the app exists to prevent. It now alerts at the same level, and is marked as an error so Do Not Disturb and notification ranking treat it accordingly. It is rare, dismisses itself, and disappears the moment a restore succeeds.
One notification category was renamed. "Temporary full charge" is now "Full charge and restore", because it covers both halves of the lifecycle — allowing the full charge, and putting your limit back afterwards. The restore-progress notification was already filed under it, which the old name didn't reflect.
Every notification category now describes itself in Android's notification settings. One had no description at all, and two others were not translatable.
Technical Context
Why the gesture channel lost nothing by dropping to LOW. All four foreground-service notifications (
monitoring,session,gesture,recovering) shareSESSION_IDand setsetOnlyAlertOnce(true). TheWAITING_FOR_RECONNECTcountdown is an update to an already-posted notification, so it never re-alerted regardless of importance. DEFAULT bought exactly one alert, when the service first went foreground. LOW keeps the status-bar icon; only MIN would have removed it.Neither importance change reaches an existing install, deliberately. A channel's importance is fixed at creation —
createNotificationChannelon an existing channel ignores it, and a channel the user has adjusted has that field locked outright. Reaching existing installs would need new channel ids, which is migration cruft the app does not need pre-launch. Names and descriptions do update on an existing channel, so the rename and the new descriptions apply everywhere immediately. Verifying the two importance changes therefore requires a fresh install —pm cleardoes not delete channels.Why
recovering()still shares the session channel rather than moving tocharge_policy_recovery, whose name matches it better: that channel is now HIGH, which would make a passive progress notification heads-up on every boot that owes a restore. It is the passive tail of the same override the user started, so it belongs withsession(). The channel rename is what resolves the mismatch. There is a comment on the function recording this, so it does not get "fixed" later.The session channel keeps its
temporary_full_chargeid despite the rename. Channel ids are invisible to users and permanent; changing one resets that channel's settings. Also commented at the constant.Review guidance
The behavioural surface is four constant/string values; the rest is comments explaining decisions that are easy to reverse by accident. Worth a look:
charge_policy_recovery? It is the one judgement call here. The argument is severity — unprotected overnight charging — against the risk of a heads-up for something the user may not be able to fix immediately (it fires on write failures, including transient ones).enableVibration(true), whichcharge_alarmdoes. So it will heads-up and sound but may not buzz. Left inconsistent on purpose rather than silently widened; say the word and it is one line.Testing
Both flavours assemble, 739 unit tests pass,
lintFossDebugreports 0 errors and no new warnings. Not yet verified on a device — that needs a fresh install to observe the new importances, which is why this is a draft.