From 87ee24967b552632496efde918ec7759c83f83da Mon Sep 17 00:00:00 2001 From: Dave Kneeland Date: Tue, 25 Aug 2026 16:44:55 -0700 Subject: [PATCH 1/2] feat(android): assemble vehicle VIN natively in the C++ decoder Move VIN assembly into the C++ layer (repo/bridge/car/vin_assembler.h), hooked into VehicleDecoder::updateFrame so it rides the existing decode path with no extra pass. The assembler reads raw frame bytes on purpose: VIN_B405/VIN_C405 are 56-bit signals, wider than double's 53-bit mantissa, so the normal Signal::getValue path would silently corrupt the top byte(s). VIN is surfaced next to CarState rather than inside it: CarState is a fixed DoubleArray across JNI that flows to dash apps, and the plan keeps the VIN out of dash-app data/persistence/logs. New JNI surface: nativeGetVin(handle) -> String? and nativeResetVin(handle); Kotlin side exposes them via CanFrameDecoder.getVin()/resetVin(). DashKitDataSource keeps its vinState StateFlow and teardown resets unchanged, so downstream layers need no changes. VehicleVinState stays in Kotlin as the flow's type; the pure-Kotlin assembler and its tests are deleted, CanPacketVinTest now covers packet parsing only, and host-run native unit tests cover the assembler (dashpilot-android/bridge/tests/vin_assembler_test.cpp, wired as a CMake/CTest target for non-Android builds). --- bridge/car/vin_assembler.h | 91 ++++++++++++ bridge/common/vehicle_decoder.h | 8 ++ .../dashpilot/datasource/CanPacket.kt | 40 ++++++ .../dashpilot/datasource/DashKitDataSource.kt | 62 ++++---- .../dashpilot/vehicle/CanFrameDecoder.kt | 7 + .../dashpilot/vehicle/VehicleVinState.kt | 7 + .../viewmodel/ConnectionViewModel.kt | 13 +- .../dashpilot/datasource/CanPacketVinTest.kt | 45 ++++++ dashpilot-android/bridge/CMakeLists.txt | 11 ++ .../bridge/src/main/cpp/jni_bridge.cpp | 17 +++ .../dashpilot/jni/VehicleBridge.kt | 2 + .../bridge/tests/vin_assembler_test.cpp | 132 ++++++++++++++++++ 12 files changed, 398 insertions(+), 37 deletions(-) create mode 100644 bridge/car/vin_assembler.h create mode 100644 dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/CanPacket.kt create mode 100644 dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/VehicleVinState.kt create mode 100644 dashpilot-android/app/src/test/java/com/softwiredtech/dashpilot/datasource/CanPacketVinTest.kt create mode 100644 dashpilot-android/bridge/tests/vin_assembler_test.cpp diff --git a/bridge/car/vin_assembler.h b/bridge/car/vin_assembler.h new file mode 100644 index 00000000..da446660 --- /dev/null +++ b/bridge/car/vin_assembler.h @@ -0,0 +1,91 @@ +#pragma once + +#include +#include +#include + +// Assembles the vehicle VIN from muxed CAN frames carried by the DashKit BLE +// CAN stream. +// +// Frame contract (from bus_1_tesla_vehicle.dbc, message VIN_info / 0x405): +// DLC 8, byte 0 = mux selector: +// mux 0x10 -> VIN chars 1-3 in bytes 5-7 (bytes 1-4 must be zero) +// mux 0x11 -> VIN chars 4-10 in bytes 1-7 +// mux 0x12 -> VIN chars 11-17 in bytes 1-7 +// +// Segments may arrive in any order and may duplicate; a frame that doesn't +// match this contract is ignored. Segments are validated as raw ASCII and as +// legal VIN characters (digits, A-Z minus I/O/Q). The completed VIN stays +// readable until reset(). +// +// Raw bytes are read directly on purpose: VIN_B405/VIN_C405 are 56-bit signals, +// wider than double's 53-bit mantissa, so decoding them through the normal +// signal path would silently corrupt the top byte(s). +class VinAssembler { +public: + static constexpr int kVinBus = 1; + static constexpr uint32_t kVinCanId = 0x405; + static constexpr size_t kFrameLen = 8; + static constexpr size_t kVinLen = 17; + static constexpr uint8_t kMuxA = 0x10; + static constexpr uint8_t kMuxB = 0x11; + static constexpr uint8_t kMuxC = 0x12; + + enum class State { Waiting, Invalid, Ready }; + + // Feeds one raw frame; returns the assembly state after processing it. + // Called from VehicleDecoder::updateFrame(), so no extra decode pass is + // needed. Not synchronized, same threading model as CANParsers. + State onFrame(int bus, uint32_t address, const uint8_t* data, size_t len) { + if (!completed_.empty()) return State::Ready; + if (bus != kVinBus || address != kVinCanId || len != kFrameLen) { + return State::Waiting; + } + + int index; + size_t offset; + switch (data[0]) { + case kMuxA: index = 0; offset = 5; break; + case kMuxB: index = 1; offset = 1; break; + case kMuxC: index = 2; offset = 1; break; + default: return State::Waiting; + } + if (index == 0) { + // Mux A carries only 3 real chars; bytes 1-4 must be zero. + for (size_t i = 1; i < offset; i++) { + if (data[i] != 0x00) return State::Invalid; + } + } + for (size_t i = offset; i < kFrameLen; i++) { + if (!isVinChar(data[i])) return State::Invalid; + } + + segments_[index].assign(reinterpret_cast(data) + offset, + kFrameLen - offset); + for (const auto& segment : segments_) { + if (segment.empty()) return State::Waiting; + } + completed_ = segments_[0] + segments_[1] + segments_[2]; + return State::Ready; + } + + bool ready() const { return !completed_.empty(); } + + const std::string& vin() const { return completed_; } + + void reset() { + for (auto& segment : segments_) segment.clear(); + completed_.clear(); + } + +private: + // ISO 3779: digits and letters except I, O, Q. + static bool isVinChar(uint8_t c) { + if (c >= '0' && c <= '9') return true; + if (c >= 'A' && c <= 'Z') return c != 'I' && c != 'O' && c != 'Q'; + return false; + } + + std::array segments_; + std::string completed_; +}; diff --git a/bridge/common/vehicle_decoder.h b/bridge/common/vehicle_decoder.h index 9a1e4442..e3fe0ac2 100644 --- a/bridge/common/vehicle_decoder.h +++ b/bridge/common/vehicle_decoder.h @@ -3,6 +3,7 @@ #include "car/can_parsers.h" #include "car/car_state.h" #include "car/car_state_mapper.h" +#include "car/vin_assembler.h" #include "car/cars/tesla.h" #include "msgq/ipc.h" #include @@ -34,6 +35,7 @@ class VehicleDecoder { void updateFrame(int bus, uint32_t address, const uint8_t* data, size_t len) { parsers_.updateFrame(bus, address, data, len); + vin_.onFrame(bus, address, data, len); } void updateMapper() { @@ -42,8 +44,14 @@ class VehicleDecoder { CarState& state() { return state_; } + // Vehicle VIN assembled from the CAN stream (empty until complete). + bool vinReady() const { return vin_.ready(); } + const std::string& vin() const { return vin_.vin(); } + void resetVin() { vin_.reset(); } + private: CANParsers parsers_; std::unique_ptr mapper_; CarState state_; + VinAssembler vin_; }; diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/CanPacket.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/CanPacket.kt new file mode 100644 index 00000000..f671ee38 --- /dev/null +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/CanPacket.kt @@ -0,0 +1,40 @@ +package com.softwiredtech.dashpilot.datasource + +class RawCanFrame( + val bus: Int, + val address: Int, + val data: ByteArray, +) + +// Wire format from firmware (build_ble_packet): +// [count : 1] +// per frame: +// [timestamp_us : LE32] +// [bus : 1] +// [addr : LE32] +// [len : 1] +// [data : len bytes] +internal fun parseCanPacket(payload: ByteArray): List { + if (payload.isEmpty()) return emptyList() + val frames = ArrayList(payload[0].toInt() and 0xFF) + var offset = 1 + val count = payload[0].toInt() and 0xFF + for (i in 0 until count) { + if (offset + 4 > payload.size) break + offset += 4 + if (offset >= payload.size) break + val bus = payload[offset].toInt() and 0xFF + offset += 1 + if (offset + 4 > payload.size) break + val addr = java.nio.ByteBuffer.wrap(payload, offset, 4) + .order(java.nio.ByteOrder.LITTLE_ENDIAN).int + offset += 4 + if (offset >= payload.size) break + val len = payload[offset].toInt() and 0xFF + offset += 1 + if (offset + len > payload.size) break + frames.add(RawCanFrame(bus, addr, payload.copyOfRange(offset, offset + len))) + offset += len + } + return frames +} diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/DashKitDataSource.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/DashKitDataSource.kt index 29e34b51..7ee6d17d 100644 --- a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/DashKitDataSource.kt +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/DashKitDataSource.kt @@ -5,33 +5,36 @@ import android.bluetooth.BluetoothGatt import android.bluetooth.BluetoothGattCharacteristic import android.util.Log import com.softwiredtech.dashpilot.datamodel.dash.CarState -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableSharedFlow -import java.nio.ByteBuffer -import java.nio.ByteOrder -import java.util.UUID import com.softwiredtech.dashpilot.vehicle.CanFrameDecoder +import com.softwiredtech.dashpilot.vehicle.VehicleVinState import kotlinx.coroutines.FlowPreview +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.sample +import java.util.UUID @SuppressLint("MissingPermission") class DashKitDataSource( private val manager: DashKitBleManager, - private val decoder: CanFrameDecoder + private val decoder: CanFrameDecoder, ) : IDataSource, GattListener { companion object { private const val TAG = "DashKitDataSource" private val SERVICE_UUID = UUID.fromString("CADA0000-CA00-B1E0-B0D6-C000AA0100A1") private val CHAR_UUID = UUID.fromString("CADA0001-CA00-B1E0-B0D6-C000AA0100A1") - // The firmware now applies CAN acceptance filtering in hardware (per-bus - // MCP251xFD filters), so the app no longer pushes a BLE filter list. } private val _incoming = MutableSharedFlow(replay = 1) @OptIn(FlowPreview::class) override val incomingMessages: Flow = _incoming.sample(40) + private val _vinState = MutableStateFlow(VehicleVinState.Waiting) + val vinState: StateFlow = _vinState.asStateFlow() + private var currentState = CarState() override fun connect(address: String) { @@ -40,10 +43,13 @@ class DashKitDataSource( } override fun disconnect() { + resetVin() manager.removeGattListener(this) } override fun onServicesReady(gatt: BluetoothGatt) { + resetVin() + val service = gatt.getService(SERVICE_UUID) if (service == null) { Log.e(TAG, "CAN BLE service not found") @@ -67,37 +73,21 @@ class DashKitDataSource( parseAndEmit(value) } + override fun onDisconnected() = resetVin() + private fun parseAndEmit(payload: ByteArray) { - // Wire format from firmware (build_ble_packet): - // [count : 1] - // per frame: - // [timestamp_us : LE32] - // [bus : 1] - // [addr : LE32] - // [len : 1] - // [data : len bytes] if (payload.isEmpty()) return - val count = payload[0].toInt() and 0xFF - var offset = 1 - for (i in 0 until count) { - if (offset + 4 > payload.size) break - // Timestamp is currently unused by the decoder; skip it. - offset += 4 - if (offset >= payload.size) break - val bus = payload[offset].toInt() and 0xFF - offset += 1 - if (offset + 4 > payload.size) break - val addr = ByteBuffer.wrap(payload, offset, 4).order(ByteOrder.LITTLE_ENDIAN).int - offset += 4 - if (offset >= payload.size) break - val len = payload[offset].toInt() and 0xFF - offset += 1 - if (offset + len > payload.size) break - val data = payload.copyOfRange(offset, offset + len) - offset += len - - currentState = decoder.decodeFrame(bus, addr, data) + for (frame in parseCanPacket(payload)) { + currentState = decoder.decodeFrame(frame.bus, frame.address, frame.data) } + // decodeFrame drives the native VIN assembler + _vinState.value = decoder.getVin()?.let { VehicleVinState.Available(it) } + ?: VehicleVinState.Waiting _incoming.tryEmit(currentState) } + + private fun resetVin() { + decoder.resetVin() + _vinState.value = VehicleVinState.Waiting + } } diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/CanFrameDecoder.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/CanFrameDecoder.kt index f2e5210c..7b1f073d 100644 --- a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/CanFrameDecoder.kt +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/CanFrameDecoder.kt @@ -18,6 +18,13 @@ class CanFrameDecoder( return arrayToCarState(values) } + // VIN assembled natively during decodeFrame; null until complete. + fun getVin(): String? = bridge.nativeGetVin(decoderHandle) + + fun resetVin() { + bridge.nativeResetVin(decoderHandle) + } + fun destroy() { bridge.nativeDestroyVehicleDecoder(decoderHandle) } diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/VehicleVinState.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/VehicleVinState.kt new file mode 100644 index 00000000..00677ae9 --- /dev/null +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/VehicleVinState.kt @@ -0,0 +1,7 @@ +package com.softwiredtech.dashpilot.vehicle + +sealed interface VehicleVinState { + data object Waiting : VehicleVinState + data class Available(val vin: String) : VehicleVinState + data object Invalid : VehicleVinState +} diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/viewmodel/ConnectionViewModel.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/viewmodel/ConnectionViewModel.kt index 787e2920..70c2210f 100644 --- a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/viewmodel/ConnectionViewModel.kt +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/viewmodel/ConnectionViewModel.kt @@ -60,6 +60,7 @@ import com.softwiredtech.dashpilot.jni.VehicleBridge import com.softwiredtech.dashpilot.util.NetworkUtil import com.softwiredtech.dashpilot.vehicle.CanFrameDecoder import com.softwiredtech.dashpilot.vehicle.VehicleProfileLoader +import com.softwiredtech.dashpilot.vehicle.VehicleVinState import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.delay @@ -278,6 +279,9 @@ class ConnectionViewModel(private var networkUtil: NetworkUtil) : ViewModel() { private val _dashState = MutableStateFlow?>(null) val dashState = _dashState.asStateFlow() + private val _vehicleVin = MutableStateFlow(VehicleVinState.Waiting) + val vehicleVin: StateFlow = _vehicleVin.asStateFlow() + private fun phoneBatteryFlow(context: Context): Flow = flow { val batteryManager = context.getSystemService(Context.BATTERY_SERVICE) as BatteryManager while (true) { @@ -344,7 +348,13 @@ class ConnectionViewModel(private var networkUtil: NetworkUtil) : ViewModel() { } } val decoder = CanFrameDecoder(bridge, profile) - DashKitDataSource(manager, decoder) + val ds = DashKitDataSource(manager, decoder) + launch { + ds.vinState.collect { + if (_dataSource.value === ds) _vehicleVin.value = it + } + } + ds } DataSourceType.WEBSOCKET -> WebsocketDataSource() else -> CommaDataSource(bridge, profile) @@ -442,6 +452,7 @@ class ConnectionViewModel(private var networkUtil: NetworkUtil) : ViewModel() { _bleManager.value?.disconnect() _bleManager.value = null _dashState.value = null + _vehicleVin.value = VehicleVinState.Waiting _hasAutoNavigatedToDashboard.value = false } diff --git a/dashpilot-android/app/src/test/java/com/softwiredtech/dashpilot/datasource/CanPacketVinTest.kt b/dashpilot-android/app/src/test/java/com/softwiredtech/dashpilot/datasource/CanPacketVinTest.kt new file mode 100644 index 00000000..85626d71 --- /dev/null +++ b/dashpilot-android/app/src/test/java/com/softwiredtech/dashpilot/datasource/CanPacketVinTest.kt @@ -0,0 +1,45 @@ +package com.softwiredtech.dashpilot.datasource + +import org.junit.Assert.assertEquals +import org.junit.Test +import java.nio.ByteBuffer +import java.nio.ByteOrder + +class CanPacketVinTest { + @Test + fun parses_vin_frames_from_ble_notification() { + val vin = "5YJ3E7EB1MF123456" + val frames = listOf( + Triple(0x12, vin.substring(10), 1), + Triple(0x10, vin.substring(0, 3), 5), + Triple(0x11, vin.substring(3, 10), 1), + ) + val packet = ByteBuffer.allocate(1 + frames.size * 18) + .order(ByteOrder.LITTLE_ENDIAN) + .put(frames.size.toByte()) + for ((mux, text, offset) in frames) { + packet.putInt(0) + packet.put(1.toByte()) + packet.putInt(0x405) + packet.put(8.toByte()) + val data = ByteArray(8) + data[0] = mux.toByte() + text.forEachIndexed { index, char -> data[offset + index] = char.code.toByte() } + packet.put(data) + } + + val parsed = parseCanPacket(packet.array()) + + assertEquals(frames.size, parsed.size) + for ((i, frame) in parsed.withIndex()) { + val (mux, text, offset) = frames[i] + assertEquals(1, frame.bus) + assertEquals(0x405, frame.address) + assertEquals(8, frame.data.size) + assertEquals(mux.toByte(), frame.data[0]) + text.forEachIndexed { index, char -> + assertEquals(char.code.toByte(), frame.data[offset + index]) + } + } + } +} diff --git a/dashpilot-android/bridge/CMakeLists.txt b/dashpilot-android/bridge/CMakeLists.txt index 6ebaca56..12c15fa8 100644 --- a/dashpilot-android/bridge/CMakeLists.txt +++ b/dashpilot-android/bridge/CMakeLists.txt @@ -4,6 +4,8 @@ project(dashpilot-android LANGUAGES CXX) set(CMAKE_CXX_STANDARD 17) set(MSGQ_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../../bridge") + +if(ANDROID) set(ZMQ_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../third_party/zmq") set(CAPNP_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../third_party/capnp") set(GENERATED_CEREAL_FILES "${CMAKE_CURRENT_SOURCE_DIR}/../../bridge/cereal/gen") @@ -71,3 +73,12 @@ target_link_libraries(bridge android log ) +else() +# Host-side unit tests for the header-only native logic (not built for Android). +# Build & run: cmake -B build-host -S . && cmake --build build-host && ctest --test-dir build-host +enable_testing() +add_executable(vin_assembler_test tests/vin_assembler_test.cpp) +target_include_directories(vin_assembler_test PRIVATE ${MSGQ_ROOT}) +target_compile_options(vin_assembler_test PRIVATE -Wall -Wextra) +add_test(NAME vin_assembler COMMAND vin_assembler_test) +endif() diff --git a/dashpilot-android/bridge/src/main/cpp/jni_bridge.cpp b/dashpilot-android/bridge/src/main/cpp/jni_bridge.cpp index 9ed7cf94..c156e981 100644 --- a/dashpilot-android/bridge/src/main/cpp/jni_bridge.cpp +++ b/dashpilot-android/bridge/src/main/cpp/jni_bridge.cpp @@ -223,6 +223,23 @@ Java_com_softwiredtech_dashpilot_jni_VehicleBridge_nativeDestroyVehicleDecoder( bridge::destroyVehicleDecoder(reinterpret_cast(decoderHandle)); } +JNIEXPORT jstring JNICALL +Java_com_softwiredtech_dashpilot_jni_VehicleBridge_nativeGetVin( + JNIEnv* env, jobject thiz, + jlong decoderHandle) { + auto* decoder = reinterpret_cast(decoderHandle); + if (!decoder || !decoder->vinReady()) return nullptr; + return env->NewStringUTF(decoder->vin().c_str()); +} + +JNIEXPORT void JNICALL +Java_com_softwiredtech_dashpilot_jni_VehicleBridge_nativeResetVin( + JNIEnv* env, jobject thiz, + jlong decoderHandle) { + auto* decoder = reinterpret_cast(decoderHandle); + if (decoder) decoder->resetVin(); +} + // === Receive loop === JNIEXPORT void JNICALL Java_com_softwiredtech_dashpilot_jni_VehicleBridge_nativeStartReceiveLoop( diff --git a/dashpilot-android/bridge/src/main/java/com/softwiredtech/dashpilot/jni/VehicleBridge.kt b/dashpilot-android/bridge/src/main/java/com/softwiredtech/dashpilot/jni/VehicleBridge.kt index 7426e192..73a34349 100644 --- a/dashpilot-android/bridge/src/main/java/com/softwiredtech/dashpilot/jni/VehicleBridge.kt +++ b/dashpilot-android/bridge/src/main/java/com/softwiredtech/dashpilot/jni/VehicleBridge.kt @@ -46,6 +46,8 @@ class VehicleBridge { // VehicleDecoder external fun nativeCreateVehicleDecoder(dbcContents: Array, busIndices: IntArray, vehicleType: String): Long external fun nativeDecodeCanFrame(decoderHandle: Long, bus: Int, address: Int, data: ByteArray): DoubleArray + external fun nativeGetVin(decoderHandle: Long): String? + external fun nativeResetVin(decoderHandle: Long) external fun nativeDestroyVehicleDecoder(decoderHandle: Long) // Message diff --git a/dashpilot-android/bridge/tests/vin_assembler_test.cpp b/dashpilot-android/bridge/tests/vin_assembler_test.cpp new file mode 100644 index 00000000..719cc850 --- /dev/null +++ b/dashpilot-android/bridge/tests/vin_assembler_test.cpp @@ -0,0 +1,132 @@ +// Host-run unit tests for VinAssembler (repo/bridge/car/vin_assembler.h). +// Built by CMakeLists.txt when not configuring for Android: +// cmake -B build-host -S . && cmake --build build-host && ctest --test-dir build-host + +#include "car/vin_assembler.h" + +#include +#include +#include + +static int failures = 0; + +#define CHECK(cond) \ + do { \ + if (!(cond)) { \ + std::printf("FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + failures++; \ + } \ + } while (0) + +namespace { + +const char* kVin = "5YJ3E7EB1MF123456"; + +using State = VinAssembler::State; + +// Builds an 8-byte frame: byte 0 = mux, text written at offset, rest zero. +std::string vinFrame(uint8_t mux, const char* text, size_t offset) { + std::string frame(VinAssembler::kFrameLen, '\0'); + frame[0] = static_cast(mux); + std::memcpy(frame.data() + offset, text, std::strlen(text)); + return frame; +} + +std::string frameA() { return vinFrame(0x10, std::string(kVin, 0, 3).c_str(), 5); } +std::string frameB() { return vinFrame(0x11, std::string(kVin, 3, 7).c_str(), 1); } +std::string frameC() { return vinFrame(0x12, std::string(kVin, 10, 7).c_str(), 1); } + +State feed(VinAssembler& a, int bus, uint32_t address, const std::string& data) { + return a.onFrame(bus, address, reinterpret_cast(data.data()), + data.size()); +} + +void assembles_out_of_order_frames_with_duplicates() { + VinAssembler a; + CHECK(feed(a, 1, 0x405, frameC()) == State::Waiting); + CHECK(feed(a, 1, 0x405, frameA()) == State::Waiting); + CHECK(feed(a, 1, 0x405, frameA()) == State::Waiting); // duplicate is tolerated + CHECK(feed(a, 1, 0x405, frameB()) == State::Ready); + CHECK(a.vin() == kVin); +} + +void completion_persists_until_reset() { + VinAssembler a; + feed(a, 1, 0x405, frameB()); + feed(a, 1, 0x405, frameC()); + CHECK(feed(a, 1, 0x405, frameA()) == State::Ready); + // Subsequent unrelated frames keep reporting the completed VIN. + CHECK(feed(a, 1, 0x405, vinFrame(0x13, "junk", 1)) == State::Ready); + CHECK(a.vin() == kVin); + + a.reset(); + CHECK(!a.ready()); + CHECK(a.vin().empty()); + CHECK(feed(a, 1, 0x405, frameC()) == State::Waiting); + CHECK(a.vin().empty()); +} + +void rejects_illegal_vin_characters() { + const char* illegal[] = {"I", "O", "Q", "a", "\xC3\x84"}; + for (const char* c : illegal) { + VinAssembler a; + std::string frame = frameC(); + frame.replace(1, std::strlen(c), c); + feed(a, 1, 0x405, frameA()); + feed(a, 1, 0x405, frameB()); + CHECK(feed(a, 1, 0x405, frame) == State::Invalid); + CHECK(!a.ready()); + + // Invalid isn't latched: a valid retransmission of the segment completes. + CHECK(feed(a, 1, 0x405, frameC()) == State::Ready); + CHECK(a.vin() == kVin); + } +} + +void rejects_zero_padding_violation_on_mux_a() { + VinAssembler a; + std::string frame = frameA(); + frame[2] = 'X'; // bytes 1-4 must be zero for mux A + CHECK(feed(a, 1, 0x405, frame) == State::Invalid); + CHECK(!a.ready()); +} + +void ignores_frames_outside_the_vin_contract() { + VinAssembler a; + // Wrong bus and wrong CAN id. + CHECK(feed(a, 0, 0x405, frameA()) == State::Waiting); + CHECK(feed(a, 1, 0x404, frameA()) == State::Waiting); + // Wrong DLC. + std::string short_frame = frameA(); + short_frame.resize(6); + CHECK(feed(a, 1, 0x405, short_frame) == State::Waiting); + // Unknown mux. + CHECK(feed(a, 1, 0x405, vinFrame(0x20, "junk", 1)) == State::Waiting); + CHECK(!a.ready()); +} + +void assembles_in_order_from_single_frames() { + VinAssembler a; + CHECK(feed(a, 1, 0x405, frameA()) == State::Waiting); + CHECK(feed(a, 1, 0x405, frameB()) == State::Waiting); + CHECK(feed(a, 1, 0x405, frameC()) == State::Ready); + CHECK(a.vin() == kVin); +} + +} // namespace + +int main() { + assembles_out_of_order_frames_with_duplicates(); + completion_persists_until_reset(); + rejects_illegal_vin_characters(); + rejects_zero_padding_violation_on_mux_a(); + ignores_frames_outside_the_vin_contract(); + assembles_in_order_from_single_frames(); + + if (failures > 0) { + std::printf("%d check(s) failed\n", failures); + return 1; + } + std::printf("all vin_assembler tests passed\n"); + return 0; +} From 84269ce473326a0f5695ae31900aa0d27fe16c4a Mon Sep 17 00:00:00 2001 From: Ahmed Harmouche Date: Thu, 27 Aug 2026 10:45:37 +0200 Subject: [PATCH 2/2] Simplify VIN updater --- bridge/car/can_parsers.h | 17 ++ bridge/car/car_state.h | 14 +- bridge/car/cars/tesla.h | 65 +++++++ bridge/car/vin_assembler.h | 91 ---------- bridge/common/vehicle_decoder.h | 8 - bridge/dbc/dbc.cc | 16 +- bridge/dbc/dbc.h | 3 + dashpilot-android/.gitignore | 1 + .../dashpilot/datamodel/dash/CarState.kt | 7 +- .../dashpilot/datasource/DashKitDataSource.kt | 20 --- .../dashpilot/vehicle/CanFrameDecoder.kt | 26 ++- .../dashpilot/vehicle/VehicleVinState.kt | 7 - .../viewmodel/ConnectionViewModel.kt | 13 +- dashpilot-android/bridge/CMakeLists.txt | 14 +- .../bridge/src/main/cpp/jni_bridge.cpp | 17 -- .../dashpilot/jni/VehicleBridge.kt | 2 - .../bridge/tests/tesla_vin_mapper_test.cpp | 165 ++++++++++++++++++ .../bridge/tests/vin_assembler_test.cpp | 132 -------------- 18 files changed, 312 insertions(+), 306 deletions(-) delete mode 100644 bridge/car/vin_assembler.h delete mode 100644 dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/VehicleVinState.kt create mode 100644 dashpilot-android/bridge/tests/tesla_vin_mapper_test.cpp delete mode 100644 dashpilot-android/bridge/tests/vin_assembler_test.cpp diff --git a/bridge/car/can_parsers.h b/bridge/car/can_parsers.h index 6c3306bc..60dfc2b0 100644 --- a/bridge/car/can_parsers.h +++ b/bridge/car/can_parsers.h @@ -58,6 +58,23 @@ class CANParsers { return value; } + // Unscaled signal bits; false if the signal is unknown, no frame has been + // seen, or the frame's multiplexor doesn't select this signal. + bool getRaw(int bus, const std::string& msgName, const std::string& sigName, uint64_t* out) const { + int maskedBus = bus & 3; + auto busIt = busSignalCache_.find(maskedBus); + if (busIt == busSignalCache_.end()) return false; + + auto it = busIt->second.find(msgName + "::" + sigName); + if (it == busIt->second.end()) return false; + + const auto& cached = it->second; + auto frameIt = latestFrames_.find(frameKey(maskedBus, cached.address)); + if (frameIt == latestFrames_.end()) return false; + + return cached.signal->getRawBits(frameIt->second.data(), frameIt->second.size(), out); + } + private: struct CachedSignal { uint32_t address; diff --git a/bridge/car/car_state.h b/bridge/car/car_state.h index 064e49a1..693a7263 100644 --- a/bridge/car/car_state.h +++ b/bridge/car/car_state.h @@ -1,6 +1,7 @@ #pragma once #include +#include struct CarState { // Party bus @@ -40,7 +41,12 @@ struct CarState { double experimentalMode = 0; double changingLane = 0; - static constexpr size_t FIELD_COUNT = 31; + // Vehicle VIN, empty until fully assembled by the mapper. + static constexpr size_t VIN_LENGTH = 17; + char vin[VIN_LENGTH + 1] = {}; + + static constexpr size_t VIN_DOUBLE_COUNT = 3; + static constexpr size_t FIELD_COUNT = 31 + VIN_DOUBLE_COUNT; void toArray(double* out) const { // Party bus @@ -80,5 +86,11 @@ struct CarState { out[29] = changingLane; out[30] = acTemp; + + // VIN chars ride the double array as raw bit patterns, 8 bytes per + // double; decoded by CanFrameDecoder.arrayToCarState. + char padded[VIN_DOUBLE_COUNT * sizeof(double)] = {}; + std::memcpy(padded, vin, VIN_LENGTH); + std::memcpy(out + 31, padded, sizeof(padded)); } }; diff --git a/bridge/car/cars/tesla.h b/bridge/car/cars/tesla.h index d58b7325..274808f4 100644 --- a/bridge/car/cars/tesla.h +++ b/bridge/car/cars/tesla.h @@ -1,9 +1,68 @@ #pragma once +#include #include +#include +#include #include "car/car_state_mapper.h" +// Assembles CarState::vin from VIN_info (0x405) on the vehicle bus. The VIN +// arrives as three muxed segments in any order; each is read as unscaled bits +// because the 56-bit VIN signals are wider than double's 53-bit mantissa, so +// the normal cp.get() path would silently corrupt the top byte(s). +class TeslaVinUpdater { +public: + void update(const CANParsers& cp, CarState& cs) { + if (seen_ == kAllSegments) return; + for (size_t i = 0; i < kSegments.size(); i++) { + if (seen_ & (1u << i)) continue; + const Segment& seg = kSegments[i]; + + uint64_t raw; + if (!cp.getRaw(1, "VIN_info", seg.signal, &raw)) continue; + + char chars[7]; + for (int b = 0; b < 7; b++) chars[b] = static_cast(raw >> (8 * b)); + + bool valid = true; + for (int b = 0; b < seg.textStart; b++) valid &= chars[b] == '\0'; + for (int b = seg.textStart; b < 7; b++) valid &= isVinChar(chars[b]); + if (!valid) continue; + + std::memcpy(pending_ + seg.vinOffset, chars + seg.textStart, 7 - seg.textStart); + seen_ |= 1u << i; + } + if (seen_ == kAllSegments) { + std::memcpy(cs.vin, pending_, CarState::VIN_LENGTH); + } + } + +private: + struct Segment { + const char* signal; + int textStart; // first payload byte; earlier bytes must be zero + int vinOffset; + }; + + static constexpr std::array kSegments{{ + {"VIN_A405", 4, 0}, + {"VIN_B405", 0, 3}, + {"VIN_C405", 0, 10}, + }}; + static constexpr uint8_t kAllSegments = 0b111; + + // ISO 3779: digits and letters except I, O, Q. + static bool isVinChar(char c) { + if (c >= '0' && c <= '9') return true; + if (c >= 'A' && c <= 'Z') return c != 'I' && c != 'O' && c != 'Q'; + return false; + } + + uint8_t seen_ = 0; + char pending_[CarState::VIN_LENGTH] = {}; +}; + static inline void updatePartyBus(const CANParsers& cp, CarState& cs) { cs.egoSteeringAngle = cp.get(2, "SCCM_steeringAngleSensor", "SCCM_steeringAngle"); cs.gear = cp.get(2, "DI_systemStatus", "DI_gear"); @@ -59,7 +118,11 @@ class TeslaCommaExtraMapper : public CarStateMapper { void update(const CANParsers& cp, CarState& cs) override { updatePartyBus(cp, cs); updateVehicleBus(cp, cs); + vin_.update(cp, cs); } + +private: + TeslaVinUpdater vin_; }; class TeslaDashKitMapper : public CarStateMapper { @@ -101,6 +164,7 @@ class TeslaDashKitMapper : public CarStateMapper { cs.acTemp = cp.get(1, "UI_hvacRequest", "UI_hvacReqTempSetpointLeft"); updateVehicleBus(cp, cs); + vin_.update(cp, cs); } private: @@ -127,4 +191,5 @@ class TeslaDashKitMapper : public CarStateMapper { BlinkerHold leftBlinkerHold_; BlinkerHold rightBlinkerHold_; + TeslaVinUpdater vin_; }; \ No newline at end of file diff --git a/bridge/car/vin_assembler.h b/bridge/car/vin_assembler.h deleted file mode 100644 index da446660..00000000 --- a/bridge/car/vin_assembler.h +++ /dev/null @@ -1,91 +0,0 @@ -#pragma once - -#include -#include -#include - -// Assembles the vehicle VIN from muxed CAN frames carried by the DashKit BLE -// CAN stream. -// -// Frame contract (from bus_1_tesla_vehicle.dbc, message VIN_info / 0x405): -// DLC 8, byte 0 = mux selector: -// mux 0x10 -> VIN chars 1-3 in bytes 5-7 (bytes 1-4 must be zero) -// mux 0x11 -> VIN chars 4-10 in bytes 1-7 -// mux 0x12 -> VIN chars 11-17 in bytes 1-7 -// -// Segments may arrive in any order and may duplicate; a frame that doesn't -// match this contract is ignored. Segments are validated as raw ASCII and as -// legal VIN characters (digits, A-Z minus I/O/Q). The completed VIN stays -// readable until reset(). -// -// Raw bytes are read directly on purpose: VIN_B405/VIN_C405 are 56-bit signals, -// wider than double's 53-bit mantissa, so decoding them through the normal -// signal path would silently corrupt the top byte(s). -class VinAssembler { -public: - static constexpr int kVinBus = 1; - static constexpr uint32_t kVinCanId = 0x405; - static constexpr size_t kFrameLen = 8; - static constexpr size_t kVinLen = 17; - static constexpr uint8_t kMuxA = 0x10; - static constexpr uint8_t kMuxB = 0x11; - static constexpr uint8_t kMuxC = 0x12; - - enum class State { Waiting, Invalid, Ready }; - - // Feeds one raw frame; returns the assembly state after processing it. - // Called from VehicleDecoder::updateFrame(), so no extra decode pass is - // needed. Not synchronized, same threading model as CANParsers. - State onFrame(int bus, uint32_t address, const uint8_t* data, size_t len) { - if (!completed_.empty()) return State::Ready; - if (bus != kVinBus || address != kVinCanId || len != kFrameLen) { - return State::Waiting; - } - - int index; - size_t offset; - switch (data[0]) { - case kMuxA: index = 0; offset = 5; break; - case kMuxB: index = 1; offset = 1; break; - case kMuxC: index = 2; offset = 1; break; - default: return State::Waiting; - } - if (index == 0) { - // Mux A carries only 3 real chars; bytes 1-4 must be zero. - for (size_t i = 1; i < offset; i++) { - if (data[i] != 0x00) return State::Invalid; - } - } - for (size_t i = offset; i < kFrameLen; i++) { - if (!isVinChar(data[i])) return State::Invalid; - } - - segments_[index].assign(reinterpret_cast(data) + offset, - kFrameLen - offset); - for (const auto& segment : segments_) { - if (segment.empty()) return State::Waiting; - } - completed_ = segments_[0] + segments_[1] + segments_[2]; - return State::Ready; - } - - bool ready() const { return !completed_.empty(); } - - const std::string& vin() const { return completed_; } - - void reset() { - for (auto& segment : segments_) segment.clear(); - completed_.clear(); - } - -private: - // ISO 3779: digits and letters except I, O, Q. - static bool isVinChar(uint8_t c) { - if (c >= '0' && c <= '9') return true; - if (c >= 'A' && c <= 'Z') return c != 'I' && c != 'O' && c != 'Q'; - return false; - } - - std::array segments_; - std::string completed_; -}; diff --git a/bridge/common/vehicle_decoder.h b/bridge/common/vehicle_decoder.h index e3fe0ac2..9a1e4442 100644 --- a/bridge/common/vehicle_decoder.h +++ b/bridge/common/vehicle_decoder.h @@ -3,7 +3,6 @@ #include "car/can_parsers.h" #include "car/car_state.h" #include "car/car_state_mapper.h" -#include "car/vin_assembler.h" #include "car/cars/tesla.h" #include "msgq/ipc.h" #include @@ -35,7 +34,6 @@ class VehicleDecoder { void updateFrame(int bus, uint32_t address, const uint8_t* data, size_t len) { parsers_.updateFrame(bus, address, data, len); - vin_.onFrame(bus, address, data, len); } void updateMapper() { @@ -44,14 +42,8 @@ class VehicleDecoder { CarState& state() { return state_; } - // Vehicle VIN assembled from the CAN stream (empty until complete). - bool vinReady() const { return vin_.ready(); } - const std::string& vin() const { return vin_.vin(); } - void resetVin() { vin_.reset(); } - private: CANParsers parsers_; std::unique_ptr mapper_; CarState state_; - VinAssembler vin_; }; diff --git a/bridge/dbc/dbc.cc b/bridge/dbc/dbc.cc index 64297a13..510e726b 100644 --- a/bridge/dbc/dbc.cc +++ b/bridge/dbc/dbc.cc @@ -138,6 +138,14 @@ bool cabana::Signal::getValue(const uint8_t *data, size_t data_size, double *val return true; } +bool cabana::Signal::getRawBits(const uint8_t *data, size_t data_size, uint64_t *val) const { + if (multiplexor && get_raw_value(data, data_size, *multiplexor) != multiplex_value) { + return false; + } + *val = get_raw_bits(data, data_size, *this); + return true; +} + bool cabana::Signal::operator==(const cabana::Signal &other) const { return name == other.name && size == other.size && start_bit == other.start_bit && @@ -150,7 +158,7 @@ bool cabana::Signal::operator==(const cabana::Signal &other) const { // helper functions -double get_raw_value(const uint8_t *data, size_t data_size, const cabana::Signal &sig) { +uint64_t get_raw_bits(const uint8_t *data, size_t data_size, const cabana::Signal &sig) { const int msb_byte = sig.msb / 8; if (msb_byte >= (int)data_size) return 0; @@ -175,6 +183,12 @@ double get_raw_value(const uint8_t *data, size_t data_size, const cabana::Signal } } + return val; +} + +double get_raw_value(const uint8_t *data, size_t data_size, const cabana::Signal &sig) { + uint64_t val = get_raw_bits(data, data_size, sig); + // Sign extension (if needed) if (sig.is_signed && (val & (1ULL << (sig.size - 1)))) { val |= ~((1ULL << sig.size) - 1); diff --git a/bridge/dbc/dbc.h b/bridge/dbc/dbc.h index d1db07f9..44b45519 100644 --- a/bridge/dbc/dbc.h +++ b/bridge/dbc/dbc.h @@ -75,6 +75,8 @@ class Signal { Signal(const Signal &other) = default; void update(); bool getValue(const uint8_t *data, size_t data_size, double *val) const; + // Unscaled signal bits, for signals wider than double's 53-bit mantissa. + bool getRawBits(const uint8_t *data, size_t data_size, uint64_t *val) const; bool operator==(const cabana::Signal &other) const; inline bool operator!=(const cabana::Signal &other) const { return !(*this == other); } @@ -132,5 +134,6 @@ class Msg { // Helper functions double get_raw_value(const uint8_t *data, size_t data_size, const cabana::Signal &sig); +uint64_t get_raw_bits(const uint8_t *data, size_t data_size, const cabana::Signal &sig); void updateMsbLsb(cabana::Signal &s); inline int flipBitPos(int start_bit) { return 8 * (start_bit / 8) + 7 - start_bit % 8; } diff --git a/dashpilot-android/.gitignore b/dashpilot-android/.gitignore index aa724b77..c7b17b4c 100644 --- a/dashpilot-android/.gitignore +++ b/dashpilot-android/.gitignore @@ -9,6 +9,7 @@ /.idea/assetWizardSettings.xml .DS_Store /build +bridge/build-host /captures .externalNativeBuild .cxx diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datamodel/dash/CarState.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datamodel/dash/CarState.kt index a1426aba..7dae0acc 100644 --- a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datamodel/dash/CarState.kt +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datamodel/dash/CarState.kt @@ -36,7 +36,10 @@ data class CarState( val experimentalMode: Boolean = false, val madsActive: Boolean = false, - val changingLane: Boolean = false + val changingLane: Boolean = false, + + // Empty until fully assembled from the CAN stream by the native mapper. + val vin: String = "" ) { fun toImperial(): CarState = copy( odometer = odometer * KM_TO_MILES, @@ -47,7 +50,7 @@ data class CarState( ) companion object { - const val FIELD_COUNT = 31 + const val FIELD_COUNT = 34 private const val KM_TO_MILES = 0.621371f private val MILES_COUNTRIES = setOf("US", "GB", "MM", "LR") diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/DashKitDataSource.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/DashKitDataSource.kt index 7ee6d17d..262de43e 100644 --- a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/DashKitDataSource.kt +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/datasource/DashKitDataSource.kt @@ -6,13 +6,9 @@ import android.bluetooth.BluetoothGattCharacteristic import android.util.Log import com.softwiredtech.dashpilot.datamodel.dash.CarState import com.softwiredtech.dashpilot.vehicle.CanFrameDecoder -import com.softwiredtech.dashpilot.vehicle.VehicleVinState import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.sample import java.util.UUID @@ -32,9 +28,6 @@ class DashKitDataSource( @OptIn(FlowPreview::class) override val incomingMessages: Flow = _incoming.sample(40) - private val _vinState = MutableStateFlow(VehicleVinState.Waiting) - val vinState: StateFlow = _vinState.asStateFlow() - private var currentState = CarState() override fun connect(address: String) { @@ -43,13 +36,10 @@ class DashKitDataSource( } override fun disconnect() { - resetVin() manager.removeGattListener(this) } override fun onServicesReady(gatt: BluetoothGatt) { - resetVin() - val service = gatt.getService(SERVICE_UUID) if (service == null) { Log.e(TAG, "CAN BLE service not found") @@ -73,21 +63,11 @@ class DashKitDataSource( parseAndEmit(value) } - override fun onDisconnected() = resetVin() - private fun parseAndEmit(payload: ByteArray) { if (payload.isEmpty()) return for (frame in parseCanPacket(payload)) { currentState = decoder.decodeFrame(frame.bus, frame.address, frame.data) } - // decodeFrame drives the native VIN assembler - _vinState.value = decoder.getVin()?.let { VehicleVinState.Available(it) } - ?: VehicleVinState.Waiting _incoming.tryEmit(currentState) } - - private fun resetVin() { - decoder.resetVin() - _vinState.value = VehicleVinState.Waiting - } } diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/CanFrameDecoder.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/CanFrameDecoder.kt index 7b1f073d..bc5d85c2 100644 --- a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/CanFrameDecoder.kt +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/CanFrameDecoder.kt @@ -18,13 +18,6 @@ class CanFrameDecoder( return arrayToCarState(values) } - // VIN assembled natively during decodeFrame; null until complete. - fun getVin(): String? = bridge.nativeGetVin(decoderHandle) - - fun resetVin() { - bridge.nativeResetVin(decoderHandle) - } - fun destroy() { bridge.nativeDestroyVehicleDecoder(decoderHandle) } @@ -64,8 +57,25 @@ class CanFrameDecoder( experimentalMode = values[27] > 0, madsActive = values[28] > 0, changingLane = values[29] > 0, - acTemp = values[30].toFloat() + acTemp = values[30].toFloat(), + vin = decodeVin(values, offset = 31) ) } + + // VIN chars arrive bit-cast into doubles, 8 ASCII bytes per double, + // little-endian; see CarState::toArray in bridge/car/car_state.h. + private fun decodeVin(values: DoubleArray, offset: Int): String { + val bytes = ByteArray(VIN_DOUBLE_COUNT * 8) + for (chunk in 0 until VIN_DOUBLE_COUNT) { + val bits = values[offset + chunk].toRawBits() + for (b in 0 until 8) { + bytes[chunk * 8 + b] = (bits ushr (8 * b)).toByte() + } + } + val length = bytes.indexOf(0).let { if (it < 0) bytes.size else it } + return String(bytes, 0, length, Charsets.US_ASCII) + } + + private const val VIN_DOUBLE_COUNT = 3 } } diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/VehicleVinState.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/VehicleVinState.kt deleted file mode 100644 index 00677ae9..00000000 --- a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/vehicle/VehicleVinState.kt +++ /dev/null @@ -1,7 +0,0 @@ -package com.softwiredtech.dashpilot.vehicle - -sealed interface VehicleVinState { - data object Waiting : VehicleVinState - data class Available(val vin: String) : VehicleVinState - data object Invalid : VehicleVinState -} diff --git a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/viewmodel/ConnectionViewModel.kt b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/viewmodel/ConnectionViewModel.kt index 70c2210f..787e2920 100644 --- a/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/viewmodel/ConnectionViewModel.kt +++ b/dashpilot-android/app/src/main/java/com/softwiredtech/dashpilot/viewmodel/ConnectionViewModel.kt @@ -60,7 +60,6 @@ import com.softwiredtech.dashpilot.jni.VehicleBridge import com.softwiredtech.dashpilot.util.NetworkUtil import com.softwiredtech.dashpilot.vehicle.CanFrameDecoder import com.softwiredtech.dashpilot.vehicle.VehicleProfileLoader -import com.softwiredtech.dashpilot.vehicle.VehicleVinState import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.delay @@ -279,9 +278,6 @@ class ConnectionViewModel(private var networkUtil: NetworkUtil) : ViewModel() { private val _dashState = MutableStateFlow?>(null) val dashState = _dashState.asStateFlow() - private val _vehicleVin = MutableStateFlow(VehicleVinState.Waiting) - val vehicleVin: StateFlow = _vehicleVin.asStateFlow() - private fun phoneBatteryFlow(context: Context): Flow = flow { val batteryManager = context.getSystemService(Context.BATTERY_SERVICE) as BatteryManager while (true) { @@ -348,13 +344,7 @@ class ConnectionViewModel(private var networkUtil: NetworkUtil) : ViewModel() { } } val decoder = CanFrameDecoder(bridge, profile) - val ds = DashKitDataSource(manager, decoder) - launch { - ds.vinState.collect { - if (_dataSource.value === ds) _vehicleVin.value = it - } - } - ds + DashKitDataSource(manager, decoder) } DataSourceType.WEBSOCKET -> WebsocketDataSource() else -> CommaDataSource(bridge, profile) @@ -452,7 +442,6 @@ class ConnectionViewModel(private var networkUtil: NetworkUtil) : ViewModel() { _bleManager.value?.disconnect() _bleManager.value = null _dashState.value = null - _vehicleVin.value = VehicleVinState.Waiting _hasAutoNavigatedToDashboard.value = false } diff --git a/dashpilot-android/bridge/CMakeLists.txt b/dashpilot-android/bridge/CMakeLists.txt index 12c15fa8..271958d2 100644 --- a/dashpilot-android/bridge/CMakeLists.txt +++ b/dashpilot-android/bridge/CMakeLists.txt @@ -74,11 +74,15 @@ target_link_libraries(bridge log ) else() -# Host-side unit tests for the header-only native logic (not built for Android). +# Host-side unit tests for the native decode logic (not built for Android). # Build & run: cmake -B build-host -S . && cmake --build build-host && ctest --test-dir build-host enable_testing() -add_executable(vin_assembler_test tests/vin_assembler_test.cpp) -target_include_directories(vin_assembler_test PRIVATE ${MSGQ_ROOT}) -target_compile_options(vin_assembler_test PRIVATE -Wall -Wextra) -add_test(NAME vin_assembler COMMAND vin_assembler_test) +add_executable(tesla_vin_mapper_test + tests/tesla_vin_mapper_test.cpp + ${MSGQ_ROOT}/dbc/dbc.cc + ${MSGQ_ROOT}/dbc/dbcfile.cc +) +target_include_directories(tesla_vin_mapper_test PRIVATE ${MSGQ_ROOT}) +target_compile_options(tesla_vin_mapper_test PRIVATE -Wall -Wextra) +add_test(NAME tesla_vin_mapper COMMAND tesla_vin_mapper_test) endif() diff --git a/dashpilot-android/bridge/src/main/cpp/jni_bridge.cpp b/dashpilot-android/bridge/src/main/cpp/jni_bridge.cpp index c156e981..9ed7cf94 100644 --- a/dashpilot-android/bridge/src/main/cpp/jni_bridge.cpp +++ b/dashpilot-android/bridge/src/main/cpp/jni_bridge.cpp @@ -223,23 +223,6 @@ Java_com_softwiredtech_dashpilot_jni_VehicleBridge_nativeDestroyVehicleDecoder( bridge::destroyVehicleDecoder(reinterpret_cast(decoderHandle)); } -JNIEXPORT jstring JNICALL -Java_com_softwiredtech_dashpilot_jni_VehicleBridge_nativeGetVin( - JNIEnv* env, jobject thiz, - jlong decoderHandle) { - auto* decoder = reinterpret_cast(decoderHandle); - if (!decoder || !decoder->vinReady()) return nullptr; - return env->NewStringUTF(decoder->vin().c_str()); -} - -JNIEXPORT void JNICALL -Java_com_softwiredtech_dashpilot_jni_VehicleBridge_nativeResetVin( - JNIEnv* env, jobject thiz, - jlong decoderHandle) { - auto* decoder = reinterpret_cast(decoderHandle); - if (decoder) decoder->resetVin(); -} - // === Receive loop === JNIEXPORT void JNICALL Java_com_softwiredtech_dashpilot_jni_VehicleBridge_nativeStartReceiveLoop( diff --git a/dashpilot-android/bridge/src/main/java/com/softwiredtech/dashpilot/jni/VehicleBridge.kt b/dashpilot-android/bridge/src/main/java/com/softwiredtech/dashpilot/jni/VehicleBridge.kt index 73a34349..7426e192 100644 --- a/dashpilot-android/bridge/src/main/java/com/softwiredtech/dashpilot/jni/VehicleBridge.kt +++ b/dashpilot-android/bridge/src/main/java/com/softwiredtech/dashpilot/jni/VehicleBridge.kt @@ -46,8 +46,6 @@ class VehicleBridge { // VehicleDecoder external fun nativeCreateVehicleDecoder(dbcContents: Array, busIndices: IntArray, vehicleType: String): Long external fun nativeDecodeCanFrame(decoderHandle: Long, bus: Int, address: Int, data: ByteArray): DoubleArray - external fun nativeGetVin(decoderHandle: Long): String? - external fun nativeResetVin(decoderHandle: Long) external fun nativeDestroyVehicleDecoder(decoderHandle: Long) // Message diff --git a/dashpilot-android/bridge/tests/tesla_vin_mapper_test.cpp b/dashpilot-android/bridge/tests/tesla_vin_mapper_test.cpp new file mode 100644 index 00000000..d2226e94 --- /dev/null +++ b/dashpilot-android/bridge/tests/tesla_vin_mapper_test.cpp @@ -0,0 +1,165 @@ +// Host-run unit tests for the VIN path of the Tesla CarState mappers +// (repo/bridge/car/cars/tesla.h). Built by CMakeLists.txt when not configuring +// for Android: +// cmake -B build-host -S . && cmake --build build-host && ctest --test-dir build-host + +#include "car/cars/tesla.h" + +#include +#include +#include + +static int failures = 0; + +#define CHECK(cond) \ + do { \ + if (!(cond)) { \ + std::printf("FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + failures++; \ + } \ + } while (0) + +namespace { + +const char* kVin = "5YJ3E7EB1MF123456"; + +// VIN_info as defined in bus_1_tesla_vehicle.dbc. +const char* kVehicleDbc = + "BO_ 1029 VIN_info: 8 VEH\n" + " SG_ VIN_infoIndex M : 0|8@1+ (1,0) [0|255] \"\" X\n" + " SG_ VIN_B405 m17 : 8|56@1+ (1,0) [0|7.2057594038E+016] \"\" X\n" + " SG_ VIN_C405 m18 : 8|56@1+ (1,0) [0|7.2057594038E+016] \"\" X\n" + " SG_ VIN_A405 m16 : 8|56@1+ (1,0) [0|7.2057594038E+016] \"\" X\n"; + +// Builds an 8-byte VIN_info frame: byte 0 = mux, text written at offset, rest zero. +std::string vinFrame(uint8_t mux, const std::string& text, size_t offset) { + std::string frame(8, '\0'); + frame[0] = static_cast(mux); + std::memcpy(&frame[offset], text.data(), text.size()); + return frame; +} + +std::string frameA() { return vinFrame(0x10, std::string(kVin, 0, 3), 5); } +std::string frameB() { return vinFrame(0x11, std::string(kVin, 3, 7), 1); } +std::string frameC() { return vinFrame(0x12, std::string(kVin, 10, 7), 1); } + +struct Harness { + CANParsers cp; + TeslaDashKitMapper mapper; + CarState cs; + + Harness() { + cp.addBus(1, kVehicleDbc); + cp.buildCache(); + } + + void feed(const std::string& data, int bus = 1, uint32_t address = 0x405) { + cp.updateFrame(bus, address, reinterpret_cast(data.data()), + data.size()); + mapper.update(cp, cs); + } + + std::string vin() const { return cs.vin; } +}; + +void assembles_out_of_order_frames_with_duplicates() { + Harness h; + h.feed(frameC()); + CHECK(h.vin().empty()); + h.feed(frameA()); + h.feed(frameA()); // duplicate is tolerated + CHECK(h.vin().empty()); + h.feed(frameB()); + CHECK(h.vin() == kVin); +} + +void completion_persists_across_further_frames() { + Harness h; + h.feed(frameB()); + h.feed(frameC()); + h.feed(frameA()); + CHECK(h.vin() == kVin); + h.feed(vinFrame(0x11, "JUNKJUN", 1)); + CHECK(h.vin() == kVin); +} + +void rejects_illegal_vin_characters_until_retransmission() { + const char* illegal[] = {"I", "O", "Q", "a", "\x00", "\xC4"}; + for (const char* c : illegal) { + Harness h; + std::string bad = frameC(); + bad[1] = c[0]; + h.feed(frameA()); + h.feed(frameB()); + h.feed(bad); + CHECK(h.vin().empty()); + + // A valid retransmission of the segment completes the VIN. + h.feed(frameC()); + CHECK(h.vin() == kVin); + } +} + +void rejects_zero_padding_violation_on_mux_a() { + Harness h; + std::string frame = frameA(); + frame[2] = 'X'; // bytes 1-4 must be zero for mux A + h.feed(frame); + h.feed(frameB()); + h.feed(frameC()); + CHECK(h.vin().empty()); + h.feed(frameA()); + CHECK(h.vin() == kVin); +} + +void ignores_frames_outside_the_vin_contract() { + Harness h; + h.feed(frameA(), 0, 0x405); // wrong bus + h.feed(frameB(), 1, 0x404); // wrong CAN id + h.feed(vinFrame(0x20, "JUNKJUN", 1)); // unknown mux + std::string short_frame = frameC(); + short_frame.resize(6); // truncated DLC + h.feed(short_frame); + CHECK(h.vin().empty()); +} + +void vin_round_trips_through_the_double_array() { + Harness h; + h.feed(frameA()); + h.feed(frameB()); + h.feed(frameC()); + + double out[CarState::FIELD_COUNT]; + h.cs.toArray(out); + char decoded[CarState::VIN_DOUBLE_COUNT * sizeof(double) + 1] = {}; + std::memcpy(decoded, out + 31, CarState::VIN_DOUBLE_COUNT * sizeof(double)); + CHECK(std::string(decoded) == kVin); +} + +void empty_vin_encodes_as_zero_doubles() { + CarState cs; + double out[CarState::FIELD_COUNT]; + cs.toArray(out); + for (size_t i = 31; i < CarState::FIELD_COUNT; i++) { + CHECK(out[i] == 0.0); + } +} + +} // namespace + +int main() { + assembles_out_of_order_frames_with_duplicates(); + completion_persists_across_further_frames(); + rejects_illegal_vin_characters_until_retransmission(); + rejects_zero_padding_violation_on_mux_a(); + ignores_frames_outside_the_vin_contract(); + vin_round_trips_through_the_double_array(); + empty_vin_encodes_as_zero_doubles(); + + if (failures > 0) { + std::printf("%d check(s) failed\n", failures); + return 1; + } + std::printf("all tesla_vin_mapper tests passed\n"); + return 0; +} diff --git a/dashpilot-android/bridge/tests/vin_assembler_test.cpp b/dashpilot-android/bridge/tests/vin_assembler_test.cpp deleted file mode 100644 index 719cc850..00000000 --- a/dashpilot-android/bridge/tests/vin_assembler_test.cpp +++ /dev/null @@ -1,132 +0,0 @@ -// Host-run unit tests for VinAssembler (repo/bridge/car/vin_assembler.h). -// Built by CMakeLists.txt when not configuring for Android: -// cmake -B build-host -S . && cmake --build build-host && ctest --test-dir build-host - -#include "car/vin_assembler.h" - -#include -#include -#include - -static int failures = 0; - -#define CHECK(cond) \ - do { \ - if (!(cond)) { \ - std::printf("FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ - failures++; \ - } \ - } while (0) - -namespace { - -const char* kVin = "5YJ3E7EB1MF123456"; - -using State = VinAssembler::State; - -// Builds an 8-byte frame: byte 0 = mux, text written at offset, rest zero. -std::string vinFrame(uint8_t mux, const char* text, size_t offset) { - std::string frame(VinAssembler::kFrameLen, '\0'); - frame[0] = static_cast(mux); - std::memcpy(frame.data() + offset, text, std::strlen(text)); - return frame; -} - -std::string frameA() { return vinFrame(0x10, std::string(kVin, 0, 3).c_str(), 5); } -std::string frameB() { return vinFrame(0x11, std::string(kVin, 3, 7).c_str(), 1); } -std::string frameC() { return vinFrame(0x12, std::string(kVin, 10, 7).c_str(), 1); } - -State feed(VinAssembler& a, int bus, uint32_t address, const std::string& data) { - return a.onFrame(bus, address, reinterpret_cast(data.data()), - data.size()); -} - -void assembles_out_of_order_frames_with_duplicates() { - VinAssembler a; - CHECK(feed(a, 1, 0x405, frameC()) == State::Waiting); - CHECK(feed(a, 1, 0x405, frameA()) == State::Waiting); - CHECK(feed(a, 1, 0x405, frameA()) == State::Waiting); // duplicate is tolerated - CHECK(feed(a, 1, 0x405, frameB()) == State::Ready); - CHECK(a.vin() == kVin); -} - -void completion_persists_until_reset() { - VinAssembler a; - feed(a, 1, 0x405, frameB()); - feed(a, 1, 0x405, frameC()); - CHECK(feed(a, 1, 0x405, frameA()) == State::Ready); - // Subsequent unrelated frames keep reporting the completed VIN. - CHECK(feed(a, 1, 0x405, vinFrame(0x13, "junk", 1)) == State::Ready); - CHECK(a.vin() == kVin); - - a.reset(); - CHECK(!a.ready()); - CHECK(a.vin().empty()); - CHECK(feed(a, 1, 0x405, frameC()) == State::Waiting); - CHECK(a.vin().empty()); -} - -void rejects_illegal_vin_characters() { - const char* illegal[] = {"I", "O", "Q", "a", "\xC3\x84"}; - for (const char* c : illegal) { - VinAssembler a; - std::string frame = frameC(); - frame.replace(1, std::strlen(c), c); - feed(a, 1, 0x405, frameA()); - feed(a, 1, 0x405, frameB()); - CHECK(feed(a, 1, 0x405, frame) == State::Invalid); - CHECK(!a.ready()); - - // Invalid isn't latched: a valid retransmission of the segment completes. - CHECK(feed(a, 1, 0x405, frameC()) == State::Ready); - CHECK(a.vin() == kVin); - } -} - -void rejects_zero_padding_violation_on_mux_a() { - VinAssembler a; - std::string frame = frameA(); - frame[2] = 'X'; // bytes 1-4 must be zero for mux A - CHECK(feed(a, 1, 0x405, frame) == State::Invalid); - CHECK(!a.ready()); -} - -void ignores_frames_outside_the_vin_contract() { - VinAssembler a; - // Wrong bus and wrong CAN id. - CHECK(feed(a, 0, 0x405, frameA()) == State::Waiting); - CHECK(feed(a, 1, 0x404, frameA()) == State::Waiting); - // Wrong DLC. - std::string short_frame = frameA(); - short_frame.resize(6); - CHECK(feed(a, 1, 0x405, short_frame) == State::Waiting); - // Unknown mux. - CHECK(feed(a, 1, 0x405, vinFrame(0x20, "junk", 1)) == State::Waiting); - CHECK(!a.ready()); -} - -void assembles_in_order_from_single_frames() { - VinAssembler a; - CHECK(feed(a, 1, 0x405, frameA()) == State::Waiting); - CHECK(feed(a, 1, 0x405, frameB()) == State::Waiting); - CHECK(feed(a, 1, 0x405, frameC()) == State::Ready); - CHECK(a.vin() == kVin); -} - -} // namespace - -int main() { - assembles_out_of_order_frames_with_duplicates(); - completion_persists_until_reset(); - rejects_illegal_vin_characters(); - rejects_zero_padding_violation_on_mux_a(); - ignores_frames_outside_the_vin_contract(); - assembles_in_order_from_single_frames(); - - if (failures > 0) { - std::printf("%d check(s) failed\n", failures); - return 1; - } - std::printf("all vin_assembler tests passed\n"); - return 0; -}