Skip to content

refactor: Remove AIDL API and modernize service architecture#5586

Merged
jamesarich merged 38 commits into
release/2.8.0from
jamesarich/remove-aidl-api
Jun 4, 2026
Merged

refactor: Remove AIDL API and modernize service architecture#5586
jamesarich merged 38 commits into
release/2.8.0from
jamesarich/remove-aidl-api

Conversation

@jamesarich

@jamesarich jamesarich commented May 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Remove the deprecated AIDL API and all associated infrastructure, unlocking a comprehensive modernization of the radio communication architecture. This PR eliminates thousands of lines of legacy code and replaces it with a modern, testable, KMP-compatible architecture.

262 files changed: +3,978 / −6,537 (net −2,559 lines)

Before: AIDL-based Architecture

graph TD
    subgraph "Client Apps / UI"
        VM[ViewModels]
    end

    subgraph "core:api (AIDL)"
        AIDL[IMeshService.aidl]
        Binder[Bound Service Binder]
        BC[Broadcast Receivers]
    end

    subgraph "Service Layer"
        MS[MeshService<br/>God-class]
        MR[MeshRouter<br/>Service Locator]
    end

    subgraph "Handlers"
        FRP[FromRadioPacketHandler]
        MMP[MeshMessageProcessor]
        MDH[MeshDataHandler]
        MCH[MeshConfigHandler]
        NIH[NeighborInfoHandler]
        TRH[TracerouteHandler]
    end

    subgraph "Radio"
        DRC[DirectRadioControllerImpl<br/>~450 lines, 17 deps, 44 functions]
    end

    VM -->|"IPC / Binder"| AIDL
    AIDL --> Binder
    Binder --> MS
    MS -->|"broadcasts"| BC
    BC -->|"intents"| VM
    MS --> MR
    MR -->|"lazy getters"| FRP
    MR -->|"lazy getters"| MMP
    MR -->|"lazy getters"| MDH
    MR -->|"lazy getters"| MCH
    MR -->|"lazy getters"| NIH
    MR -->|"lazy getters"| TRH
    MS --> DRC

    style AIDL fill:#ff6b6b,stroke:#c92a2a
    style Binder fill:#ff6b6b,stroke:#c92a2a
    style BC fill:#ff6b6b,stroke:#c92a2a
    style MR fill:#ff6b6b,stroke:#c92a2a
    style MS fill:#ffa94d,stroke:#e67700
    style DRC fill:#ffa94d,stroke:#e67700
Loading

After: Modern KMP Architecture

graph TD
    subgraph "UI Layer"
        VM[ViewModels]
    end

    subgraph "core:repository — segregated interfaces"
        CMD["Command interfaces<br/>Admin · Messaging · Node · Query"]
        READ["Read providers<br/>ConnectionState · Traceroute · NeighborInfo"]
        WRITE["ServiceStateWriter<br/>(write-only)"]
    end

    subgraph "core:service / core:data"
        DRC[RadioControllerImpl<br/>delegates to 4 sub-controllers]
        SR[ServiceRepositoryImpl]
        CS[CommandSender]
        H[Packet handlers<br/>direct injection, no service locator]
    end

    VM -->|"suspend calls"| CMD
    VM -->|"observe StateFlow"| READ
    DRC -.implements.-> CMD
    SR -.implements.-> READ
    SR -.implements.-> WRITE
    DRC --> CS
    H -->|"set* / emit*"| WRITE
    CS -->|"sends to radio"| H
Loading

What was removed

  • core:api module — AIDL interfaces, bound service, broadcast-based command dispatch
  • ServiceBroadcasts / ServiceAction — intent-based command routing and status broadcasts
  • MeshActionHandler — folded into the radio controller
  • MeshRouter — service-locator facade with zero routing logic
  • Publishing infrastructure — JitPack/Maven publishing from all modules
  • Legacy patternsrunBlocking in UI paths, redundant compiler flags, stale kotlin-android plugin references, Parcelable plumbing in core:model

Architecture improvements

