From f45993ede2c4724c6372d205a09b3e10cdc1c84c Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:08:55 -0500 Subject: [PATCH] feat(desktop): implement DI auto-wiring and validation (#4782) Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com> --- .../desktop_di_autowiring_20260313/index.md | 5 ++ .../metadata.json | 8 ++++ .../desktop_di_autowiring_20260313/plan.md | 16 +++++++ .../desktop_di_autowiring_20260313/spec.md | 25 ++++++++++ .../desktop/di/DesktopKoinModule.kt | 10 +++- .../meshtastic/desktop/stub/CompassStubs.kt | 38 +++++++++++++++ .../org/meshtastic/desktop/stub/NoopStubs.kt | 2 +- .../meshtastic/desktop/di/DesktopKoinTest.kt | 47 +++++++++++++++++++ docs/archive/kmp-migration.md | 2 +- docs/decisions/architecture-review-2026-03.md | 29 ++++-------- docs/roadmap.md | 6 +-- 11 files changed, 160 insertions(+), 28 deletions(-) create mode 100644 conductor/archive/desktop_di_autowiring_20260313/index.md create mode 100644 conductor/archive/desktop_di_autowiring_20260313/metadata.json create mode 100644 conductor/archive/desktop_di_autowiring_20260313/plan.md create mode 100644 conductor/archive/desktop_di_autowiring_20260313/spec.md create mode 100644 desktop/src/main/kotlin/org/meshtastic/desktop/stub/CompassStubs.kt create mode 100644 desktop/src/test/kotlin/org/meshtastic/desktop/di/DesktopKoinTest.kt diff --git a/conductor/archive/desktop_di_autowiring_20260313/index.md b/conductor/archive/desktop_di_autowiring_20260313/index.md new file mode 100644 index 000000000..1bc0ce56b --- /dev/null +++ b/conductor/archive/desktop_di_autowiring_20260313/index.md @@ -0,0 +1,5 @@ +# Track desktop_di_autowiring_20260313 Context + +- [Specification](./spec.md) +- [Implementation Plan](./plan.md) +- [Metadata](./metadata.json) \ No newline at end of file diff --git a/conductor/archive/desktop_di_autowiring_20260313/metadata.json b/conductor/archive/desktop_di_autowiring_20260313/metadata.json new file mode 100644 index 000000000..7ea36cf65 --- /dev/null +++ b/conductor/archive/desktop_di_autowiring_20260313/metadata.json @@ -0,0 +1,8 @@ +{ + "track_id": "desktop_di_autowiring_20260313", + "type": "chore", + "status": "new", + "created_at": "2026-03-13T12:00:00Z", + "updated_at": "2026-03-13T12:00:00Z", + "description": "Architecture Health & DI (Immediate Priority) * Desktop Koin checkModules() test: Add a test to ensure Desktop DI bindings are validated at compile-time/test-time so we catch missing interfaces early. * Auto-wire Desktop ViewModels: Configure KSP so we can eliminate the manual ViewModel wiring in DesktopKoinModule and rely on @KoinViewModel annotations like Android does." +} \ No newline at end of file diff --git a/conductor/archive/desktop_di_autowiring_20260313/plan.md b/conductor/archive/desktop_di_autowiring_20260313/plan.md new file mode 100644 index 000000000..b5d55c6ed --- /dev/null +++ b/conductor/archive/desktop_di_autowiring_20260313/plan.md @@ -0,0 +1,16 @@ +# Implementation Plan: Desktop DI Auto-Wiring and Validation + +## Phase 1: Setup KSP for Desktop and Test Scaffolding +- [x] Task: Update the `meshtastic.koin` convention plugin (or equivalent `build-logic` files) to apply KSP to the `jvmMain` (Desktop) target for `@KoinViewModel` auto-wiring. +- [x] Task: Write Failing Test: Create `DesktopKoinTest.kt` in `desktop/src/test/kotlin/org/meshtastic/desktop/di/` using `kotlin.test`. + - [x] Initialize Koin application. + - [x] Include `desktopModule()`, `desktopPlatformModule()`, and `desktopPlatformStubsModule()`. + - [x] Call `checkModules()` inside the test and ensure it fails if there are missing interfaces. +- [x] Task: Implement to Pass Tests: Add any missing stubs or correct module includes in `desktopPlatformStubsModule()` to ensure the basic Koin graph resolves. +- [x] Task: Conductor - User Manual Verification 'Phase 1: Setup KSP for Desktop and Test Scaffolding' (Protocol in workflow.md) + +## Phase 2: Auto-wire ViewModels and Clean Up +- [x] Task: Refactor: Remove manual `viewModel { ... }` blocks from `DesktopKoinModule.kt` (if any are present). +- [x] Task: Implement: Ensure the desktop build configuration (`desktop/build.gradle.kts`) correctly includes the KSP-generated Koin modules and that KSP targets the JVM platform. +- [x] Task: Implement to Pass Tests: Verify that running `./gradlew :desktop:test` succeeds and that `DesktopKoinTest.kt` validates the new KSP-wired graph. +- [x] Task: Conductor - User Manual Verification 'Phase 2: Auto-wire ViewModels and Clean Up' (Protocol in workflow.md) \ No newline at end of file diff --git a/conductor/archive/desktop_di_autowiring_20260313/spec.md b/conductor/archive/desktop_di_autowiring_20260313/spec.md new file mode 100644 index 000000000..5c91bb14a --- /dev/null +++ b/conductor/archive/desktop_di_autowiring_20260313/spec.md @@ -0,0 +1,25 @@ +# Specification: Desktop DI Auto-Wiring and Validation + +## Overview +This track addresses immediate architecture health priorities for the Desktop KMP target: +1. **Desktop Koin `checkModules()` test:** Add a compile-time/test-time validation test to ensure Desktop DI bindings resolve correctly and catch missing interfaces early. +2. **Auto-wire Desktop ViewModels:** Configure KSP to generate Koin modules for ViewModels annotated with `@KoinViewModel` in the JVM target, eliminating the need for manual ViewModel wiring in `DesktopKoinModule`. + +## Functional Requirements +- **KSP Configuration:** Update the `meshtastic.koin` (or equivalent) convention plugin to apply KSP and Koin annotations processing to the `jvmMain` (Desktop) target. +- **ViewModel Auto-Wiring:** Remove all manual `viewModel { ... }` definitions in `DesktopKoinModule` and ensure they are successfully replaced by the KSP-generated Koin modules. +- **DI Validation Test:** Implement a new test file (e.g., `DesktopKoinTest.kt`) in `desktop/src/test/kotlin/org/meshtastic/desktop/di/` using `kotlin.test`. +- **Test Scope:** The `checkModules()` test must include and validate all active Desktop Koin modules, including `desktopModule()`, `desktopPlatformModule()`, `desktopPlatformStubsModule()`, and any KSP-generated modules. + +## Non-Functional Requirements +- **Build Performance:** The addition of KSP to the JVM target should not unnecessarily degrade build times. Cacheability must be maintained. +- **Style:** Adhere strictly to the project's existing Kotlin code style and Koin best practices. + +## Acceptance Criteria +- [ ] Running `./gradlew :desktop:test` executes the new `checkModules()` test successfully. +- [ ] No manual ViewModel definitions remain in `DesktopKoinModule` for shared ViewModels (they are auto-wired). +- [ ] If a dependency is missing from the Desktop DI graph, the `checkModules()` test fails explicitly. + +## Out of Scope +- Migrating other platforms (Android, iOS) DI implementations. +- Refactoring the internal logic of the ViewModels themselves. \ No newline at end of file diff --git a/desktop/src/main/kotlin/org/meshtastic/desktop/di/DesktopKoinModule.kt b/desktop/src/main/kotlin/org/meshtastic/desktop/di/DesktopKoinModule.kt index b7e5d668f..c4ba76edb 100644 --- a/desktop/src/main/kotlin/org/meshtastic/desktop/di/DesktopKoinModule.kt +++ b/desktop/src/main/kotlin/org/meshtastic/desktop/di/DesktopKoinModule.kt @@ -44,11 +44,14 @@ import org.meshtastic.core.repository.ServiceRepository import org.meshtastic.desktop.radio.DesktopMeshServiceController import org.meshtastic.desktop.radio.DesktopRadioInterfaceService import org.meshtastic.desktop.stub.NoopAppWidgetUpdater +import org.meshtastic.desktop.stub.NoopCompassHeadingProvider import org.meshtastic.desktop.stub.NoopLocationRepository import org.meshtastic.desktop.stub.NoopMQTTRepository +import org.meshtastic.desktop.stub.NoopMagneticFieldProvider import org.meshtastic.desktop.stub.NoopMeshLocationManager import org.meshtastic.desktop.stub.NoopMeshServiceNotifications import org.meshtastic.desktop.stub.NoopMeshWorkerManager +import org.meshtastic.desktop.stub.NoopPhoneLocationProvider import org.meshtastic.desktop.stub.NoopPlatformAnalytics import org.meshtastic.desktop.stub.NoopServiceBroadcasts import org.meshtastic.core.common.di.module as coreCommonModule @@ -71,7 +74,7 @@ import org.meshtastic.feature.settings.di.module as featureSettingsModule /** * Koin module for the Desktop target. * - * Includes the generated KSP modules from core KMP libraries (which provide real implementations of prefs, data + * Includes the generated Koin K2 modules from core KMP libraries (which provide real implementations of prefs, data * repositories, managers, datastore data sources, use cases, and ViewModels from `commonMain`). * * Only truly platform-specific interfaces are stubbed here — things that require Android APIs (BLE/USB transport, @@ -80,7 +83,7 @@ import org.meshtastic.feature.settings.di.module as featureSettingsModule * Platform infrastructure (DataStores, Room database, Lifecycle) is provided by [desktopPlatformModule]. */ fun desktopModule() = module { - // Include generated KSP modules from core KMP libraries (commonMain implementations) + // Include generated Koin K2 modules from core KMP libraries (commonMain implementations) includes( org.meshtastic.core.di.di.CoreDiModule().coreDiModule(), org.meshtastic.core.common.di.CoreCommonModule().coreCommonModule(), @@ -131,6 +134,9 @@ private fun desktopPlatformStubsModule() = module { single { NoopMeshLocationManager() } single { NoopLocationRepository() } single { NoopMQTTRepository() } + single { NoopCompassHeadingProvider() } + single { NoopPhoneLocationProvider() } + single { NoopMagneticFieldProvider() } // Desktop mesh service controller — replaces Android's MeshService lifecycle single { diff --git a/desktop/src/main/kotlin/org/meshtastic/desktop/stub/CompassStubs.kt b/desktop/src/main/kotlin/org/meshtastic/desktop/stub/CompassStubs.kt new file mode 100644 index 000000000..5e223ed67 --- /dev/null +++ b/desktop/src/main/kotlin/org/meshtastic/desktop/stub/CompassStubs.kt @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.desktop.stub + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf +import org.meshtastic.feature.node.compass.CompassHeadingProvider +import org.meshtastic.feature.node.compass.HeadingState +import org.meshtastic.feature.node.compass.MagneticFieldProvider +import org.meshtastic.feature.node.compass.PhoneLocationProvider +import org.meshtastic.feature.node.compass.PhoneLocationState + +class NoopCompassHeadingProvider : CompassHeadingProvider { + override fun headingUpdates(): Flow = flowOf(HeadingState(hasSensor = false)) +} + +class NoopPhoneLocationProvider : PhoneLocationProvider { + override fun locationUpdates(): Flow = + flowOf(PhoneLocationState(permissionGranted = false, providerEnabled = false)) +} + +class NoopMagneticFieldProvider : MagneticFieldProvider { + override fun getDeclination(latitude: Double, longitude: Double, altitude: Double, timeMillis: Long): Float = 0f +} diff --git a/desktop/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt b/desktop/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt index c777204b8..e4b12d6e8 100644 --- a/desktop/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt +++ b/desktop/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt @@ -52,7 +52,7 @@ import org.meshtastic.proto.Position as ProtoPosition * * These stubs exist ONLY for interfaces that have no `commonMain` implementation and require Android-specific APIs * (BLE/USB transport, notifications, WorkManager, location services, broadcasts, widgets). All other interfaces use - * real `commonMain` implementations wired through the generated KSP Koin modules. + * real `commonMain` implementations wired through the generated Koin K2 modules. * * As real desktop implementations become available (e.g., serial transport, TCP transport), they replace individual * stubs in [desktopModule]. diff --git a/desktop/src/test/kotlin/org/meshtastic/desktop/di/DesktopKoinTest.kt b/desktop/src/test/kotlin/org/meshtastic/desktop/di/DesktopKoinTest.kt new file mode 100644 index 000000000..b1136e71a --- /dev/null +++ b/desktop/src/test/kotlin/org/meshtastic/desktop/di/DesktopKoinTest.kt @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.desktop.di + +import androidx.lifecycle.SavedStateHandle +import io.ktor.client.HttpClient +import io.ktor.client.engine.HttpClientEngine +import kotlinx.coroutines.CoroutineDispatcher +import org.koin.core.annotation.KoinExperimentalAPI +import org.koin.dsl.module +import org.koin.test.verify.verify +import kotlin.test.Test + +@OptIn(KoinExperimentalAPI::class) +class DesktopKoinTest { + + @Test + fun `verify desktop koin modules`() { + // This test validates the full Koin DI graph for the Desktop target. + // It includes the main desktopModule (repositories, use cases, ViewModels, stubs) + // and the desktopPlatformModule (DataStores, Room database, lifecycle). + module { includes(desktopModule(), desktopPlatformModule()) } + .verify( + extraTypes = + listOf( + SavedStateHandle::class, + CoroutineDispatcher::class, + HttpClient::class, + HttpClientEngine::class, + ), + ) + } +} diff --git a/docs/archive/kmp-migration.md b/docs/archive/kmp-migration.md index 6e6c13b64..55f5ae1ee 100644 --- a/docs/archive/kmp-migration.md +++ b/docs/archive/kmp-migration.md @@ -76,7 +76,7 @@ When contributing to `core` modules, adhere to the following KMP standards: * **Resources:** Use Compose Multiplatform Resources (`core:resources`) for all strings and drawables. Never use Android `strings.xml` in `commonMain`. * **Coroutines & Flows:** Use `StateFlow` and `SharedFlow` for all asynchronous state management across the domain layer. * **Persistence:** Use `androidx.datastore` for preferences and Room KMP for complex relational data. -* **Dependency Injection:** We use **Koin Annotations + KSP**. Per 2026 KMP industry standards, it is recommended to push Koin `@Module`, `@ComponentScan`, and `@KoinViewModel` annotations into `commonMain`. This encapsulates dependency graphs per feature, providing a Hilt-like experience (compile-time validation) while remaining fully multiplatform-compatible. +* **Dependency Injection:** We use **Koin Annotations + K2 Compiler Plugin**. Per 2026 KMP industry standards, it is recommended to push Koin `@Module`, `@ComponentScan`, and `@KoinViewModel` annotations into `commonMain`. This encapsulates dependency graphs per feature, providing a Hilt-like experience (compile-time validation) while remaining fully multiplatform-compatible. --- *Document refreshed on 2026-03-10 as a historical companion to `docs/kmp-progress-review-2026.md`.* diff --git a/docs/decisions/architecture-review-2026-03.md b/docs/decisions/architecture-review-2026-03.md index b4d25df15..fbad97ebd 100644 --- a/docs/decisions/architecture-review-2026-03.md +++ b/docs/decisions/architecture-review-2026-03.md @@ -128,27 +128,15 @@ Vico chart screens (DeviceMetrics, EnvironmentMetrics, SignalMetrics, PowerMetri ## C. DI Improvements -### C1. Desktop manual ViewModel wiring +### C1. ~~Desktop manual ViewModel wiring~~ *(resolved 2026-03-13)* -`DesktopKoinModule.kt` has ~120 lines of hand-written `viewModel { Constructor(get(), get(), ...) }` with 8–17 parameters each. These will drift from the annotation-generated Android wiring. +`DesktopKoinModule.kt` originally had ~120 lines of hand-written `viewModel { ... }` blocks. These have been successfully replaced by including Koin modules from `commonMain` generated via the Koin K2 Compiler Plugin for automatic wiring. -**Fix:** Ensure `@KoinViewModel` annotations on shared ViewModels in `feature/*/commonMain` generate KSP modules for the JVM target. Desktop's `desktopModule()` should then `includes()` generated modules — zero manual ViewModel wiring. +### C2. ~~Desktop stubs lack compile-time validation~~ *(resolved 2026-03-13)* -**Validation:** If KSP already processes JVM targets (check `meshtastic.koin` convention plugin), this may only need import wiring. If not, configure `ksp(libs.koin.annotations)` for the JVM source set. +`desktopPlatformStubsModule()` previously had stubs that were only validated at runtime. -### C2. Desktop stubs lack compile-time validation - -`desktopPlatformStubsModule()` has 12 `single { Noop() }` bindings. Adding a new interface to `core:repository` won't cause a build failure — it fails at runtime. - -**Fix:** Add `checkModules()` test: -```kotlin -@Test fun `all Koin bindings resolve`() { - koinApplication { - modules(desktopModule(), desktopPlatformModule()) - checkModules() - } -} -``` +**Outcome:** Added `DesktopKoinTest.kt` using Koin's `verify()` API. This test validates the entire Desktop DI graph (including platform stubs and DataStores) during the build. Discovered and fixed missing stubs for `CompassHeadingProvider`, `PhoneLocationProvider`, and `MagneticFieldProvider`. ### C3. DI module naming convention @@ -187,10 +175,9 @@ Android uses `@Module`-annotated classes (`CoreDataModule`, `CoreBleAndroidModul - `core:ble` (connection state machine) - `core:ui` (utility functions) -### D4. Desktop has 5 tests +### D4. Desktop has 6 tests -`desktop/src/test/` contains `DemoScenarioTest.kt` with 5 test cases. Still needs: -- Koin module validation (`checkModules()`) +`desktop/src/test/` contains `DemoScenarioTest.kt` and `DesktopKoinTest.kt`. Still needs: - `DesktopRadioInterfaceService` connection state tests - Navigation graph coverage @@ -208,7 +195,7 @@ Ordered by impact × effort: | 4 | Feature `commonTest` (D1) | Medium | Medium | KMP test coverage | | 5 | `feature:connections` (A3) | High | Medium | ~~Desktop connections~~ ✅ Done | | 6 | Service/worker extraction from `app` (A1) | Medium | Medium | Thin app module | -| 7 | Desktop Koin auto-wiring (C1) | Medium | Low | DI parity | +| 7 | ~~Desktop Koin auto-wiring (C1, C2)~~ | Medium | Low | ✅ Resolved 2026-03-13 | | 8 | MQTT KMP (B3) | Medium | High | Desktop/iOS MQTT | | 9 | KMP charts (B4) | Medium | High | Desktop metrics | | 10 | iOS target declaration | High | Low | CI purity gate | diff --git a/docs/roadmap.md b/docs/roadmap.md index 6ae46165a..45161fa3e 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -14,8 +14,8 @@ These items address structural gaps identified in the March 2026 architecture re | Replace `ConcurrentHashMap` in `commonMain` (3 files) | High | Low | ✅ | | Create `core:testing` shared test fixtures | Medium | Low | ✅ | | Add feature module `commonTest` (settings, node, messaging) | Medium | Medium | ✅ | -| Desktop Koin `checkModules()` integration test | Medium | Low | ❌ | -| Auto-wire Desktop ViewModels via KSP (eliminate manual wiring) | Medium | Low | ❌ | +| Desktop Koin `checkModules()` integration test | Medium | Low | ✅ | +| Auto-wire Desktop ViewModels via K2 Compiler (eliminate manual wiring) | Medium | Low | ✅ | ## Active Work @@ -86,7 +86,7 @@ These items address structural gaps identified in the March 2026 architecture re 1. **App module thinning** — 63 files remaining (down from 90). Extracted ChannelViewModel, NodeMapViewModel, NodeContextMenu, EmptyDetailPlaceholder to shared modules. Remaining: extract service/worker/radio files from `app` to `core:service/androidMain` and `core:network/androidMain` 2. **Serial/USB transport** — direct radio connection on Desktop via jSerialComm 3. **MQTT transport** — cloud relay operation (KMP, benefits all targets) -4. **Desktop ViewModel auto-wiring** — ensure Koin KSP generates ViewModel modules for JVM target; eliminate manual wiring in `DesktopKoinModule` +4. **Desktop ViewModel auto-wiring** — ✅ Done: ensured Koin K2 Compiler Plugin generates ViewModel modules for JVM target; eliminated manual wiring in `DesktopKoinModule` 5. **KMP charting** — ✅ Done: Vico charts migrated to `feature:node/commonMain` using KMP artifacts; desktop wires them directly 6. **Navigation contract extraction** — ✅ Done: shared `TopLevelDestination` enum in `core:navigation`; icon mapping in `core:ui`; parity tests in place. Both shells derive from the same source of truth. 7. **Dependency stabilization** — track stable releases for CMP, Koin, Lifecycle, Nav3