Merge branch 'feat/physical-button-and-contacts'

Author: malys

AGENTS.md

@@ -1,25 +1,31 @@
 # AGENTS.md — MG4Tasker
 
-Companion app to [MG4Control](../MG4Control). Rule-based automation for the SAIC MG4:
-at ignition, evaluate rules and apply vehicle settings — **through MG4Control**, never
-directly.
+Rule-based automation for the SAIC MG4 and part of **MG4Suite**. At ignition it evaluates
+rules and applies supported actions directly through the shared MG4Hardware safety layer.
+MG4Control is optional and is used only for the "apply saved profile" action.
+
+The workspace `AGENTS.md` and normative workspace `DESIGN.md` apply; this file contains
+only automation-specific additions.
 
 Commit author: malys.training@gmail.com
 
 ## The one rule that shapes everything
 
-**MG4Tasker never touches the vehicle.** It reads a snapshot from MG4Control and requests
-named actions; MG4Control performs every write and applies the 0 km/h speed gate. So:
+**Automation never bypasses MG4Hardware.** MG4Tasker reads a snapshot and executes named,
+typed catalogue actions through MG4Hardware; it never accepts or sends a raw property ID.
+The low-level safety gate applies regardless of which app invokes the library. So:
 
-- No `sharedUserId`, no `android.car.*` permission. MG4Tasker is an ordinary app.
-- One process writes to the car → no concurrent writes, one place enforces the gate.
-- The IPC surface is 4 methods (`ITaskerBridge`), and none takes a raw property id.
+- Vehicle access code, firmware routing and property/transaction IDs live in MG4Hardware,
+  not in this app.
+- Rule actions are serialized; no two automation writes may interleave.
+- MG4Control IPC is limited to profile discovery/application and takes no raw property ID.
 
-## Boundary = the signature permission
+## MG4Control profile boundary
 
-`com.mg4.control.permission.TASKER_BRIDGE` is declared by MG4Control at
-`protectionLevel="signature"`. Both apps must be signed with the **same platform key** or
-MG4Tasker installs and does nothing. The Diagnostic tab reports the bridge state.
+The MG4Control profile bridge is protected by a signature permission. Both apps must be
+signed with the same key for profile discovery/application; all other Tasker actions work
+without MG4Control. The Diagnostic tab reports profile-bridge and hardware-layer state
+separately.
 
 ## Unreadable ≠ false
 
@@ -31,8 +37,8 @@ Covered by `ConditionEvaluatorTest` and `RuleEngineTest`.
 ## Firmware compatibility is annotation-driven
 
 Every vehicle `ConditionType` / `ActionType` entry carries `@SupportedOn(...)`, derived
-from MG4Control's `FirmwareInfo` (`hasHeatFeatures`, `isVsmBased`, `isNewGenVsm`) and the
-per-generation routing in `MG4Hardware`. It is the single source of truth for:
+from MG4Hardware's `FirmwareInfo` and its per-generation routing. It is the single source
+of truth for:
 
 - `docs/firmware-matrix.md` — **generated** by `FirmwareMatrix`, refreshed by the test
   run. Never hand-edit it.
@@ -63,18 +69,12 @@ constructor, so a Kotlin default never applies to a key absent from stored JSON;
 enum would be null at runtime for every pre-existing rule. Read it through `firesOn`, export
 the raw field. Same trap as the TypeToken bug — see `ShrinkerSafeGsonTest`.
 
-## The standstill gate is configurable, and still fails closed
-
-`VehicleWriteGate.allowUpToKmh` moves the "moving" boundary from 0 up to 50 km/h; the app
-stores the user's choice and pushes it in wherever the vehicle layer is initialised. Two
-invariants are not negotiable and are unit-tested: an unreadable or negative speed refuses at
-any threshold, and the shared layer clamps the value. Default is 0.
+## The standstill gate is fixed and fails closed
 
