Fix telemetry decoding so scooter data actually reaches the UIFix/telemetry reply layout and speed decode - #7
Merged
Conversation
added 2 commits
September 20, 2026 14:07
ScooterReply.parse() expected the decrypted buffer to start with a size byte
(`[size][direction][type][attribute][data]`), but decrypt_uart() returns only
`msg[1..] + rand`: encrypt_uart keeps the size byte outside the ciphertext and
the decryptor never puts it back. The parser was therefore reading the
direction byte (0x23 = 35) as a length and mis-sizing every reply.
* 0x25 (9 bytes) and 0x3A (11 bytes) were rejected outright as
"size byte says 35 bytes but the frame is only N";
* 0xB0 (39 bytes) passed the length check by luck, after which every field
was shifted one byte. `attribute` read the first *data* byte instead of
0xB0, so the reply fell through to the "unknown attribute" branch - which
is a Log.d, stripped by R8 in release builds, leaving no trace at all.
parseTelemetry() therefore never reached parseMotorInfoFromData(),
logger.log() was never called, and every telemetry CSV was header-only: the UI
showed em-dashes for speed and scooter battery over a healthy connected link.
Fix: HEADER_LEN is 3 (direction, type, attribute) and the data runs to
raw.size - PADDING_LEN, because no length field survives decryption. build()
now mirrors decrypt_uart's output; the old builder modelled the *wire* frame,
which is exactly why the suite passed while every real reply was rejected.
Verified on hardware (Xiaomi M365 "MIScooter8964", xiaomi_mi dialect):
* before: 113-byte CSV (header only), "Dropped malformed reply" per reply,
UI speed and scooter battery both em-dash
* after: 63 rows of real values in 35 s (battery 52%, temp 30.0 C,
mileage 399.787 km), zero rejections, UI shows 0.0 km/h and 52%
* the fixed build also feeds the Rokid glasses gateway: the HUD renders
speed and scooter battery at ~1 update/sec
Tests: 313 pass, 0 failures. The new regression test reproduces the field
failure exactly ("size byte says 35 bytes but the frame is only 9") when the
old logic is restored, so it has teeth rather than matching whatever the code
happens to do.
Also corrects the comments and IMPROVEMENT_PLAN section 6.1, which recorded the
opposite conclusion ("the size byte does exist") from buildPacket - buildPacket
describes the pre-encryption command, not the decrypted buffer.
…entinel Two defects in the same two bytes of the 0xB0 motor-info reply. 1. `speed` was read with `bb.getShort(10)` and never masked, while `avgSpeed` two fields over has always been masked. B5 is an unsigned m/h speed, so every genuine reading above 32.767 km/h came back negative: a downhill or an over-speed moment rendered as "-5 km/h" on the HUD. 2. Masking alone would have been worse. When the ESC has no speed estimate it parks B5 just below 0xFFFF (observed 0xFF3E..0xFFF4 on hardware, 11 to 193 counts below the top). Read unsigned those become ~65 km/h, further from the truth than the ~-0.1 km/h the signed read produced. They are now folded to zero; no M365 reaches 65 km/h, so a cut at 0xFF00 separates the sentinel from every real reading with room either side. The offsets and the ÷1000 scale are unchanged, because a capture says they were already right. Rear wheel spun off the ground, odometer rate as an independent reference (3.6 x delta-metres / delta-seconds): over 43 two-second windows the decoded speed matched with a median ratio of 1.023, inside the odometer's own 1 m quantisation. Only the sign and the sentinel were wrong. Also drops the "preserve shipped behavior, not a verified register map" caveat: the B0..BB block layout is now hardware-confirmed, and the class doc records the observed offsets and scales. Known limit, recorded in the code: an isolated mid-range spike is not filtered. One was observed at 0xEA76 (60.022 km/h) while the wheel was barely turning. Nothing in the register distinguishes it from a genuine reading on a modified scooter, so clamping it would be a guess. Tests: 318 pass, 0 failures. MotorInfoParserTest gains real captured payloads as fixtures plus regression cases for both defects. Restoring the old signed read fails 4 of its 12 tests, reproducing the field values exactly (0x8000 decoded as -32.768; the 0xFF3E sentinel as -0.194, which is what the hardware log showed).
|
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
1 participant
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.
What this fixes
Two independent decoding defects that together made a perfectly healthy BLE link show nothing on the HUD.
1. The decrypted reply was read with a size byte that is not there (
9cd5d53)ScooterReply.parse()expected[size][direction][type][attribute][data], butdecrypt_uart()returns onlymsg[1..] + rand—encrypt_uartkeeps the size byte outside the ciphertext and the decryptor never puts it back. The parser was therefore reading the direction byte (0x23= 35) as a length:The SonarQube Scan step is no longer
continue-on-error, and SonarCloud Automatic Analysis must be disabled for CI analysis to run. If it is still on, the scanner fails with "You are running CI analysis while Automatic Analysis is enabled" and this PR's CI will fail regardless of the code. Worth confirming before merging.Known follow-ups (deliberately not in this PR)
ScooterRepository.kt:2144has the same signed-read bug inparseSpeedFromData(bb.getShort(0)unmasked). Left alone: the file is excluded from coverage, the poll loop never requests0xB5(so the path appears dead), and there is no capture to confirm that reply's layout.0xEA76(60.022 km/h) while the wheel was barely turning. Nothing in the register distinguishes it from a genuine reading on a modified scooter, so clamping it would be a guess. Recorded in the code.