Radio controller redesign

  • Interface SegregationRadioController split into 4 focused sub-interfaces: AdminController, MessagingController, NodeController, QueryController
  • Composition over a God object — the ~450-line, 17-dependency controller is now RadioControllerImpl, a thin composition root that assembles four focused impls (AdminControllerImpl, MessagingControllerImpl, NodeControllerImpl, QueryControllerImpl) via Kotlin interface delegation; its constructor is unchanged so DI + the integration test are untouched
  • Direct suspend calls — replaced broadcast-based command dispatch with type-safe suspend functions (admin sends are fire-and-forget; the device is the source of truth)

ServiceRepository ISP decomposition

  • Interface SegregationServiceRepository decomposed into focused sub-interfaces: ConnectionStateProvider, TracerouteResponseProvider, NeighborInfoResponseProvider, ServiceStateWriter (ServiceRepository extends all for compatibility)
  • Read-side narrowing — ViewModels inject the minimal interface they need:
    • MessageViewModelMessagingController + ConnectionStateProvider
    • NodeListViewModelAdminController + ConnectionStateProvider
    • NodeDetailViewModelQueryController
    • ContactsViewModel, ConnectionsViewModel, LocalStatsWidgetStateProviderConnectionStateProvider
    • MetricsViewModelTracerouteResponseProvider
  • Write-side adoption — 9 handlers that only mutate state now inject ServiceStateWriter instead of the full repository (TracerouteHandlerImpl, NeighborInfoHandlerImpl, MeshMessageProcessorImpl, MeshConfigFlowManagerImpl, MeshConfigHandlerImpl, MeshDataHandlerImpl, FromRadioPacketHandlerImpl, MqttManagerImpl, MeshServiceOrchestrator); PacketHandlerImpl (read-only) injects ConnectionStateProvider. The genuinely mixed read+write holders (MeshConnectionManagerImpl, RadioControllerImpl) keep the full interface.
  • DI bindings — sub-interfaces registered in both Android (@Single(binds=[...])) and Desktop (single<>) Koin modules; verified by KoinVerificationTest (Android) and DesktopKoinTest

New abstractions

  • NodeAddress sealed class — type-safe node addressing (Local, Broadcast, ByNum, ById)
  • ContactKey value class — consolidated contact-key parsing (was duplicated in 6 places), with a nullable-channel accessor that preserves legacy-DM semantics
  • CommandSender — replaces stringly-typed sender identification
  • DeviceVersion @JvmInline value class — zero-overhead version comparison with pre-compiled regex
  • RequestTimer — shared request-timing utility extracted from NeighborInfo/Traceroute handlers
  • editSettings { } DSL (AdminEditScope) — replaces begin/commitEditSettings ceremony and removes destNum/packetId boilerplate

Behavioral improvements

  • Idempotent node opssetFavorite/setIgnored(Boolean) skip the radio command when state is unchanged (replacing toggles; mirrors the SDK and fixes a latent un-favorite bug in SendMessageUseCase)
  • Forced manual verification on contact import (security fix)
  • Hardened orchestrator — CAS loop in start() eliminates a race condition

SDK alignment

The interface shapes deliberately mirror meshtastic-sdk (AdminApi/TelemetryApi/RoutingApi, layered controllers) to ease a future migration. Two further SDK-alignment items — AdminResult<T> return types (R4) and a NodeId value class (R5) — were investigated and deferred to the SDK migration: R4 would reimplement the SDK's admin RPC engine and reverse the intentional fire-and-forget design; R5 is a ~350-site change that overlaps NodeAddress.ByNum. Both come naturally with the SDK swap.

Testing

  • New/expanded suites: RadioControllerImplTest, NodeAddressTest, CommandSenderImplTest, NeighborInfoHandlerImplTest, RequestTimerTest, ConnectionsViewModelTest
  • Strengthened assertions (replaced no-op atLeast(0) with exactly(0); added idempotency no-op assertions)
  • Koin graph verified on Android + desktop

