feat: Refactor MeshService into smaller, single-responsibility components#4108
Merged
Conversation
…ents
This commit refactors the monolithic `MeshService` into smaller, more focused, and testable components. The goal is to improve maintainability, clarify responsibilities, and facilitate future development by adopting a more modular architecture.
Key changes include:
- **`MeshService`**: Stripped down to a lifecycle and binding coordinator. Most of its logic has been moved to new, specialized classes.
- **`MeshRouter`**: A new central class that orchestrates various handlers for incoming packets.
- **Component Breakdown**:
- **`ConnectionStateHandler`**: Manages and broadcasts the device connection state (renamed from `MeshServiceConnectionStateHolder`).
- **`MeshConnectionManager`**: Handles connection lifecycle events (connect, disconnect, sleep), notifications, and starts/stops other services like location updates.
- **`MeshConfigHandler`**: Manages device and module configuration state.
- **`MeshConfigFlowManager`**: Orchestrates the multi-stage config/node-info download process.
- **`MeshNodeManager`**: Manages the node database, including caching, updates, and lookups.
- **`MeshDataHandler`**: Processes application-layer data packets (e.g., text messages, positions, telemetry).
- **`MeshMessageProcessor`**: Acts as the entry point for all incoming radio data, handling early packet buffering and dispatching to the appropriate router or handler.
- **`MeshCommandSender`**: Centralizes the logic for creating and sending commands and data packets to the radio.
- **`MeshLocationManager`**: Manages location updates from the device's GPS.
- **`MeshMqttManager`**: Handles MQTT proxy functionality.
- **`MeshHistoryManager`**: Manages store-and-forward history requests.
- **`MeshTracerouteHandler` & `MeshNeighborInfoHandler`**: New dedicated handlers for their respective packet types.
- **Exception Hierarchy**: `BLEException` and its related classes are now subclasses of `RadioNotConnectedException` for more consistent error handling.
- **Constants**: Centralized broadcast action strings and other constants into `Constants.kt`.
- **Testing**: Added new unit tests for the refactored components (`MeshNodeManagerTest`, `MeshCommandSenderTest`, `MeshDataMapperTest`) and updated existing tests.
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4108 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 2 2
Lines 19 19
Branches 7 7
=====================================
Misses 19 19 ☔ View full report in Codecov by Sentry. |
Collaborator
Author
MeshService Refactor: Complete Architectural Summary & ComparisonThis document provides a comprehensive technical overview and "Before & After" comparison of the 🏗️ Architectural OverviewThe core of the refactor was the shift from a "God Object" model to a Delegated Pipeline. Summary of Shifts
🛡️ Stability & Lifecycle "Tightening"One of the primary goals was to ensure zero resource leaks.
📉 Lifecycle ComparisonBefore: Ad-Hoc Scope ManagementIn the monolith, graph TD
MS[MeshService] -- created --> Job[Manual serviceJob: Job]
MS -- created --> Scope[Manual serviceScope: CoroutineScope]
MS -->|Direct Use| PH[PacketHandler]
MS -->|Direct Use| HRD[handleReceivedData]
Note["Management was ad-hoc; no enforced start(scope) pattern."]
After: Standardized Scope Propagationgraph TD
MS[MeshService] -- creates --> Scope[serviceScope]
MS -- passes to --> Router
Router -- spreads to --> DH[DataHandler]
Router -- spreads to --> CH[ConfigHandler]
Router -- spreads to --> NH[NeighborInfoHandler]
MS -- passes to --> NM[NodeManager]
MS -- onStop/Destroy --> Cancel[Scope.cancel]
Cancel --> AllHandlers[All Handlers Stop Simultaneously]
Important This architecture ensures that zero background work continues if the service is stopped, making the app much more battery efficient and stable during rapid reconnects. 🛰️ Data Flow: Incoming Radio PacketsThese diagrams illustrate how a packet travels from the radio to its final destination. Before: The Monolithic TangleIn the monolithic version, graph TD
Radio[RadioInterfaceService] -->|ByteArray| MS[MeshService]
subgraph monolith["MeshService Monolith (main branch)"]
MS -->|handleFromRadio| Proto[MeshProtos.FromRadio]
Proto -->|PACKET| HRMP[handleReceivedMeshPacket]
HRMP -->|isReady| PRMP[processReceivedMeshPacket]
HRMP -->|!isReady| Buffer[EarlyPacketBuffer]
PRMP -->|metadata| HRD[handleReceivedData]
HRD -->|PortNum when| Dispatch{Internal Dispatch}
Dispatch -->|TEXT| Rem[rememberDataPacket]
Dispatch -->|ADMIN| HRA[handleReceivedAdmin]
Dispatch -->|POSITION| HRP[handleReceivedPosition]
Dispatch -->|TELEMETRY| HRT[handleReceivedTelemetry]
HRA -->|inline mutation| DB[(Node DB)]
HRP -->|inline mutation| DB
HRT -->|inline mutation| DB
HRD -->|End| BC[broadcastReceivedData]
end
After: The Specialized PipelineIn the refactored version, responsibilities are cleanly separated. Each stage has a specific purpose. graph TD
Radio[RadioInterfaceService] -->|ByteArray| MP[MeshMessageProcessor]
subgraph processing["Processing Layer"]
MP -->|parse| Proto[FromRadio]
Proto -->|PACKET| HRMP[handleReceivedMeshPacket]
HRMP -->|isReady| PRMP[processReceivedMeshPacket]
HRMP -->|!isReady| Buffer[EarlyPacketBuffer]
PRMP -->|metadata| DH[MeshDataHandler]
end
subgraph handlers["Handlers"]
DH -->|Data/Text| DH
DH -->|Admin| CH[MeshConfigHandler]
DH -->|NeighborInfo| NH[MeshNeighborInfoHandler]
DH -->|Traceroute| TH[MeshTracerouteHandler]
end
DH -->|Update| NM[MeshNodeManager]
CH -->|Update| NM
NM -->|Persist| Repo[NodeRepository]
DH -->|Notify| SB[MeshServiceBroadcasts]
SB -->|Intent| OS[Android System]
⚡ Action Flow: Binder IPC CallsHow client applications interact with the service. Before: Direct ImplementationThe Binder was often used as a dumping ground for logic, leading to a massive service file and difficult-to-trace state changes. graph LR
App[Client App] -->|IPC| Stub[IMeshService.Stub]
Repo[ServiceRepository] -->|ServiceAction| MS[MeshService]
subgraph monolith_ipc["MeshService Monolith (main branch)"]
Stub -->|Direct Logic| PH[PacketHandler]
Stub -->|Direct Mutation| DB[(Node DB)]
MS -->|onServiceAction| LocalMethods["Private Methods: favoriteNode, ignoreNode, etc."]
LocalMethods -->|PacketHandler| Radio[Radio]
end
After: Clean DelegationThe graph LR
App[Client App] -->|IPC| Stub[IMeshService.Stub]
subgraph delegation["Delegation Layer"]
Stub -->|Delegates to| AH[MeshActionHandler]
end
AH -->|Orchestrates| CS[MeshCommandSender]
AH -->|Updates| NM[MeshNodeManager]
AH -->|Triggers| SB[MeshServiceBroadcasts]
✅ Functional Parity Details
Note This refactor reduces the complexity of the core service loop by orders of magnitude while providing a much more maintainable and testable foundation for future features. This writeup was clanker generated, but reviewed by me. |
Migrated the asynchronous operation handling in `PacketHandler.kt` from `java8.util.concurrent.CompletableFuture` to `kotlinx.coroutines.CompletableDeferred`. This change modernizes the concurrency model by: * Using `withTimeout` and `await()` for cleaner, idiomatic coroutine-based timeout and result handling. * Removing the `java.util.concurrent.TimeoutException` in favor of `kotlinx.coroutines.TimeoutCancellationException`. * Replacing `isDone` with `isCompleted` for checking the status of the deferred task.
f371433 to
8d0b670
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This refactors the monolithic
MeshServiceinto smaller, more focused, and testable components. The goal is to improve maintainability, clarify responsibilities, and facilitate future development by adopting a more modular architecture.Key changes include:
MeshService: Stripped down to a lifecycle and binding coordinator. Most of its logic has been moved to new, specialized classes.MeshRouter: A new central class that orchestrates various handlers for incoming packets.ConnectionStateHandler: Manages and broadcasts the device connection state (renamed fromMeshServiceConnectionStateHolder).MeshConnectionManager: Handles connection lifecycle events (connect, disconnect, sleep), notifications, and starts/stops other services like location updates.MeshConfigHandler: Manages device and module configuration state.MeshConfigFlowManager: Orchestrates the multi-stage config/node-info download process.MeshNodeManager: Manages the node database, including caching, updates, and lookups.MeshDataHandler: Processes application-layer data packets (e.g., text messages, positions, telemetry).MeshMessageProcessor: Acts as the entry point for all incoming radio data, handling early packet buffering and dispatching to the appropriate router or handler.MeshCommandSender: Centralizes the logic for creating and sending commands and data packets to the radio.MeshLocationManager: Manages location updates from the device's GPS.MeshMqttManager: Handles MQTT proxy functionality.MeshHistoryManager: Manages store-and-forward history requests.MeshTracerouteHandler&MeshNeighborInfoHandler: New dedicated handlers for their respective packet types.BLEExceptionand its related classes are now subclasses ofRadioNotConnectedExceptionfor more consistent error handling.Constants.kt.MeshNodeManagerTest,MeshCommandSenderTest,MeshDataMapperTest) and updated existing tests.