Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,9 @@ gives the timestamped jar name; a CI run started before that timestamp cannot se
with `DtcNewCodesAnnouncer.announce`, so every connect path is covered; only `CONNECT` scans with a VIN reply; all
codes on the vehicle's first scan, else only `diff.isNew` ones. "First scan" is read before `DtcHistoryStore.append`:
history replaces a connect scan equal to the previous one, so "one scan stored" re-announced unchanged codes forever.
Channel `dtc_alerts` is IMPORTANCE_HIGH (pop-up); default importance only lists it in the shade. Importance is frozen
once a channel exists on a device: changing it means a new channel id.
A pop-up shows only the collapsed line: keep it short (`DtcAnnouncement.summary`), full list in `bigText`.
Posted from `:datalogger` (service), not from `MainActivity`'s receivers, which die with the activity. Gate
`pref.dtc.notify_new_codes` (default on; the old `pref.dtc.show_notification` toast on *any* codes is gone). AA gets
`DATA_LOGGER_DTC_NEW_CODES` as a CarToast on `SurfaceRendererScreen` only. A notification's `PendingIntent` needs its
Expand Down
6 changes: 4 additions & 2 deletions common/src/main/java/org/obd/graphs/Notifications.kt
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ object Notifications {
text: String,
pendingIntent: PendingIntent? = null,
isOngoing: Boolean = false,
importance: Int = NotificationManager.IMPORTANCE_DEFAULT
importance: Int = NotificationManager.IMPORTANCE_DEFAULT,
// Shown when expanded; text is the collapsed (and pop-up) line. Null = text in both.
bigText: String? = null
) {
try {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
Expand All @@ -68,7 +70,7 @@ object Notifications {
val builder = NotificationCompat.Builder(context, channelId)
.setContentTitle(title)
.setContentText(text)
.setStyle(NotificationCompat.BigTextStyle().bigText(text))
.setStyle(NotificationCompat.BigTextStyle().bigText(bigText ?: text))
.setSmallIcon(R.drawable.ic_mygiulia_logo)
.setPriority(NotificationCompat.PRIORITY_DEFAULT)
.setOngoing(isOngoing)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.obd.graphs.bl.datalogger.dtc

import android.app.NotificationManager
import android.app.PendingIntent
import android.content.Context
import android.util.Log
Expand All @@ -34,18 +35,30 @@ const val PREF_DTC_NOTIFY_NEW_CODES = "pref.dtc.notify_new_codes"

private const val LOG_TAG = "DtcNewCodes"
private const val NOTIFICATION_ID = 12347
private const val CHANNEL_ID = "dtc_new_codes"

// High importance: a fault alert pops up (heads-up); default importance only lists it. A channel's importance is
// fixed once created on a device, so changing it needs a new id ("dtc_new_codes" was the default-importance one).
private const val CHANNEL_ID = "dtc_alerts"
private const val MAX_NOTIFICATION_LINES = 8

// Codes in the collapsed line, which is all a pop-up shows (one line): more get cut mid-code on a phone.
private const val MAX_SUMMARY_CODES = 3

// What the scan on connect tells the driver. firstScan: the vehicle had no earlier scan, so every code
// counts ("Trouble codes found"); otherwise only the codes its history calls new ("New trouble codes").
// lines: one per code, "P0123-1A · Engine ECU". Broadcast to AA as the DATA_LOGGER_DTC_NEW_CODES extras.
// lines: one per code, "P0123-1A · Engine ECU"; codes: the same codes alone, "P0123-1A", for the one-line summary.
// Broadcast to AA as the DATA_LOGGER_DTC_NEW_CODES extras.
data class DtcAnnouncement(
val lines: List<String>,
val firstScan: Boolean
val firstScan: Boolean,
val codes: List<String> = emptyList()
) : Serializable {
// At most max lines, the rest as a count (the caller's "+N more" text).
fun shown(max: Int): Pair<List<String>, Int> = lines.take(max) to (lines.size - max).coerceAtLeast(0)

// "P0123-1A, P0100, U1601 +27": the line a pop-up shows.
fun summary(max: Int): String =
codes.take(max).joinToString(", ") + if (codes.size > max) " +${codes.size - max}" else ""
}

// Only the automatic scan on connect (a read the user started is on screen anyway), and only for a vehicle
Expand All @@ -62,7 +75,7 @@ fun dtcAnnouncement(
if (kind != DtcScanKind.CONNECT || historyKey(vin) == UNKNOWN_VEHICLE) return null
val diff = dtcHistoryDiff(scans)
val codes = dtc.filter { firstScan || diff.isNew(it) }.sortedForDisplay()
return if (codes.isEmpty()) null else DtcAnnouncement(codes.map { it.announcementLine() }, firstScan)
return if (codes.isEmpty()) null else DtcAnnouncement(codes.map { it.announcementLine() }, firstScan, codes.map { it.displayCode() })
}

// "P0123-1A · Engine ECU"; the bare code when the module is the default one.
Expand Down Expand Up @@ -113,8 +126,10 @@ internal object DtcNewCodesAnnouncer {
channelName = context.getString(R.string.dtc_new_codes_channel),
channelDescription = context.getString(R.string.dtc_new_codes_channel_description),
title = context.getString(title, announcement.lines.size),
text = (lines + listOfNotNull(more.takeIf { it > 0 }?.let { context.getString(R.string.dtc_codes_more, it) })).joinToString("\n"),
pendingIntent = openDtcList(context)
text = announcement.summary(MAX_SUMMARY_CODES),
bigText = (lines + listOfNotNull(more.takeIf { it > 0 }?.let { context.getString(R.string.dtc_codes_more, it) })).joinToString("\n"),
pendingIntent = openDtcList(context),
importance = NotificationManager.IMPORTANCE_HIGH
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class VehicleCapabilitiesManagerTest : TestSetup() {
every { org.obd.graphs.sendBroadcastEvent(any<String>()) } returns Unit
every { org.obd.graphs.sendBroadcastEvent(any<String>(), any<Serializable>()) } returns Unit
mockkObject(Notifications)
every { Notifications.show(any(), any(), any(), any(), any(), any(), any(), any(), any(), any()) } returns Unit
every { Notifications.show(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()) } returns Unit
DtcHistoryStore.file = { File(folder.root, "dtc_history.json") }
DtcHistoryStore.reset()
}
Expand Down Expand Up @@ -339,8 +339,8 @@ class VehicleCapabilitiesManagerTest : TestSetup() {

connectScans(setOf(dtc("P0100", "a")), setOf(dtc("P0100", "a"), dtc("P0200", "b")))

verify(exactly = 1) { org.obd.graphs.sendBroadcastEvent(DATA_LOGGER_DTC_NEW_CODES, DtcAnnouncement(listOf("P0200"), firstScan = false)) }
verify(exactly = 1) { Notifications.show(any(), any(), any(), any(), any(), any(), "P0200", any(), any(), any()) }
verify(exactly = 1) { org.obd.graphs.sendBroadcastEvent(DATA_LOGGER_DTC_NEW_CODES, DtcAnnouncement(listOf("P0200"), firstScan = false, codes = listOf("P0200"))) }
verify(exactly = 1) { Notifications.show(any(), any(), any(), any(), any(), any(), "P0200", any(), any(), any(), "P0200") }
}

@Test
Expand All @@ -351,7 +351,7 @@ class VehicleCapabilitiesManagerTest : TestSetup() {
VehicleCapabilitiesManager.updateDTC(setOf(dtc("P0100", "a"), dtc("P0200", "b")), now = 10, kind = DtcScanKind.READ)

verify(exactly = 1) { org.obd.graphs.sendBroadcastEvent(DATA_LOGGER_DTC_NEW_CODES, any<Serializable>()) }
verify(exactly = 1) { org.obd.graphs.sendBroadcastEvent(DATA_LOGGER_DTC_NEW_CODES, DtcAnnouncement(listOf("P0100"), firstScan = true)) }
verify(exactly = 1) { org.obd.graphs.sendBroadcastEvent(DATA_LOGGER_DTC_NEW_CODES, DtcAnnouncement(listOf("P0100"), firstScan = true, codes = listOf("P0100"))) }
}

@Test
Expand All @@ -361,7 +361,7 @@ class VehicleCapabilitiesManagerTest : TestSetup() {
connectScans(setOf(dtc("P0100", "a")), setOf(dtc("P0100", "a"), dtc("P0200", "b")))

verify(exactly = 0) { org.obd.graphs.sendBroadcastEvent(DATA_LOGGER_DTC_NEW_CODES, any<Serializable>()) }
verify(exactly = 0) { Notifications.show(any(), any(), any(), any(), any(), any(), any(), any(), any(), any()) }
verify(exactly = 0) { Notifications.show(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()) }
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class DtcNewCodesTest {
val announcement = announced(listOf(dtc("P0100"), dtc("P0200")), scan(1, dtc("P0100")))!!

assertEquals(listOf("P0200"), announcement.lines)
assertEquals(listOf("P0200"), announcement.codes)
assertFalse(announcement.firstScan)
}

Expand Down Expand Up @@ -112,4 +113,12 @@ class DtcNewCodesTest {
assertEquals(listOf("P01", "P02", "P03") to 7, announcement.shown(3))
assertEquals(announcement.lines to 0, announcement.shown(20))
}

@Test
fun `the pop-up line lists the first codes alone and counts the rest`() {
val announcement = announced((1..5).map { dtc("P010$it", "Engine ECU") })!!

assertEquals("P0101, P0102, P0103 +2", announcement.summary(3))
assertEquals("P0101, P0102, P0103, P0104, P0105", announcement.summary(5))
}
}
14 changes: 10 additions & 4 deletions doc/specs/dtc-new-code-notification.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ the DTC list from the notification, old toast removed), `:automotive` (car toast

| Area | Before | After |
|---|---|---|
| Connect scan finds a code the vehicle never reported | Toast "DTC are available" if the setting was on, app in foreground only | Phone notification "New trouble codes (N)", one line per code (`P0123-1A · Engine ECU`) |
| Connect scan finds a code the vehicle never reported | Toast "DTC are available" if the setting was on, app in foreground only | Pop-up (heads-up) phone notification "New trouble codes (N)", one line per code (`P0123-1A · Engine ECU`) |
| Connect scan, only known codes | Same toast, every connect | Nothing |
| Tap on the notification | — | Opens the app on the DTC settings screen with the DTC list dialog open |
| Android Auto, main (surface) screen | Nothing | Car toast "New trouble codes (N): P0123-1A · Engine ECU, ..." |
| Manual read / clear | Nothing | Nothing (the result is on screen) |
| Vehicle's first scan (e.g. after a fresh install) | Toast (if on) | Notification "Trouble codes found (N)" with every code; AA "Trouble codes found" |
| Vehicle without a VIN reply | Toast (if on) | Nothing: its codes may come from any car (`NO_VIN`, a VIN not programmed, does announce) |
| Many codes | — | Notification lists 8, then "+N more"; AA toast lists 3 |
| Many codes | — | Expanded notification lists 8, then "+N more"; AA toast lists 3 |
| Pop-up / collapsed line | — | Codes only, at most 3: `P0120-14, P0100, U1601 +27` (a pop-up shows one line; module names cut it mid-code) |
| Setting | "Enable DTC notifications", off | "Notify about new trouble codes", **on** by default |

"New" is the history's definition (`DtcHistoryDiff.newCodes`): in this scan, in no earlier scan of the vehicle that
Expand Down Expand Up @@ -57,7 +58,12 @@ not count as having looked.
notification. The notification is posted from the service process, not the activity's receiver: the activity's
receivers are gone once it is destroyed, the service runs while driving. A failure to notify is logged and never
breaks storing the scan.
* Notification: channel `dtc_new_codes` (default importance), id 12347, small icon as the other notifications.
* Notification: channel `dtc_alerts`, **high importance** so it pops up (heads-up); id 12347, small icon as the other
notifications. Wrong turn: default importance (first channel `dtc_new_codes`) only put it in the shade, no pop-up.
A channel's importance cannot be changed from code once created on a device, hence the new id.
A pop-up shows only the collapsed layout (title + one line), never the expanded text, so the two differ:
`Notifications.show(text = summary, bigText = lines)`. Wrong turn: the first "code · module" line as the collapsed
text, cut mid-sentence in the pop-up.
Its `PendingIntent` uses its own request code: launch intents differ only by extras, and the code-0 launch intent
of the service / GPS notifications would otherwise replace the navigation extra.
* Opening the list: the launch intent carries `EXTRA_NAVIGATE_TO = "dtc"` (constant moved to `:common`).
Expand All @@ -78,7 +84,7 @@ not count as having looked.

* `DtcNewCodesTest` (done): connect scan announces only new codes; read / clear announce nothing; first scan announces
every code (none when empty); no VIN reply announces nothing, `NO_VIN` does; known codes are not announced again;
display order; line format; line cap.
display order; line format; line cap; pop-up summary.
* `VehicleCapabilitiesManagerTest` (done), end to end through `updateDTC` and the history file: new code → broadcast
with its line + notification; first scan → all codes, the same codes on the next connect (history replaces the
scan) and a manual read → nothing; setting off → nothing; no VIN → nothing.
Expand Down
Loading