Problem
createNobleHandler in src/ble/handler-noble-shared.ts:260 spans 556 lines, making it the largest single construct in the codebase.
Unlike #368 this is not one long procedure. It holds seven nested functions (waitForPoweredOn, connectWithRetries, discoverPeripheral, broadcastScan, scanAndReadRaw, scanAndRead, scanDevices) plus their shared state. It is a module that happens to be written as a closure.
The module boundaries already exist as comments
The function is internally divided by banner comments:
// ─── Noble state management ───
// ─── Connection helpers ───
// ─── Discovery helpers ───
// ─── Broadcast scan (advertisement-based weight reading) ───
// ─── Exports ───
Somebody already decided where the seams are. They just are not files, so nothing enforces them and nothing can be imported individually.
Why the closure exists, and why that must survive
The factory shape is deliberate, not accidental. Per the header comment, the two driver entrypoints supply their own Noble instance and getState accessor so the implementation can be shared (#181):
src/ble/handler-noble.ts -> @stoprocent/noble
src/ble/handler-noble-legacy.ts -> @abandonware/noble
That dependency injection is the whole point of the file and must be preserved. A split cannot simply hoist the nested functions to module scope, because they close over noble and getState. Each extracted module needs to take those as parameters, or the group needs a small explicit context object passed between them.
The testability tell
The closure ends with:
/** Test-only export of private helpers (#163, #283). */
_internals: { broadcastScan, wrapChar },
Worth being precise here: _internals is an established convention across this BLE layer, appearing in handler-esphome-proxy/client.ts, handler-esphome-proxy/index.ts and handler-node-ble/index.ts as well, so its presence alone is not the problem. The difference is that for a module-scope function _internals is a choice, while inside this closure it is the only way to reach anything, and the two issue numbers on that line suggest the hole has been widened more than once.
Proposed shape
src/ble/handler-noble/, mirroring the banner comments, with each module taking the driver dependencies explicitly:
| Module |
Contents |
state.ts |
waitForPoweredOn and adapter state handling |
connect.ts |
connectWithRetries |
discovery.ts |
discoverPeripheral |
broadcast.ts |
broadcastScan |
index.ts |
createNobleHandler, wiring the above and exposing scanAndReadRaw / scanAndRead / scanDevices |
This is the same move #131 made for handler-mqtt-proxy.ts, and it matches the directory layout handler-node-ble/ and handler-esphome-proxy/ already use, so the BLE layer ends up consistent instead of having one handler that is a single file and the rest directories.
Constraints
- Pure refactor. Both driver entrypoints keep working unchanged, and
createNobleHandler({ noble, getState }) keeps its signature.
_internals stays available to the tests that use it, even if what it points at moves.
- No behaviour change in retry counts, timeouts or state transitions.
- Existing tests pass with no assertion edits.
Acceptance criteria
Problem
createNobleHandlerinsrc/ble/handler-noble-shared.ts:260spans 556 lines, making it the largest single construct in the codebase.Unlike #368 this is not one long procedure. It holds seven nested functions (
waitForPoweredOn,connectWithRetries,discoverPeripheral,broadcastScan,scanAndReadRaw,scanAndRead,scanDevices) plus their shared state. It is a module that happens to be written as a closure.The module boundaries already exist as comments
The function is internally divided by banner comments:
Somebody already decided where the seams are. They just are not files, so nothing enforces them and nothing can be imported individually.
Why the closure exists, and why that must survive
The factory shape is deliberate, not accidental. Per the header comment, the two driver entrypoints supply their own Noble instance and
getStateaccessor so the implementation can be shared (#181):src/ble/handler-noble.ts->@stoprocent/noblesrc/ble/handler-noble-legacy.ts->@abandonware/nobleThat dependency injection is the whole point of the file and must be preserved. A split cannot simply hoist the nested functions to module scope, because they close over
nobleandgetState. Each extracted module needs to take those as parameters, or the group needs a small explicit context object passed between them.The testability tell
The closure ends with:
Worth being precise here:
_internalsis an established convention across this BLE layer, appearing inhandler-esphome-proxy/client.ts,handler-esphome-proxy/index.tsandhandler-node-ble/index.tsas well, so its presence alone is not the problem. The difference is that for a module-scope function_internalsis a choice, while inside this closure it is the only way to reach anything, and the two issue numbers on that line suggest the hole has been widened more than once.Proposed shape
src/ble/handler-noble/, mirroring the banner comments, with each module taking the driver dependencies explicitly:state.tswaitForPoweredOnand adapter state handlingconnect.tsconnectWithRetriesdiscovery.tsdiscoverPeripheralbroadcast.tsbroadcastScanindex.tscreateNobleHandler, wiring the above and exposingscanAndReadRaw/scanAndRead/scanDevicesThis is the same move #131 made for
handler-mqtt-proxy.ts, and it matches the directory layouthandler-node-ble/andhandler-esphome-proxy/already use, so the BLE layer ends up consistent instead of having one handler that is a single file and the rest directories.Constraints
createNobleHandler({ noble, getState })keeps its signature._internalsstays available to the tests that use it, even if what it points at moves.Acceptance criteria
handler-noble.tsandhandler-noble-legacy.tsare untouched apart from the import path_internalsnpx tsc --noEmit, eslint and prettier clean