Fix: Open the right battery screen on unsupported devices - #78
Merged
Conversation
Tapping "Open battery settings" on any device Amply carries no adapter for went straight to the system Battery saver screen, which holds no charge-protection toggle. Reported on a HONOR Magic8 Pro (issue #66), but it applied to every unmapped brand: Motorola, Nothing, Sony, Fairphone, Vivo, Tecno, Huawei. nativeSettingsIntent() returned null when the registry matched no adapter, and its only caller substituted ACTION_BATTERY_SAVER_SETTINGS outright. So ACTION_POWER_USAGE_SUMMARY was never attempted, even though all four lab adapters deliberately prefer it and the manifest already declares its <queries> visibility. Same user-visible symptom the ledger records for LineageOS, reached by a different path: that one fell through to the Pixel component intent, this one had no adapter object to ask. The chain moves to OemChargingShortcuts.genericBatterySettings so the null path and DisabledLabAdapter share one implementation rather than gaining a sixth copy, and the repository's return type becomes non-null so no caller has to invent a fallback again. The four OEM adapters keep their own copies on purpose: converting them would couple qualified per-device navigation to whatever "generic" comes to mean later. Also closes a crash path in the same function, which pre-dated this fix for unmapped devices. On a launch failure the handler retried the same Battery saver intent outside runCatching, so when that intent was the one that failed, the second throw escaped. It now skips a fallback that would repeat the failed action, and guards and logs both attempts. Whether POWER_USAGE_SUMMARY resolves on MagicOS 10 is still unverified, so this is not confirmed to change what the reporter sees.
The KDoc and its test comment both claimed that a firmware reporting CURRENT_NOW in mA where Android documents uA "would look like ~1000x too much power", and that the plausibility cap therefore catches it. That is backwards, which is why the cap never caught anything of the kind: mA where uA is expected makes every reading 1000x too SMALL. A real 4 A arrives as the integer 4000, renders as "4 mA", and computes to 18 mW, which the UI formats as "0.0 W". Reported on HONOR MagicOS 10 (issue #66), where the charge counter is scaled the same way, so the ROM is a plausible instance. Not fixed here, deliberately: no lower bound can separate that from a genuine end-of-charge trickle, because a phone drawing single-digit mA at 100% produces the identical value. A clamp would break correct readings on healthy devices, so the pinning test states why not to add one, and the real fix needs context this function does not have. Also stops overstating what the upper cap does. It rejects results above 250 W, which is a backstop against gross over-reporting, not a unit check: a 10x over-report of 4 A at 4.551 V lands at ~182 W and is accepted, as the existing 20 A case already demonstrated.
The Magic8 Pro contributor (issue #66) ran the dumpsys measurement, and the result is unfavourable in a way the mapping alone could not show. The cap is real: 278 mAh went into the cell after the feature was disabled, at a point where the broadcast had reported level 100 for over a minute. That confirms the synthetic 100% against the broadcast rather than the status bar, so the session-engine hazard recorded earlier is now evidenced instead of inferred. But it is only ~4%. The C636 variant is the 7100 mAh model, corroborated by the contributor's own ~7121 mAh reading, so Smart battery capacity is a top-of-charge voltage trim, not an 80%-cap equivalent, which would have plateaued near 5800 mAh. Combined with Smart charge reaching 100% overnight, neither HONOR key is a hard cap. An adapter here could offer Adaptive on and off with no percentage at all, carrying the allowsFullCharge honesty gap already recorded for Xiaomi. The open question is therefore no longer whether the mapping works but whether that adapter is worth building. Noted that the ~4% figure is a ratio, so it survives the charge-counter unit question. policyLatchesAtPlug behaviour observed: disabling the feature with the cable connected moved nothing, and the charge only went in after a replug. Same family as GrapheneOS, so an adapter would reuse the existing pending-until-replug handling and could not support the reconnect gesture. Blocker 2's identity mechanism is resolved. MagicOS exposes app-readable system features and core com.hihonor.* system packages, the same two mechanisms that already carry LineageOS and GrapheneOS, so no property read is needed. Version scoping is still unsolved: com.hihonor.magic.api.23 does not line up with ro.build.magic_api_level=42, so the features answer "is this MagicOS" and not "is this MagicOS 10", and a features-only gate would be the first unscoped one in the project. Records the three app defects the device exposed, one fixed here, one deferred by decision (the hasSupportLead gate hides the metadata report on exactly the unrecognized ROMs whose codename it would carry), and one awaiting a single contributor reading.
d4rken
added
bug
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
On a phone Amply has no charge-protection support for, the "Open battery settings" button opened the system Battery saver screen. That screen has no charge-protection toggle in it, so the button led nowhere useful on exactly the devices that needed it most. It now opens the battery-usage screen, which on most manufacturer skins is where the built-in charging-protection option actually lives, and only falls back to Battery saver when a phone has no battery-usage screen at all.
This was reported on a HONOR phone but applied to every brand Amply carries no support for, including Motorola, Nothing, Sony, Fairphone, Vivo and Tecno. The same button in the "Help add support" wizard is fixed by the same change.
Also fixes a crash: if opening the battery screen failed, the retry could throw and take the action down with it.
The remaining two commits change no behavior. One corrects a wrong explanation in the code about how charging-power readings can be misreported, and pins the current behavior in a test. The other records a contributor's hardware measurements in the device-qualification ledger.
Technical Context
ChargingRepository.nativeSettingsIntent()returned null whenAdapterRegistrymatched no adapter, and its single caller substitutedACTION_BATTERY_SAVER_SETTINGSoutright.ACTION_POWER_USAGE_SUMMARYwas therefore never attempted, despite all four lab adapters preferring it and the manifest already declaring its<queries>visibility. Same user-visible symptom the ledger records for LineageOS, reached by a different path (that one fell through to the Pixel component intent; this one had no adapter object to ask at all).OemChargingShortcuts.genericBatterySettingsso the null path andDisabledLabAdaptershare one implementation instead of a sixth copy, and the repository's return type is now non-null so no caller can reintroduce its own fallback. The four OEM adapters keep their private copies deliberately: converting them would couple qualified per-device navigation to whatever "generic" comes to mean later.runCatching, so when that intent was the one that failed, the second throw escaped. It now skips a fallback that would repeat the failed action, and guards and logs both attempts. Worth a close look, since it is the one place this PR changes control flow rather than intent selection.POWER_USAGE_SUMMARYis verified to resolve only under Robolectric here. Whether it resolves on MagicOS 10 specifically is unconfirmed, so this is not proven to change what the original reporter sees. The change is correct for unmapped devices generally either way.StatsPowerCalculatorcommit deliberately does not fix the underlying defect. A firmware reportingCURRENT_NOWin mA where Android documents µA makes readings 1000× too small, which the plausibility cap cannot catch, but no lower bound can separate that from a genuine end-of-charge trickle either. The test states why not to add one; the real fix needs charge-level context this function does not have.Refs #66