-Park is the **only** thing that can rescue a refusal, and only the unreadable-speed one
-(`decide(speed, threshold, parked)`). It must never override a speed that did read: park at
-30 km/h means a signal is wrong, and trusting the gear there would unlock writes at speed.
-`decideNow()` is the live form and reads the gear only after the speed came back unreadable —
-one property read on the normal path.
+Vehicle-setting writes are allowed only when a valid speed read is exactly 0 km/h. Unknown,
+negative or moving speed refuses the action; park state does not rescue an unreadable speed.
+Do not expose a user-configurable moving threshold. Any existing `allowUpToKmh` or park-rescue
+path is safety debt to remove, not a pattern to extend.
 
 ## A gate refusal is "not now", so it is kept
 
@@ -85,12 +85,12 @@ applied, one attempt each, and a history entry per drain. The poller is a single
 exists only while the queue does — `stopIfEmpty()` clears the flag under the lock `offer`
 takes, otherwise an entry can be stranded with no poller running.
 
-## Vehicle writes go in MG4Control, reads too
+## Vehicle primitives go in MG4Hardware
 
-MG4Tasker adds no vehicle access code. When a new action needs a new capability, implement
-the read/write in MG4Control's `MG4Hardware` (branched per firmware generation — never
-assume universal), expose it through `TaskerBridgeService`, then add the catalogue entry
-here. Property ids and per-generation routing live in MG4Control; do not duplicate them.
+MG4Tasker adds no low-level vehicle access code. When a new action needs a capability,
+implement it in the standalone MG4Hardware repository (branched per firmware generation),
+update the typed catalogue and tests, then update this app's submodule and executor.
+Property IDs and per-generation routing never live in MG4Tasker or MG4Control app code.
 
 Climate, charging, radio and hands-free calls do **not** go through property ids. They use
 the SAIC vendor binder services the car's own apps use (`com.mg4.hardware.saic`). Keep every

CHANGELOG.md

@@ -8,16 +8,47 @@ All notable changes to this project are documented here. Format follows
 
 ### Added
 
+- **"else if" and "else" cases in a rule.** Cases are tried in the order written and exactly
+  one runs: the first whose conditions hold, or the "else" when none did. An unreadable
+  condition stops the rule where it stands instead of falling through to the next case or to
+  the "else" — unreadable is not false, and falling through would write to the vehicle
+  because a reading was missing. Each case is edited in its own window, conditions on one
+  side and actions on the other; the rule screen shows one card per case, in evaluation
+  order, with arrows to reorder the "else if" ones. Up to four "else if" cases per rule. The
+  history names the case that ran, and the wait budget compares the longest case rather than
+  the sum. A rule that names a physical button must name one in every case and takes no
+  "else": the button decides which event stream feeds the whole rule, so a case without one
+  would run on presses it never mentioned. Rules files gain `elseIf` / `elseActions` at format
+  version 2; a file with no case still claims version 1 and stays readable by earlier builds,
+  and a file whose content and version disagree is refused.
+
 - **Wait** action (1 to 60 seconds), and reordering of a rule's actions with the arrows
   on each row. Actions run one after the other in the order shown, so a wait placed
   between two of them lets the first write land before the second is asked for
-  (climate on, then fan level). A rule made only of waits is refused at save time.
+  (climate on, then fan level). A rule made only of waits is refused at save time, and one
+  cycle may wait two minutes in total across all its rules — beyond that the wait is cut
+  short and reported in the history, rather than applying later actions against a stale
+  snapshot.
 - Configuration tab. The speed threshold, the language and the rules file
   (export / import) moved there from the rules pane; the Rules tab is now the rule
   list and nothing else.
 
 ### Changed
 
+- **One "Call" action** instead of two. The same entry now takes a typed number or a name
+  from the phone book; the field is both a search and a number entry. Rules saved with the
+  old "Call a contact" action still load and still call — the entry is deprecated, not
+  removed, because a deleted enum name deserialises to null.
+- **A vehicle action is only attempted on a firmware generation its catalogue entry names.**
+  An unidentified generation reports `firmware support not confirmed` instead of writing to a
+  car nobody could recognise. The editor still offers everything, since hiding entries on a
+  guess would be worse; the Diagnostic tab runs the same check and reports the same verdict.
+- The first launch after an upgrade opens on the Diagnostic tab: a new build's verdicts about
+  this car are what changed.
+- The Diagnostic tab no longer claims "show a message" always reaches the driver: it reports
+  the notification channel being off, which is what the executor does. Radio tuning now
+  checks the vendor radio service, and webhooks and waits stop being reported as blocked by
+  a vehicle layer they never touch.
 - Dialog text raised to the suite's sizes (20sp body, 26sp title) instead of
   Material's phone defaults — the test result, the language list and every
   confirmation. The "Remove" button of a condition or an action and the weekday chips