Build

  • Unified JDK target to 21 across all modules
  • Removed publishing infrastructure and redundant OptimizeNonSkippingGroups compiler flag
  • Ktor log level set to INFO with HttpClient tag

Documentation

  • Architecture guide documents the RadioController composition + ServiceRepository ISP patterns
  • Refreshed module READMEs (core:service, core:repository, core:model); module dependency graphs regenerated
  • Session handover notes for future contributors

@github-actions github-actions Bot added build Build system changes refactor no functional changes repo Repository maintenance labels May 23, 2026
@jamesarich jamesarich marked this pull request as ready for review May 23, 2026 13:44
@jamesarich jamesarich changed the title refactor: remove ServiceBroadcasts + CommonParcelable infrastructure refactor: remove deprecated AIDL API and broadcast infrastructure May 23, 2026
@jamesarich jamesarich force-pushed the jamesarich/remove-aidl-api branch from 2777fe8 to 3fde9cc Compare May 23, 2026 13:55
@jamesarich jamesarich marked this pull request as draft May 23, 2026 13:57
@jamesarich jamesarich force-pushed the jamesarich/remove-aidl-api branch from 3fde9cc to b304b0d Compare May 23, 2026 14:00
@codecov

codecov Bot commented May 23, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
2493 1 2492 0
View the top 1 failed test(s) by shortest run time
org.meshtastic.core.database.DatabaseManagerWithDbRetryTest::withDb retries against current database when previous pool closes during switch
Stack Traces | 20.2s run time
java.lang.AssertionError: expected:<42424242> but was:<null>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:120)
	at kotlin.test.junit.JUnitAsserter.assertEquals(JUnitSupport.kt:32)
	at kotlin.test.AssertionsKt__AssertionsKt.assertEquals(Assertions.kt:63)
	at kotlin.test.AssertionsKt.assertEquals(Unknown Source)
	at kotlin.test.AssertionsKt__AssertionsKt.assertEquals$default(Assertions.kt:62)
	at kotlin.test.AssertionsKt.assertEquals$default(Unknown Source)
	at org.meshtastic.core.database.DatabaseManagerWithDbRetryTest$withDb retries against current database when previous pool closes during switch$1.invokeSuspend(DatabaseManagerWithDbRetryTest.kt:99)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:34)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:100)
	at kotlinx.coroutines.test.TestDispatcher.processEvent$kotlinx_coroutines_test(TestDispatcher.kt:24)
	at kotlinx.coroutines.test.TestCoroutineScheduler.tryRunNextTaskUnless$kotlinx_coroutines_test(TestCoroutineScheduler.kt:98)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTest$2$1$workRunner$1.invokeSuspend(TestBuilders.kt:326)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:34)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:100)
	at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:256)
	at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:54)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlockingImpl(Builders.kt:30)
	at kotlinx.coroutines.BuildersKt.runBlockingImpl(Unknown Source)
	at kotlinx.coroutines.BuildersKt__Builders_concurrentKt.runBlockingK(Builders.concurrent.kt:172)
	at kotlinx.coroutines.BuildersKt.runBlockingK(Unknown Source)
	at kotlinx.coroutines.BuildersKt__Builders_concurrentKt.runBlockingK$default(Builders.concurrent.kt:157)
	at kotlinx.coroutines.BuildersKt.runBlockingK$default(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersJvmKt.createTestResult(TestBuildersJvm.kt:10)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:309)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:1)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:167)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:1)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0$default(TestBuilders.kt:159)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0$default(TestBuilders.kt:1)
	at org.meshtastic.core.database.DatabaseManagerWithDbRetryTest.withDb retries against current database when previous pool closes during switch(DatabaseManagerWithDbRetryTest.kt:69)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:524)
	at org.robolectric.internal.SandboxTestRunner.executeInSandbox(SandboxTestRunner.java:494)
	at org.robolectric.internal.SandboxTestRunner.access$900(SandboxTestRunner.java:67)
	at org.robolectric.internal.SandboxTestRunner$7.evaluate(SandboxTestRunner.java:442)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.robolectric.internal.SandboxTestRunner.access$600(SandboxTestRunner.java:67)
	at org.robolectric.internal.SandboxTestRunner$6.evaluate(SandboxTestRunner.java:333)
	at org.robolectric.internal.SandboxTestRunner$3.evaluate(SandboxTestRunner.java:233)
	at org.robolectric.internal.SandboxTestRunner$5.lambda$evaluate$0(SandboxTestRunner.java:317)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:101)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

