Skip to content

Commit ea4757e

Browse files
malysclaude
andcommitted
refactor(chargepilot): CP-080 says each thing once
A ponytail review of c97170d: 27 lines of KDoc over 14 lines of code, and the same fact — 148 km driven, 32 km recorded — written five times across the commit, the KDoc, the manifest, the allowlist and the ticket. The KDoc keeps only what a reader of the file cannot derive from it: why exported is safe on a protected broadcast, why the action is checked anyway, why failures are caught. The measurement and the cost live in CP-080, which is where a reviewer looks for them. The manifest comment defers to the allowlist, which is the artefact the blocking CI gate actually reads. MY_PACKAGE_REPLACED is dropped. An update is a manual act — the user is in front of the screen and opens the app afterwards, which already restarts the monitor — so it closed a gap bounded by one boot cycle at the cost of a second action and a longer guard. BOOT_COMPLETED alone covers the case that was measured. The success log goes with it: the service logs "service created" and "telemetry sampler started" one second later. The refusal log stays, because "automatic detection is off" is the one outcome nothing else explains, and this car has no ADB. Net 31 lines removed, behaviour unchanged on the path that was measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c97170d commit ea4757e

3 files changed

Lines changed: 19 additions & 50 deletions

File tree

‎.github/security/permission-allowlist.txt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ android.car.permission.CAR_VENDOR_EXTENSION
88
# Install-time permissions, no vehicle access, no network.
99
android.permission.FOREGROUND_SERVICE
1010
android.permission.FOREGROUND_SERVICE_SPECIAL_USE
11-
# CP-080, 2026-09-24. The monitor is restarted after a head-unit boot and after this app is
12-
# updated, because the process dies with the car and until now only opening the app brought
13-
# recording back — 116 of 148 km driven in one measured week were never recorded. Install-time,
14-
# no vehicle access, no network, no data. It starts the same monitor the dashboard starts, and
15-
# that monitor suspends itself after ten consecutive reads with no usable speed.
11+
# CP-080, 2026-09-24. The monitor is restarted after a head-unit boot, because the process dies
12+
# with the car and until now only opening the app brought recording back — 116 of 148 km driven
13+
# in one measured week were never recorded. Install-time, no vehicle access, no network, no data.
14+
# It starts the same monitor the dashboard starts, and that monitor suspends itself after ten
15+
# consecutive reads with no usable speed.
1616
android.permission.RECEIVE_BOOT_COMPLETED
1717
# Reviewed and approved 2026-09-04 (CP-043). The first two are declared in the main
1818
# manifest as of the same day, by the owner's decision, ahead of the routing client that

‎app/src/main/AndroidManifest.xml‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,7 @@
3030
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
3131
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_SPECIAL_USE" />
3232

33-
<!-- CP-080. Automatic trip detection is worth nothing if it only starts when somebody opens
34-
the app: the head unit shuts this process down with the car, and 116 of 148 km driven
35-
between two probe bundles were never recorded for exactly that reason. This is an
36-
install-time permission, it reaches no vehicle interface and it carries no data; what it
37-
starts is the same monitor the dashboard starts, which suspends itself after ten reads
38-
with no usable speed. -->
33+
<!-- CP-080; reviewed in .github/security/permission-allowlist.txt. -->
3934
<uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED" />
4035

4136
<!-- Route planning. Reviewed and accepted by the owner under CP-043 on 2026-09-04; the
@@ -163,16 +158,11 @@
163158
android:resource="@xml/file_paths" />
164159
</provider>
165160

166-
<!-- exported, because a manifest receiver is not handed a system broadcast otherwise.
167-
Both actions are protected broadcasts — only the platform may send them — and the
168-
receiver checks the action regardless. -->
169161
<receiver
170162
android:name=".TripMonitorBootReceiver"
171-
android:enabled="true"
172163
android:exported="true">
173164
<intent-filter>
174165
<action android:name="android.intent.action.BOOT_COMPLETED" />
175-
<action android:name="android.intent.action.MY_PACKAGE_REPLACED" />
176166
</intent-filter>
177167
</receiver>
178168

