Skip to content

refactor(ble): turn the 556-line createNobleHandler closure into real modules #369

Description

@KristianP26

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

  • handler-noble.ts and handler-noble-legacy.ts are untouched apart from the import path
  • No module in the new directory exceeds roughly 200 lines
  • Extracted helpers are importable and testable without going through _internals
  • Existing tests pass with no assertion edits
  • npx tsc --noEmit, eslint and prettier clean

Metadata

Metadata

Assignees

No one assigned

    Labels

    bleBluetooth issuestech-debtRefactor / architectural debt

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions