Skip to content

Commit 010b609

Browse files
malysclaude
andcommitted
fix(editor,diagnostic): readable condition values, one entry per action
The operator and the value of a condition were the smallest text on the screen that carries what the condition means. Both come from the same spinner row layout, so raising it to title size fixes the closed drop-down and its open list at once. Two catalogue entries are duplicates of an entry that is offered: PLAY_RADIO, which "Tune and play radio" expands into, and CALL_CONTACT, the deprecated alias kept so rules saved by older releases still load. The picker hid only the first, the Diagnostic tab hid neither, so both screens listed two radio actions and two call actions with the same verdict. CatalogVisibility now holds the set both consult; the engine and the exported debug report keep every entry, since an imported rule can still carry either one. The Diagnostic summary counts blocked actions from the rows actually listed, so it cannot disagree with the list over an entry not on screen. The stored support record survives an upgrade, and an upgrade can delete catalogue entries — the four STAR_*_PRESS conditions became one PHYSICAL_BUTTON. Until the next check ran, the record still named the removed ones and the Diagnostic tab counted them as supported. Names the current catalogue does not define are dropped on read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 9ed9bc6 commit 010b609

7 files changed

Lines changed: 121 additions & 10 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,20 @@ All notable changes to this project are documented here. Format follows
3535

3636
### Changed
3737

38+
- **The operator and the value of a condition are read at title size** (26sp) rather than
39+
body. "is equal to" and the value next to it are the two words that decide what a
40+
condition means, and they were the smallest text on the screen carrying them — in the
41+
closed drop-down and in its open list alike.
42+
- **The picker and the Diagnostic tab no longer list an entry twice.** "Play the radio" is
43+
not a choice — "Tune and play radio" expands into it — and "Call a contact" is the
44+
deprecated alias kept for old saved rules. Both showed next to the entry they duplicate,
45+
giving two radio actions and two call actions. They are hidden from the two screens the
46+
driver reads, not from the engine or the exported debug report, where an imported rule can
47+
still carry either one.
48+
- **The stored support record drops entries this build no longer defines.** It survives an
49+
upgrade, and an upgrade can delete catalogue entries — the four `STAR_*_PRESS` conditions
50+
became one `PHYSICAL_BUTTON`. Until the next check ran, the Diagnostic tab counted the
51+
removed ones as supported.
3852
- **Crash reporting and paste upload now come from MG4Hardware** (`com.mg4.hardware.diag`)
3953
instead of being carried here. The crash report gains the cause chain and a truncation
4054
that counts bytes rather than characters, so a report full of accented log lines no longer

‎app/src/main/java/com/mg4/tasker/store/SupportStore.kt‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.mg4.tasker.store
22

33
import android.content.Context
4+
import com.mg4.hardware.catalog.ActionType
5+
import com.mg4.hardware.catalog.ConditionType
46
import com.mg4.tasker.BuildConfig
57

68
/**
@@ -41,12 +43,24 @@ object SupportStore {
4143

4244
/** Supported ConditionType names, or null when never checked (caller must not filter). */
4345
fun supportedConditions(context: Context): Set<String>? =
44-
prefs(context).takeIf { it.contains(KEY_CONDITIONS) }
45-
?.getStringSet(KEY_CONDITIONS, emptySet())?.toSet()
46+
stored(context, KEY_CONDITIONS)?.let { known(it, ConditionType.entries.map { e -> e.name }) }
4647

4748
fun supportedActions(context: Context): Set<String>? =
48-
prefs(context).takeIf { it.contains(KEY_ACTIONS) }
49-
?.getStringSet(KEY_ACTIONS, emptySet())?.toSet()
49+
stored(context, KEY_ACTIONS)?.let { known(it, ActionType.entries.map { e -> e.name }) }
50+
51+
private fun stored(context: Context, key: String): Set<String>? =
52+
prefs(context).takeIf { it.contains(key) }?.getStringSet(key, emptySet())?.toSet()
53+
54+
/**
55+
* Drops names the current catalogue no longer defines.
56+
*
57+
* The record survives an update, and an update can remove entries — the four
58+
* `STAR_*_PRESS` conditions became one `PHYSICAL_BUTTON`. Until the next check runs,
59+
* the stored set still names the removed ones, which the Diagnostic screen counted as
60+
* supported conditions the app cannot offer any more.
61+
*/
62+
private fun known(stored: Set<String>, catalogue: List<String>): Set<String> =
63+
stored.intersect(catalogue.toSet())
5064