MG4Hardware

@@ -33,6 +33,7 @@ profile* — becomes available.
 - [MG4Control (optional)](#mg4control-optional)
 - [The speed gate](#the-speed-gate)
 - [Conditions and actions](#conditions-and-actions)
+- [Cases: if, else if, else](#cases-if-else-if-else)
 - [Firmware compatibility](#firmware-compatibility)
 - [Vehicle services: climate, charging, radio, calls](#vehicle-services-climate-charging-radio-calls)
 - [Diagnostic](#diagnostic)
@@ -192,6 +193,13 @@ switching the climate on, then setting the fan level — the **Wait** action (1
 holds the sequence in between. It touches nothing in the car, and appears in the history at
 its place in the sequence. A rule made only of waits is refused at save time: it would hold
 the cycle and change nothing.
+
+One cycle may spend **two minutes waiting in total**, every wait of every rule it evaluates
+counted together — the rules of a cycle run one after the other, so a rule that waits also
+delays the ones after it. Past that budget the wait is cut short and the history says so:
+what follows would otherwise be applied against a snapshot taken before the driver left the
+car. A single rule whose waits exceed the budget is refused at save time, so the limit is
+learned in the editor rather than after the drive.
 - **15 minutes, and never across an ignition cycle.** A rule that fired for this drive
   belongs to this drive. At switch-off the queue is dropped, not applied: the car is stopped
   then, which is exactly what makes it the wrong moment to change the setting the next
@@ -241,6 +249,48 @@ below.
 
 ---
 
+## Cases: if, else if, else
+A rule can carry several cases. They are tried in the order they are written and **exactly
+one of them runs**: the first whose conditions hold wins, and the "else" is what happens when
+none did.
+
+```
+IF    outside temperature < 5 °C      → steering heating 2, seat heating 2
+ELSE IF  outside temperature < 18 °C  → fan level 1
+ELSE                                  → media volume 12
+```
+
+Without cases, that is three rules whose conditions have to exclude each other by hand, and
+the day one range is edited the three stop agreeing. The "else" is optional: a rule that says
+nothing about the remaining situations simply does nothing in them.
+
+**An unreadable reading stops the rule where it stands.** It does not fall through to the next
+case, and it does not reach the "else". Unreadable is not false — falling through would mean
+writing to the vehicle *because* a value was missing, which is the one thing the engine never
+does. The history reports the rule as not evaluable and names the missing signal, exactly as
+it does for a rule with a single case.
+
+**Each case is edited in its own window**, opened from its card on the rule screen: what is
+checked on the left, what is done on the right, each list with its own scroll and its own
+"add" button. The rule screen itself keeps the shape of the rule — the cases in the order they
+are tried, each with the size of what it checks and of what it does — and the order of the
+"else if" cases is changed with the arrows on their cards. A rule takes at most four "else if"
+cases: past that the chain stops being readable at a glance on a screen at arm's length.
+
+The history names the case that ran ("applied — ELSE IF 1"), because "applied" alone leaves
+the one question a branched rule raises after the fact unanswered.
+
+**A rule that names a physical button must name one in every case, and takes no "else".** A
+button condition does not only test something — it decides which event stream brings the rule
+its cycles, for the whole rule. A case that named no button would be evaluated on presses of
+buttons it never mentioned, and an "else", which tests nothing at all, would run on every
+press of every button. Refused at save and at import rather than silently rewired.
+
+The wait budget compares the **longest** case, not the sum of them: only one case runs, so
+adding them up would refuse a rule that can never wait that long.
+
+---
+
 ## Firmware compatibility
 Every vehicle catalogue entry carries a `@SupportedOn(...)` annotation naming the firmware
 generations it works on, derived from MG4Hardware's `FirmwareInfo` and per-generation
@@ -258,6 +308,14 @@ routing. That annotation is the single source of truth for three things:
 > Diagnostic tab is the source of truth for your own car: it reads each signal and shows
 > "unreadable" where the firmware does not expose it.
 
+**When the generation cannot be identified, the editor still offers everything and execution
+refuses.** Hiding entries on a guess would leave a car unable to write rules it can perfectly
+well run, so the picker filters nothing without a positive match. A vehicle action, on the
+other hand, is only attempted on a generation its annotation actually names: anything else is
+reported as `firmware support not confirmed` in the history rather than written to a car we
+could not identify. The Diagnostic tab reports the same verdict, because it runs the same
+check.
+
 Highlights (see the generated matrix for the full grid):
 
 - **Seat / steering heating** — SWI133, SWI68, SWI165 only (the SWI69/131 trims lack the
@@ -331,6 +389,10 @@ The one step it cannot take is the write itself — applying a drive mode to see
 sticks would change the car under the driver. So for vehicle writes, OK means "everything
 checked before writing passes", and the history reports what the write did afterwards.
 
+**The first launch after an upgrade opens on this tab**, because a new build's verdicts about
+this car are what changed. The marker is written by the main screen, never by a background
+receiver: a package replacement must not put an activity in front of a driver who is moving.
+
 Three sections:
 
 - **Execution context** — the prerequisites that decide whether rules run at all: vehicle
@@ -475,8 +537,8 @@ against the path the user picked:
 |---|---|
 | written by a newer version | `version` above the one this build understands |
 | unknown condition or action | a catalogue entry this build cannot perform — never silently dropped |
-| not a readable rules file | malformed, blank name, no condition, no action, duplicate ids, non-finite threshold, weekday outside 1–7 |
-| more than 20 rules | above `RuleStore.MAX_RULES` |
+| not a readable rules file | malformed, blank name, no condition, no action, duplicate ids, non-finite threshold, weekday outside 1–7, a case with no condition or no action, a button rule whose cases do not all name a button, cases in a file claiming version 1 |
+| more than 20 rules | above `RuleStore.MAX_RULES`, or a rule with more than four "else if" cases |
 | no rules | a valid envelope with an empty list |
 
 Picking JSON that belongs to another app reports "not a readable rules file". Files over 256 KB
@@ -487,17 +549,30 @@ are refused on their length, before anything is read into memory.
 ```json
 {
   "format": "mg4tasker-rules",
-  "version": 1,
+  "version": 2,
   "rules": [
     {
       "id": "1f2e…", "name": "Cold morning", "enabled": true, "match": "ALL",
       "conditions": [{ "type": "OUTSIDE_TEMP", "op": "LT", "number": 5.0 }],
-      "actions":    [{ "type": "SET_STEERING_HEAT", "number": 2 }]
+      "actions":    [{ "type": "SET_STEERING_HEAT", "number": 2 }],
+      "elseIf": [
+        {
+          "match": "ALL",
+          "conditions": [{ "type": "OUTSIDE_TEMP", "op": "LT", "number": 18.0 }],
+          "actions":    [{ "type": "SET_FAN_LEVEL", "number": 1 }]
+        }
+      ],
+      "elseActions": [{ "type": "SET_MEDIA_VOLUME", "number": 12 }]
     }
   ]
 }
 ```
 
+`elseIf` and `elseActions` arrived with version 2 and are **absent** from a rule that has no
+other case — so a file whose rules are all plain if/then still says `"version": 1` and stays
+readable by a build that predates cases. A file that does carry one says 2, and an older build
+refuses it whole rather than applying its first case alone.
+
 Renaming a `ConditionType` / `ActionType` constant therefore invalidates older files — the
 import reports the unknown entry rather than guessing.
 

README.md

@@ -106,6 +106,13 @@
             android:screenOrientation="landscape"
             android:windowSoftInputMode="adjustResize" />
 
+        <!-- One case of a rule ("if", "else if", "else"), edited on its own screen. -->
+        <activity
+            android:name=".ui.BranchEditorActivity"
+            android:exported="false"
+            android:screenOrientation="landscape"
+            android:windowSoftInputMode="adjustResize" />
+
         <!-- Persistent vehicle service: inits MG4Hardware, listens for ignition, evaluates
              rules and applies actions directly. connectedDevice matches what it does. -->
         <service