✅ Docs staleness check passed

This PR includes updates to docs/en/ alongside the source changes. Thank you!

@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

🖼️ Preview staleness check — advisory

This PR modifies UI composables but does not update any *Previews.kt files.

Previews power screenshot tests and in-app docs screenshots. Keeping them current ensures visual regression coverage stays accurate.

Changed UI files:

feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/component/MessageScreenComponents.kt
feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/component/Reaction.kt
feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/ContactItem.kt
feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/Contacts.kt
feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/ContactsViewModel.kt
feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/NodeDetailsSection.kt

What to check:

Pattern Preview file convention
feature/{name}/…/ui/ or component/ feature/{name}/…/*Previews.kt
core/ui/…/ core/ui/…/ (previews colocated)

Adding previews checklist:

  1. Create or update a *Previews.kt file in the same module with @PreviewLightDark
  2. Add @Suppress("PreviewPublic") if the preview is consumed by screenshot-tests
  3. Add corresponding @PreviewTest function in screenshot-tests/src/screenshotTest/
  4. Run ./gradlew :screenshot-tests:updateDebugScreenshotTest to generate reference images

If this PR does not require preview updates (e.g., logic-only change, non-visual refactor), add the skip-preview-check label to dismiss.

@jamesarich jamesarich force-pushed the jamesarich/remove-aidl-api branch from 51fff3c to 6f447c8 Compare May 23, 2026 18:47
@jamesarich jamesarich changed the title refactor: remove deprecated AIDL API and broadcast infrastructure refactor: remove AIDL API and modernize radio architecture May 23, 2026
@jamesarich jamesarich force-pushed the jamesarich/remove-aidl-api branch 4 times, most recently from 237f5e1 to 059ee97 Compare May 23, 2026 21:21
@jamesarich jamesarich changed the title refactor: remove AIDL API and modernize radio architecture refactor: Remove AIDL API and modernize service architecture May 26, 2026
@jamesarich jamesarich marked this pull request as ready for review May 26, 2026 20:36
@jamesarich jamesarich force-pushed the jamesarich/remove-aidl-api branch from f475045 to 66ae840 Compare May 28, 2026 02:18
jamesarich and others added 10 commits May 28, 2026 06:37
Remove the deprecated AIDL/IPC API surface and perform deep architectural
modernization of the radio command pipeline, aligning with the meshtastic-sdk
AdminApiImpl pattern for future SDK migration.

Key changes:

1. AIDL Removal & Infrastructure Cleanup
   - Delete core:api module and all AIDL interfaces
   - Remove ServiceBroadcasts + CommonParcelable infrastructure
   - Remove core:api from CI workflow lint/publish steps

2. Model Modernization
   - Introduce NodeAddress sealed class with type-safe addressing
   - Remove deprecated DataPacket constants in favor of NodeAddress
   - Consolidate dual node maps into single source with getNodeById
   - Split large model files, deduplicate NodeEntity, flatten RadioController

3. Service Layer Refactoring (SDK-aligned)
   - Remove ServiceAction sealed class, use direct suspend calls
   - Convert CommandSender & MeshActionHandler to suspend APIs
   - Merge MeshActionHandler into DirectRadioControllerImpl
     (ViewModel → RadioController → CommandSender, no intermediate layer)
   - Build AdminMessage protos directly with typed protos end-to-end
   - Apply structured concurrency to NodeRequestActions/NodeManagementActions
   - Fix CancellationException handling throughout

Architecture (before → after):
  ViewModel → RadioController → Handler → CommandSender → PacketHandler
  ViewModel → RadioController → CommandSender → PacketHandler

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- NodeAddressTest: 37 tests covering sealed class parser, roundtrip,
  extensions (ContactKey, DataPacket), edge cases
- CommandSenderImplTest: 20 tests covering packet ID generation,
  address resolution, sendData validation, admin messages, position
- DirectRadioControllerImplTest: +8 tests for reboot/shutdown/factory
  reset, importContact, refreshMetadata
- NodeManagerImplTest: +7 tests for getMyNodeInfo aggregation, getMyId,
  null telemetry handling

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
With AGP 9's built-in Kotlin, org.jetbrains.kotlin.android is no longer
applied to Android modules. Convention plugin hooks using
withPlugin("org.jetbrains.kotlin.android") were dead code — replaced
with withPlugin("com.android.application") and
withPlugin("com.android.library") which correctly trigger for
Android-only modules using built-in Kotlin.

- KoinConventionPlugin: split into app + library hooks
- AndroidRoomConventionPlugin: use com.android.library hook
- KotlinXSerializationConventionPlugin: use app + library hooks
- Remove kotlin-android from version catalog and root build.gradle.kts

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
No modules are published externally anymore (core:api was removed).
Remove all publishing-related configuration:

- Delete PublishingConventionPlugin and its registration
- Remove publish-core.yml workflow
- Strip publishing{} blocks from core/model and core/proto
- Remove PUBLISHED_MODULES set and Java 17 compatibility logic
- Unify all modules to JDK 21 toolchain and JVM target
- Remove unused imports (JavaToolchainService, JavaLanguageVersion, Test)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove x86/x86_64 from ABI filters (armeabi-v7a/arm64-v8a only)
- Remove unused takpacket-sdk-jvm from version catalog
- Raise core/proto minSdk from 21 to 26 (ATAK compat no longer needed)
- Remove stale FIXME comment about foreground service in manifest
- Replace Executors.newSingleThreadExecutor with Dispatchers.Default.asExecutor
  in BarcodeScannerProvider (removes manual thread pool management)
- Convert formatAgo() to @composable with stringResource(), eliminating
  runBlocking from the UI rendering path. Non-composable callers (map views,
  accessibility) use a 3-arg overload with pre-resolved strings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Wrap StateFlow.value read in remember{} to satisfy composition lint
- Wrap derivedStateOf in remember{} (MapView PurgeTileSourceDialog)
- Use mutableIntStateOf to avoid autoboxing (MapStyleDialog)
- Use ResourcesCompat.getDrawable() instead of deprecated getDrawable()
- Fix mixed indentation in MarkerClusterer.java

Lint result: 0 errors, 10 warnings (down from 2 errors, 12 warnings)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This feature flag is now enabled by default in the Compose compiler,
making the explicit opt-in unnecessary.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- NodeAddress.idToNum: reject >32-bit hex instead of silently truncating;
  use toLongOrNull rather than runCatching; drop redundant double prefix-strip.
- DirectRadioControllerImpl: compare destNum against the nullable
  myNodeNum.value (not the ?:0 getter) so local-side-effect branches
  don't fire spuriously for destNum=0 before connect.
- DirectRadioControllerImpl.setModuleConfig: move node_status optimistic
  write inside the local-node guard so we don't overwrite remote status.
- DirectRadioControllerImpl.sendReaction: use ContactKey wrapper instead
  of manual [0].digitToInt()/substring(1).
- DirectRadioControllerImpl.importContact: respect incoming
  manually_verified flag; early-return on node_num==0 or null user.
- DirectRadioControllerImpl.requestPosition: consolidate when-chain;
  document Position(0,0,0) as protocol "no position" sentinel.
- ContactKey: accessors safe against empty value; non-digit first char
  defaults channel to 0 rather than throwing.
- NodeDetailViewModel.openRemoteAdmin: atomic compareAndSet replaces
  check-then-act TOCTOU that allowed double-tap to queue two passkey
  exchanges + duplicate navigation events.
- MessageViewModel.frequentEmojis: toIntOrNull + mapNotNull so a
  corrupted pref entry no longer crashes every recomposition.
- AndroidMeshLocationManager.restart: log the silent-bail case
  (no-op until MeshConnectionManagerImpl wires us up).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- RadioController.setDeviceAddress is now suspend. Callers can rely on the
  device switch completing (DB switched, node DB cleared, transport
  reconfigured) before their next call — the old non-suspend impl wrapped
  the work in scope.launch{} and returned immediately, racing against
  follow-up operations like OTA disconnect-then-delay.
  - DirectRadioControllerImpl: drops the scope.launch wrapper.
  - FakeRadioController: matches the new suspend contract so tests no longer
    hide ordering bugs that production would exhibit.
  - UIViewModel / ScannerViewModel: wrap the suspend call in safeLaunch.
  - Esp32OtaUpdateHandler.disconnectMeshService: marked suspend (caller is
    already inside a withContext block).

- MeshServiceOrchestrator: replace plain `var scope: CoroutineScope?` with
  an atomicfu AtomicRef and compareAndSet on start(). Concurrent start()
  invocations could otherwise both observe the slot empty, both allocate
  scopes, and the second overwrite the first — orphaning the first scope's
  collectors on radioInterfaceService.receivedData.

- PacketHandlerImpl.sendToRadio: document the FIFO invariant. Mutex is
  not strictly fair across coroutines, but the only ordering callers rely
  on is within a single coroutine (e.g. InstallProfileUseCase issues
  beginEditSettings → install* → commitEditSettings sequentially in one
  suspend), where sequential mutex acquisition preserves order.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add a "Radio Control" section to the developer architecture guide
covering how features issue radio commands: the RadioController
composite, its four sub-interfaces (Admin/Messaging/Node/Request), and
DirectRadioControllerImpl as the in-process composition root that
assembles them via interface delegation. Notes the fire-and-forget admin
model and the deliberate alignment with the meshtastic-sdk API shape.

Additive only (no stale-reference fix); bumps last_updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jamesarich and others added 7 commits May 29, 2026 12:50
Apply Interface Segregation Principle to ServiceRepository and
RadioController consumers:

New interfaces extracted from ServiceRepository:
- ConnectionStateProvider: read-only connectionState access
- TracerouteResponseProvider: traceroute response state + clear
- NeighborInfoResponseProvider: neighbor info response state + clear
- ServiceStateWriter: write-side for handlers (set*, emit*, clear*)

RadioController now extends ConnectionStateProvider, and sub-controller
interfaces (AdminController, MessagingController, NodeController,
RequestController) are bound in DI for fine-grained injection.

ViewModel narrowing:
- MessageViewModel: RadioController+ServiceRepository → MessagingController+ConnectionStateProvider
- NodeDetailViewModel: RadioController → RequestController
- NodeListViewModel: RadioController+ServiceRepository → AdminController+ConnectionStateProvider
- ContactsViewModel: ServiceRepository → ConnectionStateProvider
- MetricsViewModel: ServiceRepository → TracerouteResponseProvider

All tests updated to use narrowed interfaces. Koin DI bindings updated
for both Android (@single binds) and Desktop (manual single<> declarations).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update core/repository README and architecture docs to reflect the new
focused provider interfaces (ConnectionStateProvider, TracerouteResponseProvider,
NeighborInfoResponseProvider, ServiceStateWriter) and ViewModel narrowing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ateProvider

Following the ServiceRepository ISP decomposition, the two consumers that
only read connectionState now inject ConnectionStateProvider instead of
the full ServiceRepository:

- ConnectionsViewModel (core:ui)
- LocalStatsWidgetStateProvider (feature:widget)

The remaining three full-ServiceRepository injections are left as-is
because they use members not carried by any narrow provider:
RadioConfigViewModel (meshPacketFlow), ScannerViewModel
(connectionProgress), and UIViewModel (clientNotification + errorMessage
reads). Extracting single-consumer providers for those would be
over-segregation.

Koin graph verifies on Android + desktop; ConnectionsViewModelTest green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The ISP decomposition extracted ServiceStateWriter but left it with zero
adopters — every write-side handler still injected the full
ServiceRepository, able to read state it only writes. Complete the
adoption:

- 9 pure writers now inject ServiceStateWriter (param renamed
  serviceStateWriter): TracerouteHandlerImpl, NeighborInfoHandlerImpl,
  MeshMessageProcessorImpl, MeshConfigFlowManagerImpl, MeshConfigHandlerImpl,
  MeshDataHandlerImpl, FromRadioPacketHandlerImpl, MqttManagerImpl,
  MeshServiceOrchestrator.
- PacketHandlerImpl reads only connectionState -> ConnectionStateProvider.

The two genuinely mixed read+write holders (MeshConnectionManagerImpl,
DirectRadioControllerImpl) keep the full ServiceRepository. Read-side
flows with a single consumer (connectionProgress/clientNotification/
errorMessage/meshPacketFlow, mostly UIViewModel) are intentionally not
extracted into providers — that would be over-segregation.

Koin graph verifies on Android + desktop; core:data/core:service suites green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Audit-driven renames:
- DirectRadioControllerImpl -> RadioControllerImpl: the "Direct" prefix
  was vestigial (it once contrasted with the AIDL-routed
  AndroidRadioControllerImpl, now deleted); it is the sole impl and now
  matches its parts (AdminControllerImpl, etc.).
- RadioController.getPacketId() -> generatePacketId(): a `get`-prefixed
  function that generates a fresh id each call violates the
  getter-is-idempotent convention; also aligns with
  CommandSender.generatePacketId().
- RequestController -> QueryController (+ Impl, + the `requestController`
  params/mocks): clearer intent for the pull/query surface; "request" was
  generic.
- RequestTimer param `label` -> `logLabel`.
- AdminControllerImpl DEFAULT_REBOOT_DELAY -> DEFAULT_DELAY_SECONDS
  (shared by reboot + shutdown; conveys the unit).

Interface-consumer-safe; docs/READMEs/architecture guide updated to match.
Koin graph verifies on Android + desktop; affected test suites green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…l-api

# Conflicts:
#	.agent_memory/session_context.md
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jamesarich jamesarich added this to the 2.8.0 milestone May 30, 2026
jamesarich added a commit that referenced this pull request May 30, 2026
Squash merge of PR #5586 into release/2.8.0.

Remove the deprecated AIDL API and all associated infrastructure. Replaces
with a modern, testable, KMP-compatible architecture. 264 files changed with
a net reduction of ~2,559 lines. Introduces RadioController, MessageQueue,
and typed ContactKey abstractions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesarich

Copy link
Copy Markdown
Collaborator Author

🔗 Release 2.8.0 Integration Report

Branch: release/2.8.0 | Status: ✅ Build + all tests passing

Integration Changes Required

  1. CommandSenderImpl.kt — Lockdown PR (feat: firmware lockdown mode (provision / unlock / lock-now) #5439) adds sendLockdownPassphrase() with maxSessionSeconds and disable parameters. These needed to be wired through the new RadioController interface (post-AIDL). The implementation constructs LockdownAuth proto and sends via radioController.sendAdminPacket().

  2. MeshActionHandlerImpl.kt — Car App (feat(car): Android Car App Library integration #5633) and App Functions (feat(ai): Add App Functions for system AI integration #5585) both call sendMessage(). After AIDL removal, this routes through SendMessageUseCaseMeshActionHandlerRadioController. Needed to ensure the handler supports both broadcast and DM paths.

  3. ServiceRepositoryImpl.kt — Multiple consumers (Car, App Functions, Discovery) depend on ServiceRepository flows. After AIDL removal, ensured all flows (connectionState, meshPackets, nodeList) are properly exposed as StateFlow/SharedFlow.

  4. RadioControllerImpl.kt — Discovery PR (feat(discovery): mesh network discovery #5275) registers a DiscoveryPacketCollectorRegistry that subscribes to radio packets. Ensured the new RadioController dispatches to registered collectors.

  5. Test fixes across 4 files:

    • MeshConnectionManagerImplTest.kt — Updated for new connection lifecycle
    • MeshDataHandlerTest.kt — Adapted packet handling assertions
    • MeshActionHandlerImplTest.kt — New test for DM routing
    • ConnectionsViewModelTest.kt — Updated for StateFlow-based connection state

Merge Guidance

  • Merge order: Fourth (after FTS5 search). This is the architectural pivot — everything after adapts to its API.
  • Impact on other PRs: All other 2.8.0 PRs reference interfaces this PR restructures. Merging this first would require rebasing all others. Merging it fourth means only Lockdown (feat: firmware lockdown mode (provision / unlock / lock-now) #5439) and Discovery (feat(discovery): mesh network discovery #5275) need post-merge fixups.
  • Recommendation: Consider merging this PR early and having remaining PRs rebase onto it, since its API surface is what everything else must conform to.

jamesarich and others added 2 commits May 31, 2026 09:03
Wrap database write operations (clearUnreadCount, clearAllUnreadCounts,
updateLastReadMessage, insertRoomPacket) with NonCancellable to prevent
connection pool leaks when the calling coroutine scope is cancelled
mid-write.

Found during release/2.8.0 integration testing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sort imports alphabetically and format single-line lambdas properly
to pass spotlessCheck and detekt.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesarich jamesarich mentioned this pull request Jun 3, 2026
@jamesarich jamesarich changed the base branch from main to release/2.8.0 June 4, 2026 00:48
jamesarich and others added 3 commits June 3, 2026 20:02
Resolved conflicts:
- SendMessageUseCase: keep #5585's Int return + #5586's NodeAddress/ContactKey parsing
- core/proto/build.gradle.kts: keep release's ATAK minSdk=21 override
- proto submodule pinned to 6b1ded4 (master HEAD)
- docs (copilot-instructions, session_context): take release/2.8.0

Post-merge API fixups (ID_BROADCAST/ID_LOCAL moved DataPacket -> NodeAddress):
- AiFunctionProviderImpl (#5585), PacketRepositoryImpl
- car: HomeScreen, ConversationShortcutManager, CarScreenDataBuilder (#5633)
… AiFunctionProvider

Final post-AIDL-removal API fixup: PKC_CHANNEL_INDEX also moved to NodeAddress.
Plus ktfmt reformat of CarScreenDataBuilder.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All DataPacket constant refs migrated to NodeAddress; the import is dead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jamesarich jamesarich merged commit 3752984 into release/2.8.0 Jun 4, 2026
15 checks passed
@jamesarich jamesarich deleted the jamesarich/remove-aidl-api branch June 4, 2026 02:28
jamesarich added a commit that referenced this pull request Jun 4, 2026
Resolved conflicts:
- Room schema renumbered v39 -> v42 (FTS5=39, air-quality=40, msh.to=41): version=42,
  AutoMigration(41->42), 3 discovery entities, regenerated 42.json, kept release's 39.json
- MeshtasticDatabase imports: union Discovery{Dao,entities} + DeviceLink{Dao,Entity}
- SettingsScreen / DesktopSettingsScreen: union Discovery + DeviceLinks (+ AppFunctions) entries
- proto pinned to 6b1ded4 (master HEAD); docs: take release/2.8.0

Post-merge API fixups (#5586 AIDL removal):
- DiscoveryScanEngine: RadioController core.model -> core.repository; getPacketId() -> generatePacketId()
- discovery tests: DataPacket.ID_BROADCAST -> NodeAddress.ID_BROADCAST
jamesarich added a commit that referenced this pull request Jun 10, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
jamesarich added a commit that referenced this pull request Jun 10, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
jamesarich added a commit that referenced this pull request Jun 10, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
jamesarich added a commit that referenced this pull request Jun 12, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
jamesarich added a commit that referenced this pull request Jun 13, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Build system changes refactor no functional changes repo Repository maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant