From 48ac4c26d62ed84faed015c448a21f07a9f3196f Mon Sep 17 00:00:00 2001 From: Tomek Zebrowski Date: Wed, 14 Oct 2026 14:33:41 +0200 Subject: [PATCH] ffeat(dtc): show modules that did not answer the scan instead of listing them as clean Per-module status from ObdMetrics is stored with the list and the history scan. A silent module gets a "No response" chip and message (refused: a "Refused" chip), sorts between modules with codes and clean ones, and is left out of the new / no longer reported comparison. AA shows it without the green check; the share report lists it; the legend explains the chips. --- CLAUDE.md | 7 ++ app/build.gradle | 6 +- ...sticTroubleCodePreferenceDialogFragment.kt | 10 +- .../dtc/DiagnosticTroubleCodeViewAdapter.kt | 29 ++++- .../res/drawable/dtc_module_no_response.xml | 11 ++ .../main/res/drawable/dtc_module_refused.xml | 11 ++ app/src/main/res/values-pl/strings.xml | 7 ++ app/src/main/res/values/strings.xml | 7 ++ .../graphs/preferences/dtc/DtcLegendTest.kt | 4 +- .../nav/DiagnosticTroubleCodesScreen.kt | 21 +++- automotive/src/main/res/values-pl/strings.xml | 2 + automotive/src/main/res/values/strings.xml | 2 + .../datalogger/VehicleCapabilitiesManager.kt | 18 +++- .../bl/datalogger/WorkflowOrchestrator.kt | 10 +- .../graphs/bl/datalogger/dtc/DtcHistory.kt | 20 +++- .../graphs/bl/datalogger/dtc/DtcListItem.kt | 48 +++++++-- .../VehicleCapabilitiesManagerTest.kt | 19 ++++ .../bl/datalogger/dtc/DtcHistoryDiffTest.kt | 32 ++++++ .../bl/datalogger/dtc/DtcListItemTest.kt | 55 ++++++++++ doc/specs/dtc-module-response.md | 101 ++++++++++++++++++ doc/specs/dtc-roadmap.md | 4 +- 21 files changed, 394 insertions(+), 30 deletions(-) create mode 100644 app/src/main/res/drawable/dtc_module_no_response.xml create mode 100644 app/src/main/res/drawable/dtc_module_refused.xml create mode 100644 doc/specs/dtc-module-response.md diff --git a/CLAUDE.md b/CLAUDE.md index 1e6f8c0f..fb409619 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -360,6 +360,13 @@ gives the timestamped jar name; a CI run started before that timestamp cannot se the user would never see the feature otherwise). AA gets "New" in the status line only, no gone section. On the first device test the user saw "no difference": markers need a *change*, and the history lived in an unmarked line of a row nobody knew expands. Hence the row chevron and the green "Code history" panel. +* **A module that did not answer is not clean** (spec `doc/specs/dtc-module-response.md`). ObdMetrics reports + `DtcModuleStatus` per scanned module via the three-argument `Lifecycle.onDTCCompleted` (the app overrides only that + one). Inside ObdMetrics, NO DATA / adapter errors never reach the DTC handler (`ObdCommandHandler` drops them), so + "no reply" = `NO_RESPONSE`; a refusal (`7F 19`) is marked by the read codec returning `DtcReadReply` (a `List` with a + status). Wrong turn: classifying `reply.getRaw()` - raw is published only with `collectRawConnectorResponseEnabled`, + so every module read as `NO_RESPONSE`. App: stored in `pref.datalogger.dtc.module_status` and `DtcScan.moduleStatus`; + `DtcScan.covers` skips unanswered modules, so they never produce "no longer reported". * **One DTC action at a time** (`dtc/DtcActionQueue.kt`). ObdMetrics' `scheduleDTCAction` clears its command buffer, so an action scheduled while another runs cuts that one short, and both completions arrive. With per-action state in single fields, Clear pressed during the connect scan stored the scan's result as the clear's and the clear's read diff --git a/app/build.gradle b/app/build.gradle index 69cd2f79..e7181b99 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -81,7 +81,7 @@ android { resValue "string", "DEFAULT_PROFILE", "profile_8" resValue "string", "applicationId", "org.obd.graphs.my.giulia.aa" applicationId "org.obd.graphs.my.giulia.aa" - versionCode 253 + versionCode 254 } giuliaPerformanceMonitor { @@ -89,7 +89,7 @@ android { resValue "string", "DEFAULT_PROFILE", "profile_8" resValue "string", "applicationId", "org.obd.graphs.my.giulia.performance_monitor" applicationId "org.obd.graphs.my.giulia.performance_monitor" - versionCode 131 + versionCode 132 } giulia { @@ -97,7 +97,7 @@ android { resValue "string", "DEFAULT_PROFILE", "profile_3" resValue "string", "applicationId", "org.obd.graphs.my.giulia" applicationId "org.obd.graphs.my.giulia" - versionCode 93 + versionCode 94 } } diff --git a/app/src/main/java/org/obd/graphs/preferences/dtc/DiagnosticTroubleCodePreferenceDialogFragment.kt b/app/src/main/java/org/obd/graphs/preferences/dtc/DiagnosticTroubleCodePreferenceDialogFragment.kt index 0dd3ddfb..e774659d 100644 --- a/app/src/main/java/org/obd/graphs/preferences/dtc/DiagnosticTroubleCodePreferenceDialogFragment.kt +++ b/app/src/main/java/org/obd/graphs/preferences/dtc/DiagnosticTroubleCodePreferenceDialogFragment.kt @@ -56,6 +56,7 @@ import org.obd.graphs.bl.datalogger.dtc.statusLine import org.obd.graphs.bl.datalogger.dtc.summary import org.obd.graphs.bl.datalogger.dtc.toDidLines import org.obd.graphs.bl.datalogger.dtc.toDtcListItems +import org.obd.graphs.bl.datalogger.dtc.unansweredModules import org.obd.graphs.bl.datalogger.dtc.withGoneSection import org.obd.graphs.bl.datalogger.dtc.withModuleSummaries import org.obd.graphs.bl.readiness.PREF_READINESS_LAST @@ -384,6 +385,10 @@ internal class DiagnosticTroubleCodePreferenceDialogFragment : CoreDialogFragmen reportBuilder.append("$reportTitle\n") reportBuilder.append("-".repeat(reportTitle.length)).append("\n\n") + VehicleCapabilitiesManager.getDtcModuleStatus().unansweredModules().takeIf { it.isNotEmpty() }?.let { + reportBuilder.append(getString(R.string.dtc_share_modules_no_response, it.joinToString(", "))).append("\n\n") + } + val statusLabels = dtcStatusLabels(requireContext()) for (code in dtcList) { if (code.standardCode.isEmpty()) continue @@ -472,7 +477,10 @@ internal class DiagnosticTroubleCodePreferenceDialogFragment : CoreDialogFragmen scannedModules = VehicleCapabilitiesManager.getDtcScannedModules(), noCodesMessage = getString(R.string.pref_dtc_no_dtc_found), noCodesForModuleMessage = getString(R.string.pref_dtc_no_dtc_found_for_module), - unknownModuleLabel = getString(R.string.pref_dtc_unknown_module) + unknownModuleLabel = getString(R.string.pref_dtc_unknown_module), + moduleStatus = VehicleCapabilitiesManager.getDtcModuleStatus(), + noResponseMessage = getString(R.string.pref_dtc_module_no_response), + refusedMessage = getString(R.string.pref_dtc_module_refused) ).markedWith(history) .withModuleSummaries() .collapsedBy { moduleExpanded[it] } diff --git a/app/src/main/java/org/obd/graphs/preferences/dtc/DiagnosticTroubleCodeViewAdapter.kt b/app/src/main/java/org/obd/graphs/preferences/dtc/DiagnosticTroubleCodeViewAdapter.kt index fecc2f1e..e9b447bc 100644 --- a/app/src/main/java/org/obd/graphs/preferences/dtc/DiagnosticTroubleCodeViewAdapter.kt +++ b/app/src/main/java/org/obd/graphs/preferences/dtc/DiagnosticTroubleCodeViewAdapter.kt @@ -55,6 +55,7 @@ import org.obd.graphs.bl.datalogger.dtc.stateLabel import org.obd.graphs.bl.datalogger.dtc.toDidLines import org.obd.graphs.ui.common.COLOR_CARDINAL import org.obd.graphs.ui.common.setText +import org.obd.metrics.command.dtc.DtcModuleStatus private const val VIEW_TYPE_HEADER = 0 private const val VIEW_TYPE_DTC = 1 @@ -79,6 +80,11 @@ internal val STATE_BADGES = internal val MIL_BADGE = BadgeStyle(R.color.dtc_state_mil, R.drawable.dtc_state_mil, R.string.dtc_state_mil_explanation) +// A module header's chip when the module did not answer the scan / refused the read: its codes are unknown. +internal val NO_RESPONSE_BADGE = + BadgeStyle(R.color.dtc_state_history, R.drawable.dtc_module_no_response, R.string.dtc_module_no_response_explanation) +internal val REFUSED_BADGE = BadgeStyle(R.color.dtc_state_mil, R.drawable.dtc_module_refused, R.string.dtc_module_refused_explanation) + // The "no longer reported" section header's chip (check + count). internal val GONE_BADGE = BadgeStyle(R.color.philippine_green, R.drawable.dtc_history_gone, R.string.dtc_legend_gone) @@ -109,7 +115,9 @@ internal val DTC_LEGEND: List> = R.string.dtc_state_stored to STATE_BADGES.getValue(DtcState.STORED), R.string.dtc_state_pending to STATE_BADGES.getValue(DtcState.PENDING), R.string.dtc_state_history to STATE_BADGES.getValue(DtcState.HISTORY), - R.string.dtc_state_mil to MIL_BADGE + R.string.dtc_state_mil to MIL_BADGE, + R.string.dtc_module_no_response to NO_RESPONSE_BADGE, + R.string.dtc_module_refused to REFUSED_BADGE ) // The explanations start with their label ("Active: ..."), which the legend already shows as the chip. @@ -382,7 +390,15 @@ internal class DiagnosticTroubleCodeViewAdapter internal constructor( .takeIf { header.summary.newCount > 0 } ) chips.forEach { (label, style, description) -> summary.addMiniBadge(label, style, description) } - if (header.codeCount == 0) { + when (header.response) { + DtcModuleStatus.NO_RESPONSE -> + context.getString(R.string.dtc_module_no_response).let { summary.addMiniBadge(it, NO_RESPONSE_BADGE, it) } + DtcModuleStatus.REFUSED -> + context.getString(R.string.dtc_module_refused).let { summary.addMiniBadge(it, REFUSED_BADGE, it) } + DtcModuleStatus.RESPONDED -> Unit + } + // The check means "answered, no codes" - never for a module that did not answer. + if (header.codeCount == 0 && header.response == DtcModuleStatus.RESPONDED) { summary.addView( ImageView(context).apply { setImageResource(R.drawable.dtc_history_gone) @@ -435,10 +451,15 @@ internal class DiagnosticTroubleCodeViewAdapter internal constructor( internal class MessageViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) { private val text: TextView = itemView.findViewById(R.id.dtc_message) - // "No codes reported" - good news, in green. + // "No codes reported" - good news, in green; a module that did not answer, muted. fun bind(message: DtcListItem.Message) { text.text = message.text - text.setTextColor(ContextCompat.getColor(itemView.context, R.color.philippine_green)) + text.setTextColor( + ContextCompat.getColor( + itemView.context, + if (message.status == DtcModuleStatus.RESPONDED) R.color.philippine_green else R.color.dialog_text_secondary + ) + ) } } diff --git a/app/src/main/res/drawable/dtc_module_no_response.xml b/app/src/main/res/drawable/dtc_module_no_response.xml new file mode 100644 index 00000000..8af675c1 --- /dev/null +++ b/app/src/main/res/drawable/dtc_module_no_response.xml @@ -0,0 +1,11 @@ + + + + + diff --git a/app/src/main/res/drawable/dtc_module_refused.xml b/app/src/main/res/drawable/dtc_module_refused.xml new file mode 100644 index 00000000..c2f4a1f9 --- /dev/null +++ b/app/src/main/res/drawable/dtc_module_refused.xml @@ -0,0 +1,11 @@ + + + + + diff --git a/app/src/main/res/values-pl/strings.xml b/app/src/main/res/values-pl/strings.xml index b4cd59b4..b400387c 100644 --- a/app/src/main/res/values-pl/strings.xml +++ b/app/src/main/res/values-pl/strings.xml @@ -444,6 +444,8 @@ Pojazd odmówił skasowania kodów błędów. Wyłącz silnik, pozostaw włączony zapłon i spróbuj ponownie. Kody błędów zgłaszane po skasowaniu: %1$s. Usterka może nadal występować. Brak zgłoszonych kodów błędów. + Moduł nie odpowiedział. Jego kody są nieznane. + Moduł odmówił odczytu. Włącz automatyczne czyszczenie DTC Automatycznie kasuje kody błędów diagnostycznych (DTC) z pamięci ECU. Dotknij, aby wyświetlić aktywne kody błędów diagnostycznych (DTC) pobrane z ECU. @@ -783,6 +785,11 @@ Ukryj szczegóły Już nie zgłaszane (%1$d) Brak kodów usterek + Brak odpowiedzi + Brak odpowiedzi: moduł nie odpowiedział na odczyt. Jego kody są nieznane, więc nie wpływa na oznaczenia Nowy i Już nie zgłaszane. + Odmowa + Odmowa: moduł odpowiedział, ale nie przesłał kodów. + Moduły bez odpowiedzi: %1$s Już nie zgłaszane Ostatnio zgłoszony %1$s Stan: %1$s diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 867bb262..5363fe0f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -440,6 +440,8 @@ The vehicle refused to clear the trouble codes. Turn the engine off, leave the ignition on and try again. Trouble codes still reported after clearing: %1$s. The fault may still be present. No trouble codes reported. + No response from the module. Its codes are unknown. + The module refused the read. Enable Automatic DTC cleanup Automatically clears Diagnostic Trouble Codes (DTCs) from the ECU memory. Tap to view active Diagnostic Trouble Codes (DTCs) retrieved from the ECU. @@ -1008,6 +1010,11 @@ Hide details No longer reported (%1$d) No trouble codes + No response + No response: the module did not answer the scan. Its codes are unknown, so it is left out of New and No longer reported. + Refused + Refused: the module answered but declined to send its codes. + Modules without an answer: %1$s No longer reported Last reported %1$s State: %1$s diff --git a/app/src/test/java/org/obd/graphs/preferences/dtc/DtcLegendTest.kt b/app/src/test/java/org/obd/graphs/preferences/dtc/DtcLegendTest.kt index a3c73aad..04e51625 100644 --- a/app/src/test/java/org/obd/graphs/preferences/dtc/DtcLegendTest.kt +++ b/app/src/test/java/org/obd/graphs/preferences/dtc/DtcLegendTest.kt @@ -29,8 +29,10 @@ class DtcLegendTest { assertTrue(styles.containsAll(STATE_BADGES.values)) assertTrue(NEW_BADGE in styles) assertTrue(MIL_BADGE in styles) + assertTrue(NO_RESPONSE_BADGE in styles) + assertTrue(REFUSED_BADGE in styles) assertEquals(styles.size, styles.distinct().size) - assertEquals(DtcState.values().count { it in STATE_BADGES } + 2, styles.size) + assertEquals(DtcState.values().count { it in STATE_BADGES } + 4, styles.size) } @Test diff --git a/automotive/src/main/java/org/obd/graphs/aa/screen/nav/DiagnosticTroubleCodesScreen.kt b/automotive/src/main/java/org/obd/graphs/aa/screen/nav/DiagnosticTroubleCodesScreen.kt index 5867366b..7a200d54 100644 --- a/automotive/src/main/java/org/obd/graphs/aa/screen/nav/DiagnosticTroubleCodesScreen.kt +++ b/automotive/src/main/java/org/obd/graphs/aa/screen/nav/DiagnosticTroubleCodesScreen.kt @@ -86,6 +86,7 @@ import org.obd.graphs.registerReceiver import org.obd.graphs.renderer.api.Fps import org.obd.graphs.renderer.api.Identity import org.obd.metrics.api.model.DiagnosticTroubleCode +import org.obd.metrics.command.dtc.DtcModuleStatus internal enum class DtcScreenIdentity( private val code: Int @@ -223,7 +224,10 @@ internal class DiagnosticTroubleCodesScreen( scannedModules = VehicleCapabilitiesManager.getDtcScannedModules(), noCodesMessage = stringProvider.getString(R.string.dtc_no_codes), noCodesForModuleMessage = stringProvider.getString(R.string.dtc_no_codes_for_module), - unknownModuleLabel = stringProvider.getString(R.string.dtc_unknown_module) + unknownModuleLabel = stringProvider.getString(R.string.dtc_unknown_module), + moduleStatus = VehicleCapabilitiesManager.getDtcModuleStatus(), + noResponseMessage = stringProvider.getString(R.string.dtc_no_response_for_module), + refusedMessage = stringProvider.getString(R.string.dtc_refused_for_module) ).markedWith(DtcHistoryStore.diff(VehicleCapabilitiesManager.getDtcReadInfo())) .toDtcSections() @@ -308,7 +312,20 @@ internal class DiagnosticTroubleCodesScreen( items.forEach { item -> when (item) { is DtcListItem.DtcRow -> addItem(row(item.dtc, item.isNew)) - is DtcListItem.Message -> addItem(Row.Builder().setTitle(item.text).setImage(icon(R.drawable.dtc_ok, CarColor.GREEN)).build()) + // A module that did not answer is not good news: plain fault icon, no green. + is DtcListItem.Message -> + addItem( + Row + .Builder() + .setTitle(item.text) + .setImage( + if (item.status == DtcModuleStatus.RESPONDED) { + icon(R.drawable.dtc_ok, CarColor.GREEN) + } else { + icon(R.drawable.dtc_fault, CarColor.DEFAULT) + } + ).build() + ) is DtcListItem.ModuleHeader, is DtcListItem.GoneHeader -> Unit } } diff --git a/automotive/src/main/res/values-pl/strings.xml b/automotive/src/main/res/values-pl/strings.xml index 606905ee..f5c274c8 100644 --- a/automotive/src/main/res/values-pl/strings.xml +++ b/automotive/src/main/res/values-pl/strings.xml @@ -20,6 +20,8 @@ Nie znaleziono kodów błędów diagnostycznych (DTC). Nieznany moduł Brak zgłoszonych kodów błędów. + Moduł nie odpowiedział. + Moduł odmówił odczytu. Aktywny Zapisany Oczekujący diff --git a/automotive/src/main/res/values/strings.xml b/automotive/src/main/res/values/strings.xml index 63cbe7b9..d5541b74 100644 --- a/automotive/src/main/res/values/strings.xml +++ b/automotive/src/main/res/values/strings.xml @@ -19,6 +19,8 @@ No diagnostic trouble codes found. Unknown module No codes reported. + No response from the module. + The module refused the read. Active Stored Pending diff --git a/datalogger/src/main/java/org/obd/graphs/bl/datalogger/VehicleCapabilitiesManager.kt b/datalogger/src/main/java/org/obd/graphs/bl/datalogger/VehicleCapabilitiesManager.kt index ec2efdce..11908b62 100644 --- a/datalogger/src/main/java/org/obd/graphs/bl/datalogger/VehicleCapabilitiesManager.kt +++ b/datalogger/src/main/java/org/obd/graphs/bl/datalogger/VehicleCapabilitiesManager.kt @@ -32,6 +32,7 @@ import org.obd.graphs.preferences.Prefs import org.obd.graphs.sendBroadcastEvent import org.obd.metrics.api.model.DiagnosticTroubleCode import org.obd.metrics.api.model.VehicleCapabilities +import org.obd.metrics.command.dtc.DtcModuleStatus import org.obd.metrics.pid.PIDsGroup class VehicleMetadata( @@ -44,6 +45,7 @@ private const val PREF_VEHICLE_METADATA = "pref.datalogger.vehicle.properties" private const val PREF_DTC = "pref.datalogger.dtc" private const val PREF_DTC_SCANNED_MODULES = "pref.datalogger.dtc.scanned_modules" private const val PREF_DTC_READ_INFO = "pref.datalogger.dtc.read_info" +private const val PREF_DTC_MODULE_STATUS = "pref.datalogger.dtc.module_status" object VehicleCapabilitiesManager { private val mapper = @@ -107,7 +109,8 @@ object VehicleCapabilitiesManager { dtc: Set, scannedModules: List = emptyList(), now: Long = System.currentTimeMillis(), - kind: DtcScanKind = DtcScanKind.READ + kind: DtcScanKind = DtcScanKind.READ, + moduleStatus: Map = emptyMap() ) { val readInfo = DtcReadInfo(timestamp = now, vin = getCurrentVin()) Prefs.edit().apply { @@ -118,9 +121,10 @@ object VehicleCapabilitiesManager { putString(PREF_DTC, mapper.writeValueAsString(dtc)) putString(PREF_DTC_SCANNED_MODULES, mapper.writeValueAsString(scannedModules)) putString(PREF_DTC_READ_INFO, mapper.writeValueAsString(readInfo)) + putString(PREF_DTC_MODULE_STATUS, mapper.writeValueAsString(moduleStatus)) commit() } - DtcHistoryStore.append(readInfo.vin, DtcScan(now, kind, scannedModules, dtc.toList())) + DtcHistoryStore.append(readInfo.vin, DtcScan(now, kind, scannedModules, dtc.toList(), moduleStatus)) } // Null for codes stored before the read info existed: their age is unknown, not "now". @@ -153,6 +157,16 @@ object VehicleCapabilitiesManager { emptySet() } + // How each scanned module answered the stored read; empty for lists stored before it was recorded (= all answered). + fun getDtcModuleStatus(): Map = + try { + val preferences = Prefs.getString(PREF_DTC_MODULE_STATUS, "")!! + if (preferences.isEmpty()) emptyMap() else mapper.readValue>(preferences) + } catch (e: Throwable) { + Log.e(LOG_TAG, "Failed to read DTC module status from preferences", e) + emptyMap() + } + fun getDtcScannedModules(): List = try { var preferences = Prefs.getString(PREF_DTC_SCANNED_MODULES, "")!! diff --git a/datalogger/src/main/java/org/obd/graphs/bl/datalogger/WorkflowOrchestrator.kt b/datalogger/src/main/java/org/obd/graphs/bl/datalogger/WorkflowOrchestrator.kt index 4c24b660..51166d87 100644 --- a/datalogger/src/main/java/org/obd/graphs/bl/datalogger/WorkflowOrchestrator.kt +++ b/datalogger/src/main/java/org/obd/graphs/bl/datalogger/WorkflowOrchestrator.kt @@ -44,6 +44,7 @@ import org.obd.metrics.api.model.* import org.obd.metrics.codec.formula.FormulaEvaluatorConfig import org.obd.metrics.command.Command import org.obd.metrics.command.dtc.DiagnosticTroubleCodeClearStatus +import org.obd.metrics.command.dtc.DtcModuleStatus import org.obd.metrics.command.group.CommandGroup import org.obd.metrics.command.group.DefaultCommandGroup import org.obd.metrics.command.routine.RoutineCommand @@ -97,9 +98,12 @@ internal class WorkflowOrchestrator internal constructor() { sendBroadcastEvent(DATA_LOGGER_CONNECTING_EVENT) } + // moduleStatus: how each scanned module answered (ObdMetrics); a module that did not answer is shown as such + // instead of as clean, and is left out of the history comparison. override fun onDTCCompleted( dtc: Set, - status: DiagnosticTroubleCodeClearStatus? + status: DiagnosticTroubleCodeClearStatus?, + moduleStatus: Map? ) { if (Log.isLoggable(LOG_TAG, Log.DEBUG)) { Log.d(LOG_TAG, "Received onDTCCompleted event.") @@ -109,8 +113,8 @@ internal class WorkflowOrchestrator internal constructor() { // for one nobody asked for, stored as a plain read like before. val (finished, next) = dtcActions.complete() val kind = finished?.kind ?: DtcScanKind.READ - Log.i(LOG_TAG, "DTC action completed: $kind, codes=${dtc.size}, next=${next?.kind}") - VehicleCapabilitiesManager.updateDTC(dtc, finished?.moduleLabels.orEmpty(), kind = kind) + Log.i(LOG_TAG, "DTC action completed: $kind, codes=${dtc.size}, next=${next?.kind}, modules=$moduleStatus") + VehicleCapabilitiesManager.updateDTC(dtc, finished?.moduleLabels.orEmpty(), kind = kind, moduleStatus = moduleStatus.orEmpty()) if (kind == DtcScanKind.CONNECT && dtc.isNotEmpty()) sendBroadcastEvent(DATA_LOGGER_DTC_AVAILABLE) diff --git a/datalogger/src/main/java/org/obd/graphs/bl/datalogger/dtc/DtcHistory.kt b/datalogger/src/main/java/org/obd/graphs/bl/datalogger/dtc/DtcHistory.kt index d9386943..c2d66fe7 100644 --- a/datalogger/src/main/java/org/obd/graphs/bl/datalogger/dtc/DtcHistory.kt +++ b/datalogger/src/main/java/org/obd/graphs/bl/datalogger/dtc/DtcHistory.kt @@ -17,6 +17,7 @@ package org.obd.graphs.bl.datalogger.dtc import org.obd.metrics.api.model.DiagnosticTroubleCode +import org.obd.metrics.command.dtc.DtcModuleStatus // Per-vehicle DTC history: every stored DTC list is also kept as a scan record, keyed by VIN. // Pure logic here; the file lives in DtcHistoryStore. Spec: doc/specs/dtc-history.md. @@ -39,12 +40,20 @@ enum class DtcScanKind { } // scannedModules: module labels the scan targeted (DiagnosticTroubleCode.module); empty = default single-ECU read. +// moduleStatus: how each targeted module answered; empty for scans recorded before it existed (= all answered). data class DtcScan( val timestamp: Long = 0, val kind: DtcScanKind = DtcScanKind.READ, val scannedModules: List = emptyList(), - val codes: List = emptyList() -) + val codes: List = emptyList(), + val moduleStatus: Map = emptyMap() +) { + // A module that did not answer (or refused) was targeted but not looked at. + fun answered(module: String): Boolean = + moduleStatus.entries.firstOrNull { it.key.equals(module, ignoreCase = true) }?.value.let { + it == null || it == DtcModuleStatus.RESPONDED + } +} fun historyKey(vin: String?): String = vin?.uppercase() ?: UNKNOWN_VEHICLE @@ -75,7 +84,8 @@ fun Map>.append( .associate { it.key to it.value } } -// Whether this scan looked at the module the code comes from. A module scan covers the modules it targeted; +// Whether this scan looked at the module the code comes from. A module scan covers the modules it targeted and +// that answered (a silent module's codes are unknown, not gone); // a code with no module of its own (the default ECU, or untagged) is only comparable with scans of the same // targets as the scan that reported it - a default single-ECU read is compared only with other default reads. internal fun DtcScan.covers( @@ -84,8 +94,8 @@ internal fun DtcScan.covers( ): Boolean { val module = code.module?.takeUnless { it.isBlank() || it.equals(DTC_DEFAULT_MODULE, ignoreCase = true) } return if (module == null) { - scannedModules.toSet() == reportedBy.scannedModules.toSet() + scannedModules.toSet() == reportedBy.scannedModules.toSet() && scannedModules.all { answered(it) } } else { - scannedModules.any { it.equals(module, ignoreCase = true) } + scannedModules.any { it.equals(module, ignoreCase = true) } && answered(module) } } diff --git a/datalogger/src/main/java/org/obd/graphs/bl/datalogger/dtc/DtcListItem.kt b/datalogger/src/main/java/org/obd/graphs/bl/datalogger/dtc/DtcListItem.kt index 226fca24..a6d00df9 100644 --- a/datalogger/src/main/java/org/obd/graphs/bl/datalogger/dtc/DtcListItem.kt +++ b/datalogger/src/main/java/org/obd/graphs/bl/datalogger/dtc/DtcListItem.kt @@ -19,6 +19,7 @@ package org.obd.graphs.bl.datalogger.dtc import org.obd.graphs.DiagnosticMappingItem import org.obd.metrics.api.model.DiagnosticTroubleCode import org.obd.metrics.api.model.SnapshotPID +import org.obd.metrics.command.dtc.DtcModuleStatus // Shared by the phone DTC dialog and the AA DTC screen - keep one copy of the grouping/ordering. @@ -30,11 +31,13 @@ const val PREF_DTC_DESELECTED_MODULES = "pref.dtc.module_picker.deselected" sealed class DtcListItem { // codeCount: the module's code rows, 0 for a module scanned clean. summary / expanded come from // withModuleSummaries / collapsedBy. None of them is part of the row identity. + // response: how the module answered the scan (RESPONDED for lists stored before it was recorded). data class ModuleHeader( val module: String, val codeCount: Int = 0, val summary: DtcModuleSummary = DtcModuleSummary(), - val expanded: Boolean = true + val expanded: Boolean = true, + val response: DtcModuleStatus = DtcModuleStatus.RESPONDED ) : DtcListItem() // isNew / isGone / history come from the vehicle's DTC history (DtcHistoryDiff); none of them is part @@ -52,7 +55,12 @@ sealed class DtcListItem { // A plain informational line (no code) - the "no DTC found" empty state, or a scanned module // that reported nothing. Keyed so DiffUtil can tell per-module messages apart. - data class Message(val key: String, val text: String) : DtcListItem() + // status: RESPONDED for good news ("no codes"), otherwise why the module has no list. + data class Message( + val key: String, + val text: String, + val status: DtcModuleStatus = DtcModuleStatus.RESPONDED + ) : DtcListItem() // Every subclass is a data class; declared here so DiffUtil's areContentsTheSame (and lint's // DiffUtilEquals check, which cannot see subclasses across modules) rely on value equality. @@ -182,8 +190,14 @@ fun List.toDtcListItems( scannedModules: List, noCodesMessage: String, noCodesForModuleMessage: String, - unknownModuleLabel: String + unknownModuleLabel: String, + moduleStatus: Map = emptyMap(), + noResponseMessage: String = noCodesForModuleMessage, + refusedMessage: String = noCodesForModuleMessage ): List { + // A module missing from moduleStatus answered (lists stored before statuses were recorded). + fun responseOf(label: String) = moduleStatus[label] ?: DtcModuleStatus.RESPONDED + val realModules = (mapNotNull { it.module } + scannedModules) .filter { it.isNotBlank() && !it.equals(DTC_DEFAULT_MODULE, ignoreCase = true) } @@ -219,17 +233,33 @@ fun List.toDtcListItems( val codesByModule = groupBy { it.groupLabel() } .mapValues { (_, codes) -> codes.distinctBy { it.standardCode to it.failureType?.code } } - // Modules reporting codes first - they are what the list is opened for - then the clean ones, each by name. + // Modules reporting codes first - they are what the list is opened for - then the ones that did not answer + // (their codes are unknown), then the clean ones, each by name. val labels = (codesByModule.keys + scannedModules.filter { it.isNotBlank() }) .distinct() - .sortedWith(compareBy { if (codesByModule[it].isNullOrEmpty()) 1 else 0 }.thenBy { it }) + .sortedWith( + compareBy { + when { + !codesByModule[it].isNullOrEmpty() -> 0 + responseOf(it) != DtcModuleStatus.RESPONDED -> 1 + else -> 2 + } + }.thenBy { it } + ) return labels.flatMap { label -> val codes = codesByModule[label].orEmpty() - listOf(DtcListItem.ModuleHeader(label, codes.size)) + + val response = responseOf(label) + listOf(DtcListItem.ModuleHeader(label, codes.size, response = response)) + if (codes.isEmpty()) { - listOf(DtcListItem.Message(key = label, text = noCodesForModuleMessage)) + val text = + when (response) { + DtcModuleStatus.RESPONDED -> noCodesForModuleMessage + DtcModuleStatus.REFUSED -> refusedMessage + DtcModuleStatus.NO_RESPONSE -> noResponseMessage + } + listOf(DtcListItem.Message(key = label, text = text, status = response)) } else { codes.map { DtcListItem.DtcRow(it) } } @@ -356,3 +386,7 @@ fun List?.toDidLines( val units = did.definition?.units?.trim().orEmpty() listOf("$description:", value, units).filter { it.isNotEmpty() }.joinToString(" ") } + +// Modules of the stored read that did not answer it (no response or refused), in scan order: for the shared report. +fun Map.unansweredModules(): List = + filterValues { it != DtcModuleStatus.RESPONDED }.keys.toList() diff --git a/datalogger/src/test/java/org/obd/graphs/bl/datalogger/VehicleCapabilitiesManagerTest.kt b/datalogger/src/test/java/org/obd/graphs/bl/datalogger/VehicleCapabilitiesManagerTest.kt index 2273f88a..6cf97527 100644 --- a/datalogger/src/test/java/org/obd/graphs/bl/datalogger/VehicleCapabilitiesManagerTest.kt +++ b/datalogger/src/test/java/org/obd/graphs/bl/datalogger/VehicleCapabilitiesManagerTest.kt @@ -40,6 +40,7 @@ import org.obd.metrics.api.model.DiagnosticTroubleCode import org.obd.metrics.api.model.SnapshotPID import org.obd.metrics.api.model.VehicleCapabilities import org.obd.metrics.command.dtc.DiagnosticTroubleCodeClearStatus +import org.obd.metrics.command.dtc.DtcModuleStatus import org.obd.metrics.pid.PidDefinition import org.obd.metrics.pid.PidDefinitionRegistry import org.robolectric.RobolectricTestRunner @@ -298,4 +299,22 @@ class VehicleCapabilitiesManagerTest : TestSetup() { assertEquals(DtcScanKind.CONNECT, DtcHistoryStore.scans(null).single().kind) assertEquals(DtcHistoryStore.scans(null), DtcHistoryStore.scans(UNKNOWN_VEHICLE)) } + + @Test + fun `updateDTC stores how each module answered, in the pref and in the history scan`() { + val slot = slot() + every { editor.putString(eq("pref.datalogger.dtc.module_status"), capture(slot)) } returns editor + val status = mapOf("Engine ECU" to DtcModuleStatus.RESPONDED, "ABS" to DtcModuleStatus.NO_RESPONSE) + + VehicleCapabilitiesManager.updateDTC(emptySet(), listOf("Engine ECU", "ABS"), now = 3, moduleStatus = status) + + every { sharedPrefs.getString(eq("pref.datalogger.dtc.module_status"), any()) } returns slot.captured + assertEquals(status, VehicleCapabilitiesManager.getDtcModuleStatus()) + assertEquals(status, DtcHistoryStore.scans(null).single().moduleStatus) + } + + @Test + fun `a list stored before module statuses were recorded has none`() { + assertTrue(VehicleCapabilitiesManager.getDtcModuleStatus().isEmpty()) + } } diff --git a/datalogger/src/test/java/org/obd/graphs/bl/datalogger/dtc/DtcHistoryDiffTest.kt b/datalogger/src/test/java/org/obd/graphs/bl/datalogger/dtc/DtcHistoryDiffTest.kt index 96f3f5fe..b01047d9 100644 --- a/datalogger/src/test/java/org/obd/graphs/bl/datalogger/dtc/DtcHistoryDiffTest.kt +++ b/datalogger/src/test/java/org/obd/graphs/bl/datalogger/dtc/DtcHistoryDiffTest.kt @@ -22,6 +22,7 @@ import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test import org.obd.metrics.api.model.DiagnosticTroubleCode +import org.obd.metrics.command.dtc.DtcModuleStatus class DtcHistoryDiffTest { private fun dtc( @@ -187,4 +188,35 @@ class DtcHistoryDiffTest { assertEquals(items, items.withGoneSection(DtcHistoryDiff.EMPTY, expanded = true)) } + + @Test + fun `a module that did not answer neither loses its codes nor counts as covered`() { + val abs = dtc("C0035", module = "ABS") + val diff = + dtcHistoryDiff( + listOf( + scan(1, abs, modules = listOf("Engine", "ABS")), + DtcScan(2, DtcScanKind.READ, listOf("Engine", "ABS"), emptyList(), mapOf("ABS" to DtcModuleStatus.NO_RESPONSE)), + DtcScan(3, DtcScanKind.READ, listOf("Engine", "ABS"), emptyList(), mapOf("ABS" to DtcModuleStatus.REFUSED)) + ) + ) + + assertTrue(diff.gone.isEmpty()) + assertEquals(DtcCodeHistory(firstSeen = 1, lastSeen = 1, seenIn = 1, coveredIn = 1), diff.history(abs)) + } + + @Test + fun `a module that answered again is compared again`() { + val abs = dtc("C0035", module = "ABS") + val diff = + dtcHistoryDiff( + listOf( + scan(1, abs, modules = listOf("ABS")), + DtcScan(2, DtcScanKind.READ, listOf("ABS"), emptyList(), mapOf("ABS" to DtcModuleStatus.NO_RESPONSE)), + DtcScan(3, DtcScanKind.READ, listOf("ABS"), emptyList(), mapOf("ABS" to DtcModuleStatus.RESPONDED)) + ) + ) + + assertEquals(listOf("C0035"), diff.gone.map { it.standardCode }) + } } diff --git a/datalogger/src/test/java/org/obd/graphs/bl/datalogger/dtc/DtcListItemTest.kt b/datalogger/src/test/java/org/obd/graphs/bl/datalogger/dtc/DtcListItemTest.kt index 74a8b65f..b72e4537 100644 --- a/datalogger/src/test/java/org/obd/graphs/bl/datalogger/dtc/DtcListItemTest.kt +++ b/datalogger/src/test/java/org/obd/graphs/bl/datalogger/dtc/DtcListItemTest.kt @@ -25,6 +25,7 @@ import org.obd.graphs.DiagnosticMappingItem import org.obd.metrics.api.model.DiagnosticTroubleCode import org.obd.metrics.api.model.SnapshotPID import org.obd.metrics.command.dtc.DtcComponent +import org.obd.metrics.command.dtc.DtcModuleStatus import org.obd.metrics.pid.PidDefinition class DtcListItemTest { @@ -307,4 +308,58 @@ class DtcListItemTest { assertEquals(listOf("header:Engine", "gone-header", "dtc:Engine:P0500"), items.map { it.itemKey() }) } + + @Test + fun `a module that did not answer is not shown as clean and sorts between codes and clean modules`() { + val items = + listOf(withStatus("P0100", "Engine", 0x09)) + .toDtcListItems( + scannedModules = listOf("Engine", "ABS", "BCM", "Airbag"), + noCodesMessage = "none", + noCodesForModuleMessage = "no codes", + unknownModuleLabel = "unknown", + moduleStatus = + mapOf( + "Engine" to DtcModuleStatus.RESPONDED, + "ABS" to DtcModuleStatus.NO_RESPONSE, + "BCM" to DtcModuleStatus.RESPONDED, + "Airbag" to DtcModuleStatus.REFUSED + ), + noResponseMessage = "no response", + refusedMessage = "refused" + ) + + val headers = items.filterIsInstance() + assertEquals(listOf("Engine", "ABS", "Airbag", "BCM"), headers.map { it.module }) + assertEquals( + listOf(DtcModuleStatus.RESPONDED, DtcModuleStatus.NO_RESPONSE, DtcModuleStatus.REFUSED, DtcModuleStatus.RESPONDED), + headers.map { it.response } + ) + val messages = items.filterIsInstance().associate { it.key to (it.text to it.status) } + assertEquals("no response" to DtcModuleStatus.NO_RESPONSE, messages["ABS"]) + assertEquals("refused" to DtcModuleStatus.REFUSED, messages["Airbag"]) + assertEquals("no codes" to DtcModuleStatus.RESPONDED, messages["BCM"]) + // Silent modules have no codes, so they start collapsed like clean ones. + assertEquals(listOf("header:Engine", "dtc:Engine:P0100", "header:ABS", "header:Airbag", "header:BCM"), items.collapsedBy { null }.map { it.itemKey() }) + } + + @Test + fun `without recorded statuses every module counts as answered`() { + val items = emptyList().toDtcListItems(listOf("ABS"), "none", "no codes", "unknown") + + assertEquals(DtcModuleStatus.RESPONDED, items.filterIsInstance().single().response) + assertEquals("no codes", items.filterIsInstance().single().text) + } + + @Test + fun `unanswered modules keep the scan order`() { + val status = + linkedMapOf( + "Engine" to DtcModuleStatus.RESPONDED, + "BCM" to DtcModuleStatus.NO_RESPONSE, + "ABS" to DtcModuleStatus.REFUSED + ) + + assertEquals(listOf("BCM", "ABS"), status.unansweredModules()) + } } diff --git a/doc/specs/dtc-module-response.md b/doc/specs/dtc-module-response.md new file mode 100644 index 00000000..706c82b7 --- /dev/null +++ b/doc/specs/dtc-module-response.md @@ -0,0 +1,101 @@ +# Spec: modules that do not answer a DTC scan + +2026-10-14 · branch `feat/dtc-module-response` (ObdGraphs + ObdMetrics) · Status: implemented, on-device checks pending + +Roadmap "To investigate: modules that do not answer" (`dtc-roadmap.md`). + +## Summary + +**Problem.** +* A scanned module that does not answer (asleep, wrong header, timeout) looks the same as a clean one: header with a + green check, "No trouble codes reported". The driver is reassured about a module nobody looked at. +* Since DTC history (`dtc-history.md`), it also produces false markers: every code the module had last time is listed + under "No longer reported", and its codes come back as not new once it answers again. + +**Why ObdMetrics.** The app gets only the merged code set. Inside ObdMetrics a module that sends `NO DATA` (or another +adapter error) produces no reply at all - `ObdCommandHandler` drops it - while `59 02 FF` (no codes) and `7F 19 xx` +(refused) both decode to an empty list. Only the DTC handler sees which module answered and how. + +**Scope.** ObdMetrics (`DiagnosticTroubleCodeHandler`, `Lifecycle`), `:datalogger` (storage, history coverage, list +logic), `:app` (header chip, message, legend, share report), `:automotive` (message row), EN + PL. + +## Behaviour changes + +| Area | Before | After | +|---|---|---| +| Module that did not answer, phone header | Green check, collapsed like a clean module | Grey "no response" chip; collapsed | +| Its expanded line | "No trouble codes reported" (green) | "No response from the module. Codes unknown." (grey) | +| Module that refused the read (`7F 19 xx`) | Same as clean | "Read refused by the module" chip + line | +| Module order | Codes, then clean | Codes, then not answered / refused, then clean | +| History | Its previous codes listed as "no longer reported" | Not answered = not scanned: no gone / new markers from it | +| AA | Green check row "No trouble codes reported" | Row "No response from the module" with a grey icon | +| Shared report | — | "Modules without response: ..." line | +| Legend | — | Line for the "no response" chip | +| Lists stored before this change | — | No statuses stored: every module counts as answered, as before | + +## Settings / keys touched + +| Key | Type | Notes | +|---|---|---| +| `pref.datalogger.dtc.module_status` | JSON `{module label → RESPONDED / REFUSED / NO_RESPONSE}` | New, written with the list in `updateDTC`; missing = all answered | +| `files/dtc_history.json` | `DtcScan.moduleStatus` | New field, default empty (older scans unchanged) | + +## Implementation + +### ObdMetrics + +* `DtcModuleStatus` enum (`command.dtc`): `RESPONDED`, `REFUSED`, `NO_RESPONSE`. +* `DiagnosticTroubleCodeReadCodec`: a negative response to service 19 (`7F 19`, NRC 78 "pending" skipped, + `DtcModuleStatus.of`) decodes to `DtcReadReply(REFUSED)`, an empty `List` carrying the status, instead of a plain + empty list. Every consumer still sees a list. +* `DiagnosticTroubleCodeHandler.onNext`: any decoded `DTC_READ` reply marks its module ("" untagged) as answered - + `RESPONDED`, or the `DtcReadReply` status. Reset with the other per-action state at `DiagnosticTroubleCodeStartCommand`. + Wrong turn: classifying `reply.getRaw()`. The decoder publishes the raw response only with + `collectRawConnectorResponseEnabled`, so every module read as `NO_RESPONSE`. +* At the schedule command, every targeted module gets its status; a module without a reply is `NO_RESPONSE`. +* `Lifecycle.onDTCCompleted(dtc, status, moduleStatus)`: new default method calling the two-argument one, so existing + implementers keep working; the handler and `Lifecycle.Subscription` call the three-argument one. A default-ECU read + (no modules) passes an empty map. + +### App + +* `WorkflowOrchestrator` overrides the three-argument callback and passes the map to `updateDTC`, which stores it + (`pref.datalogger.dtc.module_status`) and puts it into the `DtcScan`. +* `DtcScan.covers`: a module whose status is not `RESPONDED` is not covered, so the history never compares it. +* `toDtcListItems(…, moduleStatus)`: header and message per status; `ModuleHeader.response` (not in `itemKey()`). + `collapsedBy` keeps such modules collapsed by default (they have no codes). +* Phone: header chip `NO_RESPONSE_BADGE` / `REFUSED_BADGE` (grey / amber, "?" / "block" icons) in place of the + check; legend row; the share report lists them. AA: message row with a grey icon. + +## Backward compatibility + +* ObdMetrics: the new callback is a default method; the old one is still called for implementers that only override it. +* App: a list or scan without statuses counts every module as answered (old behaviour). An app built against an older + ObdMetrics is not possible: the override needs the new method (same version, `11.26.1-SNAPSHOT`). + +## Tests + +* ObdMetrics `DtcModuleStatusTest` (done): reply classification (positive, multi-frame, `7F 19 xx`, NRC 78 ahead of the + answer, empty). +* ObdMetrics `DiagnosticTroubleCodeReadingTest.scheduleDtcReadReportsHowEachModuleAnswered` (done): two modules answering + `59 02 FF` / `7F 19 11` / nothing → `RESPONDED` / `REFUSED` / `NO_RESPONSE` per module (the mock replies by request, + so one status per case). The existing tests use the two-argument callback and still pass. +* `DtcHistoryDiffTest` (done): a silent / refusing module produces no gone codes and is not counted as covered; once it + answers again it is compared again. +* `DtcListItemTest` (done): header / message per status, order (codes, silent, clean), silent modules start collapsed, + statuses missing = answered, unanswered modules in scan order. +* `VehicleCapabilitiesManagerTest` (done): statuses round trip through the pref and into the history scan. +* `DtcLegendTest` (done): the legend covers the two new chips. +* [ ] Car: unplug / skip a module (e.g. a header no ECU answers), scan: grey "no response", its codes not "gone". +* [ ] PL strings at phone width. + +## Risks + +* A module that answers slower than the adapter timeout reads as `NO_RESPONSE` while it may have codes. That is the + truth as far as the scan knows, and now visible instead of looking clean. +* `REFUSED` relies on the `7F 19` prefix; another negative-reply shape would count as answered with no codes, as today. + +## Out of scope + +* "Retry this module": a single-module read replaces the whole stored list today; it needs a merge of partial scans. +* Retrying silently inside ObdMetrics. diff --git a/doc/specs/dtc-roadmap.md b/doc/specs/dtc-roadmap.md index 27bda89f..cd6e8eea 100644 --- a/doc/specs/dtc-roadmap.md +++ b/doc/specs/dtc-roadmap.md @@ -17,6 +17,7 @@ its own `doc/specs/.md` and this list links to it. | Show what state each code is in (initial #1): chips for active / stored / pending / history + MIL, sort by state | `dtc-status-badges.md` | | Read/clear overlay no longer times out during a long scan | `dtc-status-badges.md` (fix in the same branch) | | Readiness monitors (initial #3): phone screen, clear warning, DTC list row | `readiness-monitors.md` | +| Modules that do not answer: "no response" / "refused" instead of clean, left out of history markers | `dtc-module-response.md` | | History per vehicle (initial #4) with VIN on every connect (added later #8): new / gone markers | `dtc-history.md` | ## Planned @@ -107,8 +108,7 @@ made while building the state chips: the overlay timeout, and the VIN missing fr * **Legacy header scanned with UDS:** the scan includes "OBD-II (Legacy Mode 01)" (header `DB33F1`) and sends it the UDS `19 02` read. A legacy/functional header may need Mode 03 instead, or should be skipped by the UDS scan. -* **Modules that do not answer:** they look the same as clean ones ("No trouble codes reported"). Other tools show - "no response" separately. +* ~~**Modules that do not answer**~~: done, `dtc-module-response.md`. ## Out of scope