5165
/** Firmware and timestamp of the last check, for the Diagnostic screen; null when never run. */
5266
fun lastCheck(context: Context): Info? {

‎app/src/main/java/com/mg4/tasker/ui/CatalogPicker.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ object CatalogPicker {
7171
// Applying a profile is the one action that cannot work without MG4Control. Offering
7272
// it with an empty profile list only produces a rule that fails at ignition.
7373
val hasProfiles = ProfileBridge.isMG4ControlInstalled(context)
74-
val visible = ActionType.entries.filter { it != ActionType.PLAY_RADIO }
74+
val visible = ActionType.entries.filterNot { it in CatalogVisibility.hiddenActions }
7575
val grouped = visible.groupBy { type ->
7676
when (type) {
7777
ActionType.APPLY_PROFILE, ActionType.WEBHOOK_GET, ActionType.WEBHOOK_POST -> IntegrationGroup
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package com.mg4.tasker.ui
2+
3+
import com.mg4.hardware.catalog.ActionType
4+
5+
/**
6+
* Catalogue entries the user never picks, and therefore never has to read about.
7+
*
8+
* Both are duplicates of an entry that IS offered, with the same verdict and the same
9+
* vendor service behind them, so leaving them in produced two radio entries and two call
10+
* entries — in the picker, and again in the Diagnostic list.
11+
*
12+
* - [ActionType.PLAY_RADIO] is not a choice: "Tune and play radio" expands into it
13+
* ([ActionBundles]), so it always appears in a rule that needs it.
14+
* - `CALL_CONTACT` is the deprecated alias kept only so rules saved by older releases still
15+
* load; `CALL_NUMBER`'s editor already accepts a contact.
16+
*
17+
* Hidden from the two screens the driver reads, not from the engine or the exported debug
18+
* report: a rule imported from another car can still carry either one, and whoever debugs
19+
* it needs to see the verdict.
20+
*/
21+
internal object CatalogVisibility {
22+
23+
@Suppress("DEPRECATION")
24+
val hiddenActions: Set<ActionType> = setOf(ActionType.PLAY_RADIO, ActionType.CALL_CONTACT)
25+
}

‎app/src/main/java/com/mg4/tasker/ui/DiagnosticFragment.kt‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,19 @@ class DiagnosticFragment : Fragment() {
110110
report = fresh
111111
binding.exportButton.isEnabled = true
112112

113-
val blocked = fresh.blockedConditions + fresh.blockedActions
113+
// The report keeps every catalogue entry, the screen shows the ones the user can
114+
// pick: a duplicate the picker never offers ([CatalogVisibility]) would otherwise
115+
// appear here as a second radio and a second call entry.
116+
val actions = fresh.actions.filterNot {
117+
ActionType.valueOf(it.name) in CatalogVisibility.hiddenActions
118+
}
119+
// Counted from the rows actually listed, so the summary and the list cannot
120+
// disagree over an entry that is not on screen.
121+
val blockedActions = actions.count { !it.ok }
122+
val blocked = fresh.blockedConditions + blockedActions
114123
binding.summaryStatus.text =
115124
if (blocked == 0) getString(R.string.diag_summary_ok)
116-
else getString(R.string.diag_summary_blocked, fresh.blockedConditions, fresh.blockedActions)
125+
else getString(R.string.diag_summary_blocked, fresh.blockedConditions, blockedActions)
117126
binding.summaryStatus.setTextColor(color(blocked == 0))
118127

119128
binding.bridgeHint.visibility =
@@ -130,9 +139,9 @@ class DiagnosticFragment : Fragment() {
130139
}
131140

132141
add(Row.Header(getString(R.string.diag_section_actions)))
133-
fresh.actions.forEach { entry ->
142+
actions.forEach { entry ->
134143
val type = ActionType.valueOf(entry.name)
135-
add(Row.Item(getString(type.labelRes), actionValue(entry), entry.ok))
144+
add(Row.Item(actionLabel(type), actionValue(entry), entry.ok))
136145
}
137146
}
138147
binding.diagList.adapter = Adapter(rows)
@@ -266,6 +275,11 @@ class DiagnosticFragment : Fragment() {
266275
return if (label != null) "$label ($value)" else value.toString()
267276
}
268277

278+
/** Same wording as the picker: tuning a station also switches the source to the radio. */
279+
private fun actionLabel(type: ActionType): String =
280+
if (type == ActionType.TUNE_RADIO) getString(R.string.editor_tune_and_play_radio)
281+
else getString(type.labelRes)
282+
269283
private fun actionValue(entry: Diagnostics.Entry): String =
270284
if (entry.ok) getString(R.string.diag_action_ready) + hiddenSuffix(entry) else reason(entry)
271285

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
<?xml version="1.0" encoding="utf-8"?>
2+
<!-- Both the closed spinner and its dropdown rows. Title size rather than body: the
3+
operator ("is equal to") and the chosen value are the two words that decide what a
4+
condition means, and they were the smallest text on the screen that carries them. -->
25
<TextView xmlns:android="http://schemas.android.com/apk/res/android"
36
android:id="@android:id/text1"
47
android:layout_width="match_parent"
@@ -7,4 +10,4 @@
710
android:minHeight="@dimen/touch_target"
811
android:paddingHorizontal="@dimen/spacing_md"
912
android:textColor="@color/mg4_text_primary"
10-
android:textSize="@dimen/text_body" />
13+
android:textSize="@dimen/text_title" />
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package com.mg4.tasker.store
2+
3+
import androidx.test.core.app.ApplicationProvider
4+
import com.mg4.hardware.catalog.ConditionType
5+
import org.junit.Assert.assertEquals
6+
import org.junit.Assert.assertFalse
7+
import org.junit.Assert.assertNull
8+
import org.junit.Assert.assertTrue
9+
import org.junit.Test
10+
import org.junit.runner.RunWith
11+
import org.robolectric.RobolectricTestRunner
12+
import org.robolectric.annotation.Config
13+
14+
/**
15+
* The record survives app updates, and an update can delete catalogue entries — the four
16+
* `STAR_*_PRESS` conditions became one `PHYSICAL_BUTTON`. What is read back must name only
17+
* entries this build still defines, or the Diagnostic screen counts conditions the editor
18+
* can no longer offer.
19+
*/
20+
// Plain Application: MG4TaskerApp runs a support check of its own on a background thread,
21+
// which would race this test for the same record.
22+
@RunWith(RobolectricTestRunner::class)
23+
@Config(application = android.app.Application::class)
24+
class SupportStoreTest {
25+
26+
private val context = ApplicationProvider.getApplicationContext<android.content.Context>()
27+
28+
@Test fun `names removed from the catalogue are dropped on read`() {
29+
val alive = ConditionType.PHYSICAL_BUTTON.name
30+
SupportStore.save(context, "SWI68", setOf(alive, "STAR_SHORT_PRESS"), emptySet())
31+
32+
val read = SupportStore.supportedConditions(context)!!
33+
assertTrue(alive in read)
34+
assertFalse("STAR_SHORT_PRESS" in read)
35+
assertEquals(1, read.size)
36+
}
37+
38+
@Test fun `never checked stays null so the caller does not filter`() {
39+
assertNull(SupportStore.supportedConditions(context))
40+
}
41+
}

0 commit comments

Comments
 (0)