asg_client: remove vendor (K900/BES) imports from core-bound files ahead of A6 module split#3140
asg_client: remove vendor (K900/BES) imports from core-bound files ahead of A6 module split#3140nic-olo wants to merge 47 commits into
Conversation
📋 PR Review Helper📱 Mobile App Build⏳ Waiting for build... 🕶️ ASG Client Build✅ Ready to test! (commit 🔀 Test Locallygh pr checkout 3140 |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Deploying mentra-live-ota-site with
|
| Latest commit: |
b9e093f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e268e241.mentra-live-ota-site.pages.dev |
| Branch Preview URL: | https://cursor-pre-a6-vendor-decoupl.mentra-live-ota-site.pages.dev |
Deploying mentra-store-dev with
|
| Latest commit: |
b9e093f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fb55e187.augmentos-appstore-2.pages.dev |
| Branch Preview URL: | https://cursor-pre-a6-vendor-decoupl.augmentos-appstore-2.pages.dev |
… protocol strategy
…methods to instance
… of BesWireFormat
…stead of BesWireFormat
…mation, remove K900 cast
Deploying dev-augmentos-console with
|
| Latest commit: |
b9e093f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0a1f8d02.dev-augmentos-console.pages.dev |
| Branch Preview URL: | https://cursor-pre-a6-vendor-decoupl.dev-augmentos-console.pages.dev |
|
bugbot run |
4 similar comments
|
bugbot run |
|
bugbot run |
|
bugbot run |
|
bugbot run |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ce6e512bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @Inject ICompanionTransport companionTransport; | ||
|
|
||
| /** Device-appropriate network manager, constructed by the vendor wiring layer. */ | ||
| @Inject INetworkManager injectedNetworkManager; |
There was a problem hiding this comment.
Defer transport construction until foreground start
These Hilt field injections are resolved before this service's onCreate() body reaches ensureForegroundStarted(). On K900, provideCompanionTransport() constructs K900BluetoothManager, whose constructor opens /dev/ttyS1 and starts the serial receive thread, and the injected network manager can also do device setup; previously those factories ran inside initializeServiceInitializer() after foreground promotion. When serial/network setup is slow during a startForegroundService launch, this moves heavy work back in front of startForeground() and can trip the foreground-service startup deadline; inject a Provider/factory and call it after ensureForegroundStarted() instead.
Useful? React with 👍 / 👎.
|
bugbot run |
…rom OtaHelper Co-authored-by: Nicolo Micheletti <michelettinik@gmail.com>
Co-authored-by: Nicolo Micheletti <michelettinik@gmail.com>
…er until after ensureForegroundStarted Co-authored-by: Nicolo Micheletti <michelettinik@gmail.com>
Co-authored-by: Nicolo Micheletti <michelettinik@gmail.com>
Co-authored-by: Nicolo Micheletti <michelettinik@gmail.com>
1ac9b09 to
b9e093f
Compare
PR Agent HandoffExit reason: Budget exhausted — fix or review cycle cap reached.
CI status
Remaining blocking findingsNone Nits (informational)
Humans merge when ready. Agents do not auto-merge. Label |
Scope
Pre-A6 cleanup for
asg_client: removes all K900/BES vendor imports from the files that will live in:asg-coreafter the A6 module split, so A6 becomes a mechanical directory move. No behavior changes intended.Fix 1 — Core LED constants
io/hardware/interfaces/RgbLedConstants(LED indexes 0-4, default brightness 100).RgbLedCommandHandler,MediaCaptureService,AsgClientServiceno longer importK900RgbLedController.Fix 2 — Transport-level hooks on
IBluetoothManageronMtuNegotiated,onTransportReset,onFileTransferConfirmation,onFileTransferAck.K900BluetoothManageroverrides them, delegating toBesWireFormat/ its existing file-transfer handlers.BleConfigCommandHandler,PhoneReadyCommandHandler,TransferCompleteCommandHandler,FileTransferAckEventSubscribernow call the interface — the(K900BluetoothManager)casts andBesWireFormatstatic calls are gone.BleConfigCommandHandlertakesAsgClientServiceManagerlike its sibling handlers.Fix 3 —
K900ProtocolStrategyinjected, not hardcodedCommandProtocolDetectorships only base strategies (JSON, Unknown, plus chunked support).@IntoSet(newTransportModule), injected intoAsgClientService, and registered throughServiceInitializer→CommandProcessorbefore chunked support is added. Strategy order changes from (Chunked, Json, K900, Unknown) to (Chunked, K900, Json, Unknown) — behavior-equivalent because K900 only claims invalid-JSONCpayloads, which Json/Chunked never claim (pinned by tests).Fix 4 —
IBesOtaController/IBesOtaRegistryinterfacesio/ota/interfaces/;BesOtaManager/BesOtaRegistryimplement them.BesOtaManager(getCurrentFirmwareVersion,parseServerVersionCode,isNewerVersion) became instance methods (Java forbids same-signature instance+static; their only external caller wasOtaHelper). The staticisBesOtaInProgressflag remains for internal use; the interface exposes an instance accessor.OtaHelper,BesOtaAuthEventSubscriber,K900CommandHandler,DebugBesOtaReceiver,AsgClientServiceManager.getBesOtaManager(),OtaModule,AsgClientEntryPoint,AsgClientService,ServiceInitializerall consume the interfaces.OtaHelperBES-progress checks are now null-safe registry lookups (null controller = non-K900 = not in progress).Fix 5 — Factory calls removed from
ServiceInitializerServiceInitializernow receivesICompanionTransport/INetworkManagervia constructor;AsgClientServiceinjects them from the newTransportModule.AsgClientServiceManager.cleanup()shuts the transport down on service destroy, so a singleton would re-inject a dead instance on service restart. Unscoped matches the previous fresh-instance-per-service-creation factory behavior (pinned by tests).Acceptance check
Zero vendor imports in the scoped core dirs (run from
asg_client/):Returns zero results. Sole documented exclusion:
CommandProcessor→K900CommandHandler(intra-service.coreimport today; decoupling it is its own pre-A6 task, likeOtaHelpersplitting is B3). Known remaining boundary debt, intentionally untouched:AsgClientService→com.dev.api.DevApi, all ofservice/legacy/.Tests
./gradlew :app:testDebugUnitTest: 357 tests, 0 failures, 0 errors (1 pre-existing skip inMediaUploadTest)../gradlew :app:assembleDebug: BUILD SUCCESSFUL — also validates the Hilt graph (interface bindings +@IntoSetmultibinding).New coverage (15 new test classes + 1 updated):
RgbLedConstantsTest(core/vendor parity pin),RgbLedCommandHandlerTest,IBluetoothManagerDefaultsTest,BleConfigCommandHandlerTest,PhoneReadyCommandHandlerTest,TransferCompleteCommandHandlerTest(regression guard for the oldClassCastException-prone cast, using a plain transport mock),FileTransferAckEventSubscriberTest,CommandProtocolDetectorTest(UNKNOWN without vendor strategy / K900 after runtime registration / chunked ordering),BesOtaRegistryTest,BesOtaManagerVersionTest,OtaHelperBesGuardTest(null-controller NPE guards + in-progress consultation),OtaModuleTest,TransportModuleTest(device selection + fresh-instance-per-call lifecycle pin).CommandProcessorRoutingIntegrationTest— raw bytes through realCommandProcessor/CommandParser/detector/K900CommandHandler/McuEventParserto the peripheral bus and transport interface, with and without the injected vendor strategy;BesOtaAuthRoutingIntegrationTest— realSimplePeripheralBus+ real subscribers toIBesOtaController/ICompanionTransport.BesOtaAuthEventSubscriberTestupdated to mockIBesOtaControllerinstead of the concrete vendor class.K900BluetoothManager's hook overrides are one-line delegations intoBesWireFormatstatics whose transitions are already covered byBesWireFormatTest; a construction-based test was skipped because the constructor starts aSerialPortBridgeUART thread (device-bound).Out of scope (per plan)
Splitting
OtaHelper(B3); decouplingCommandProcessorfromK900CommandHandler; moving files (A6);AsgClientServiceManagerbeyond the registry/getter type changes; BES OTA protocol logic; Kotlin migration.