‎app/src/main/java/com/evsuite/chargepilot/TripMonitorBootReceiver.kt‎

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,48 +8,27 @@ import com.evsuite.hardware.AppLogger
88
/**
99
* Restores automatic trip monitoring after a head-unit start, so recording is automatic in fact.
1010
*
11-
* [TripRecordingService] already outlives the dashboard — it is a *started* foreground service,
12-
* which is the whole reason a trip survives the driver switching to the media app. What it does
13-
* not survive is the car being switched off: the head unit shuts the process down with
14-
* everything else, and until CP-074 nothing started it again. The monitor therefore came back
15-
* only when somebody opened the app, and a drive begun without opening it was a drive nobody
16-
* recorded.
11+
* Three things a reader of this file cannot work out from it, and nothing else — the measurement
12+
* that motivated it and the cost it carries are in CP-080:
1713
*
18-
* CP-080. Measured, which is why this exists: the odometer moved 148 km between the 2026-09-17 and
19-
* 2026-09-24 bundles while the trip history recorded 32 km of it. Every threshold downstream is
20-
* paid for in recorded kilometres — the consumption fit needs a span of speeds, the health
21-
* estimate needs complete discharges the app watched, and eco-driving needs the drive it is
22-
* scoring — so coverage is upstream of all of them.
23-
*
24-
* **What it costs is bounded by the service, not by a promise here.** With automatic detection
25-
* off, nothing starts. With it on, the service samples at 1 Hz and suspends itself after ten
26-
* consecutive reads with no usable speed, which is what a head unit that booted without the
27-
* vehicle interfaces looks like. It writes to disk only when a completed trip is stored.
28-
*
29-
* `exported` is true because a manifest receiver does not hear a system broadcast otherwise,
30-
* and it is safe here for the reason the steering-wheel receiver elsewhere in this suite is
31-
* not: `BOOT_COMPLETED` and `MY_PACKAGE_REPLACED` are protected broadcasts, which only the
32-
* platform may send. The action is checked anyway — a receiver that acts on whatever arrives is
33-
* a receiver that trusted its manifest.
14+
* - `exported` is true because a manifest receiver is not handed a system broadcast otherwise,
15+
* and it is safe here for the reason the steering-wheel receivers in this suite are not:
16+
* `BOOT_COMPLETED` is a protected broadcast, which only the platform may send.
17+
* - The action is checked regardless, because a receiver that acts on whatever arrives is a
18+
* receiver that trusted its manifest.
19+
* - Failures are caught because a crash in a boot receiver is the kind of instability this suite
20+
* may not introduce on a head unit, and the service is legitimately unreachable on a device
21+
* still unlocking.
3422
*/
3523
class TripMonitorBootReceiver : BroadcastReceiver() {
3624

3725
override fun onReceive(context: Context, intent: Intent?) {
38-
val action = intent?.action
39-
if (action != Intent.ACTION_BOOT_COMPLETED && action != Intent.ACTION_MY_PACKAGE_REPLACED) {
40-
return
41-
}
26+
if (intent?.action != Intent.ACTION_BOOT_COMPLETED) return
4227
if (!TripRecordingService.isAutomaticDetectionEnabled(context)) {
43-
AppLogger.i(TAG, "boot: automatic trip detection is off, nothing started")
28+
AppLogger.i("EVChargePilot", "boot: automatic trip detection is off, nothing started")
4429
return
4530
}
46-
// A failure here must not take the platform's boot broadcast down with it: the service
47-
// is unavailable on a device still unlocking, and a crash in a boot receiver is the
48-
// kind of instability this app is not permitted to introduce on a head unit.
4931
runCatching { TripRecordingService.monitorAutomaticTrips(context) }
50-
.onSuccess { AppLogger.i(TAG, "boot: automatic trip monitor started; action=$action") }
51-
.onFailure { AppLogger.w(TAG, "boot: monitor could not start: ${it.message}") }
32+
.onFailure { AppLogger.w("EVChargePilot", "boot: monitor failed: ${it.message}") }
5233
}
53-
54-
private companion object { const val TAG = "EVChargePilot" }
5534
}

0 commit comments

Comments
 (0)