Repository navigation
Raw car replies in the live state and derive values once - #158
Merged
Merged
Conversation
The history path called the phone's clock acquiredAtMillis and carried it as a Long, converting it to the record's acquired_at Timestamp at the store. It now travels as acquiredAt: Instant, from the controller's single clock read through HistoryStore (record, recordStatus, recordDrive, recordConnection) and shouldLogStatus, so the name and type match the acquired_at field and the store no longer converts. The controller still reads the clock once per reply, in whole milliseconds (Instant.ofEpochMilli(System.currentTimeMillis())), so the stored values are unchanged; Instant.now() can carry finer digits. The UI's chargeAtMillis stays a Long. core uses Wire's Instant, so no java.* import enters portable code. No behavior change. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0129RzhbEE2cboMcYyZMuxqX
The live state no longer copies proto fields. TeslaCommands.Charge and TeslaVcsec.Status are gone: parseChargeState and parseStatusResponse return the raw ChargeState and VehicleStatus, as parseDriveState already did, TeslaConnection holds them, and HistoryStore.record takes the ChargeState. The screens read plain fields off the message (charge.battery_level). A value that needs a rule is an extension property in the new core/protocol/StateViews.kt, defined once: the status flags (locked, asleep, userPresent), ChargeState.chargingStateName and chargingMph (the rate fallback), and DriveState.shiftStateName. The samples, the screens and the debug log all use them. The rules moved verbatim from TeslaVcsec.statusOf, Charge.chargingMph and the two name functions in TeslaCommands. The history samples are filled through those properties and keep only what something reads. BatterySample drops nine fields nothing read (usable level, estimated and ideal range, ideal miles added, both charge rates, charger power, voltage and current); the raw record still has them in the log. StatusSample and DriveSample keep their shape. The read surface is unchanged, so projections, health, charts and screens compile as before, and the rules stay: a record without the car's timestamp is left out of the battery and drive samples, and status is timed by acquired_at. Tests follow the new shapes; the derived-value tests live in StateViewsTest. No behavior change: an old-versus-new dump of the read surface over 12,000 seeded random replies matched byte for byte. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0129RzhbEE2cboMcYyZMuxqX
Contributor
ScreenshotsCaptured from the demo build at 773a062. Debug APK: zip from Android CI (GitHub sign-in needed) |
7 of 12 tasks
dzid26
commented
Oct 8, 2026
| get() { | ||
| val state = charging_state ?: return null | ||
| return when { | ||
| state.Charging != null -> "Charging" |
Collaborator
Author
There was a problem hiding this comment.
Can we determine those strings from the field name automatically? What would we lose - translations ability?
The charging state and the gear were turned into strings ("Charging",
"P") and then compared as strings in core, the controller and the
screens, where a typo compiled and failed silently. They are now enums
in core/protocol/StateKinds.kt, one value per member of the car's oneof:
ChargingStateKind (Charging, Complete, Stopped, Disconnected, NoPower,
Starting, Calibrating, Unknown) and ShiftStateKind (P, R, N, D, Invalid,
SNA).
ChargeState.chargingStateName becomes chargingStateKind and
DriveState.shiftStateName becomes shiftStateKind, each still one explicit
when over the oneof and null when unset. Every user switches over:
BatterySample.chargingState and DriveSample.shiftState, the charge
session cutting, the controller's change detection and debug log (which
now prints the enum name), and chargingStateText, a when the compiler
checks for missing cases. The demo history seeds from the enum too. The
log still stores the raw protos, so nothing on disk changes.
StateViewsTest pins the mapping of every member; the test helpers take
the enums instead of names.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0129RzhbEE2cboMcYyZMuxqX
Contributor
ScreenshotsCaptured from the demo build at 2563491. Debug APK: zip from Android CI (GitHub sign-in needed) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Owner direction (2026-10-08, "no duplicating", then "add enums"): the read models stop copying proto fields, and the charging state and gear are enums instead of strings. The car's raw reply is the live state, and every derived value is defined in one place. Three commits: the
acquiredAtrename, the refactor, then the enums. This is a pure refactor with no behavior change.No issue tracks the refactor itself. Part of #120 (domain values as strings: this covers the charging-state and shift-state part; the rest of #120 is untouched). Related: #118.
TeslaCommands.ChargeandTeslaVcsec.Statusare deleted: both copied proto fields and also carriedraw.parseChargeStateandparseStatusResponsereturn the rawChargeState/VehicleStatus, asparseDriveStatealready did.TeslaConnectionholds them,HistoryStore.recordtakes aChargeState, and the screens read plain proto fields (charge.battery_level).core/.../protocol/StateViews.ktholds extension properties for the values that need a rule:VehicleStatus.locked/asleep/userPresent,ChargeState.chargingStateKind,ChargeState.chargingMph(the rate fallback) andDriveState.shiftStateKind. The rules moved verbatim fromTeslaVcsec.statusOf,Charge.chargingMphand the two name functions inTeslaCommands. The samples, the screens and the debug log all use them.ChargingStateKind(Charging, Complete, Stopped, Disconnected, NoPower, Starting, Calibrating, Unknown) andShiftStateKind(P, R, N, D, Invalid, SNA) sit incore/.../protocol/StateKinds.kt, one value per member of the car's oneof, so there is no KMP-unfriendly import.chargingStateKindandshiftStateKindare each one explicitwhenover the oneof, null when unset. Every user switched over:BatterySample.chargingState,DriveSample.shiftState, charge-session cutting, the controller's change detection and debug log (which now prints the enum name), andchargingStateText, awhenthe compiler checks for missing cases. No string comparison against a state name remains. The log still stores the raw protos, so nothing on disk changes.StateViewsTestpins the mapping of every member.BatterySample,StatusSampleandDriveSamplehold only the fields something reads, filled through theStateViewsproperties; the raw record stays on disk.BatterySampledrops nine fields nothing read (usable level, estimated and ideal range, ideal miles added, both charge rates, charger power, voltage and current). The read surface is unchanged, so projections, health, charts and screens compile as before. The rules are unchanged: a record without the car's timestamp stays out of the battery and drive samples, and status is timed byacquired_at. There is noinit { require }; thetoXSample()factories remain the guard.acquiredAt: Instant.acquiredAtMillis: LongbecomesacquiredAt: Instantalong the history path (controller, the fourHistoryStorerecord functions,shouldLogStatusand its test), so the name and type match theacquired_atfield. The controller still reads the clock once per reply, in whole milliseconds, so stored values are unchanged.chargeAtMillisstays aLong.coreuses Wire'sInstant, with no newjava.*import.TeslaCommands.Chargepoints atChargeState.chargingMph.Sample shape and memory (owner decision)
No decision had arrived when I reached the samples, so I built the recommended default: compact samples. The alternative is a wrapper that holds the whole proto. Retained heap for 20k charge samples, measured on a JVM (the full-proto rows use a
ChargeStatewith a third of its ~75 fields set, and with all of its scalars set). These were measured before the enum change, which adds a reference where a string reference was.A
DriveStatewrapper would add 156 B (parked) to 469 B (fully populated) per sample, and it would put the destination and route into memory andtoString; with the compact shapeDriveSamplenever holds them, so ADR-0008's "stays in the log only" stays true. Switching to full-proto wrappers later touches only the three sample classes, their mappings and the tests that hand-build samples.Decisions to confirm
BatterySamplefields are dropped. Nothing in main read them; they remain in the log.charge.battery_level,charge.charge_limit_soc), soBleModels.ktandCarScreen.ktnow import generated types.MainScreen.ktandBleTrackingService.ktonly gain imports for the status flags.MainActivity.ktis untouched.Type
Testing
:core:test :app:assembleDebug :app:testDebugUnitTest :app:lintDebug ktlintCheck detekt. Counts: core 174 onmainbefore, 181 now; app unit tests 30 before and after.StateViewsTest, which pins every enum member's mapping, the unset cases, and the documented fallbacks (a status without flags reads as unlocked, awake and away);BatteryHistoryTestloseschargeStatsDeriveFromTheDisplayedLevel, because its premise (a usable level on the sample) is gone; the displayed-versus-usable check now sits inChargeStateMappingTest;StatusLogPolicyTestpassesInstants.mainand from that tree match byte for byte. A deliberately perturbed rule makes the diff fail, so it can see a regression. The enum commit is covered byStateViewsTestand the existing session, chart and mapping tests rather than re-run through it.FakeCarProtocolTest) cover the parse round trips. A quick smoke test would be: connect, check the battery percentage, the charge-details line and the status line, and check that the charge, status and drive logs still grow.Checklist
expected.txtregenerated, not hand-edited (n/a: no protocol or log-format change; the car's raw replies are logged exactly as before)SPDX-License-Identifier: AGPL-3.0-only🤖 Generated with Claude Code
https://claude.ai/code/session_0129RzhbEE2cboMcYyZMuxqX