|
| 1 | +# Workstream 1: God Class Decomposition Plan |
| 2 | + |
| 3 | +**Goal**: Decompose 4 God classes using **composition + delegation**. Preserve all public APIs. No logic duplication. Single source of truth per concern. |
| 4 | + |
| 5 | +**Execution order**: Step 3 → Step 1 → Step 4 → Step 2 (simplest first, most complex last). |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## Step 1: Split `BluetoothSIGTranslator` (1,359 lines → ~682 line facade) — ✅ COMPLETE |
| 10 | + |
| 11 | +The class is nearly stateless — only `_services: dict` is mutable. All other methods delegate to static registries. Pattern: **Composition + Delegation Facade** (like `requests.Session`). |
| 12 | + |
| 13 | +### 1.1 New delegate modules under `src/bluetooth_sig/core/` |
| 14 | + |
| 15 | +| New file | Class | Methods moved from translator.py | Mutable state | |
| 16 | +|---|---|---|---| |
| 17 | +| `core/query.py` | `CharacteristicQueryEngine` | `supports`, `get_value_type`, all 8 `get_*_info_*`, `get_characteristics_info_by_uuids`, `list_supported_*`, `get_service_characteristics`, `get_sig_info_by_name`, `get_sig_info_by_uuid` | None | |
| 18 | +| `core/parser.py` | `CharacteristicParser` | `parse_characteristic` (+overloads), `parse_characteristics` (batch), 5 `_*` batch helpers | None | |
| 19 | +| `core/encoder.py` | `CharacteristicEncoder` | `encode_characteristic` (+overloads), `create_value`, `validate_characteristic_data`, `_get_characteristic_value_type_class` | None | |
| 20 | +| `core/registration.py` | `RegistrationManager` | `register_custom_characteristic_class`, `register_custom_service_class` | None (writes to registries) | |
| 21 | +| `core/service_manager.py` | `ServiceManager` | `process_services`, `get_service_by_uuid`, `discovered_services`, `clear_services` | `_services: dict` — **only mutable state** | |
| 22 | + |
| 23 | +### 1.2 Facade pattern |
| 24 | + |
| 25 | +`BluetoothSIGTranslator.__init__` creates 5 delegate instances eagerly. Every public method becomes a one-line delegation with identical signature and `@overload` decorators. Async wrappers stay on facade. Singleton `__new__`/`get_instance()`/global `BluetoothSIG` stay. |
| 26 | + |
| 27 | +### 1.3 Update `core/__init__.py` |
| 28 | + |
| 29 | +Re-export delegate classes alongside `BluetoothSIGTranslator` and `AsyncParsingSession`. |
| 30 | + |
| 31 | +--- |
| 32 | + |
| 33 | +## Step 2: Split `BaseCharacteristic` (1,761 lines → 1,258 lines) — ✅ COMPLETE |
| 34 | + |
| 35 | +Keeps Template Method contract (`_decode_value`/`_encode_value`). Internal composition invisible to ~150 subclasses. Pattern: **Internal Composition with back-reference**. |
| 36 | + |
| 37 | +### 2.1 New `pipeline/` package under `src/bluetooth_sig/gatt/characteristics/` |
| 38 | + |
| 39 | +| New file | Class | Methods extracted | Status | |
| 40 | +|---|---|---|---| |
| 41 | +| `pipeline/__init__.py` | Re-exports | — | ✅ | |
| 42 | +| `pipeline/parse_pipeline.py` | `ParsePipeline` | `parse_value` orchestration, `_perform_parse_validation`, `_extract_and_check_special_value`, `_decode_and_validate_value`, `_extract_raw_int`, `_check_special_value`, `_is_parse_trace_enabled` | ✅ | |
| 43 | +| `pipeline/encode_pipeline.py` | `EncodePipeline` | `build_value` orchestration, `_pack_raw_int`, `encode_special`, `encode_special_by_meaning` | ✅ | |
| 44 | +| `pipeline/validation.py` | `CharacteristicValidator` | `_validate_range` (3-level precedence), `_validate_type`, `_validate_length` | ✅ | |
| 45 | + |
| 46 | +### 2.2 Additional extractions |
| 47 | + |
| 48 | +| New file | Class | Methods extracted | Status | |
| 49 | +|---|---|---|---| |
| 50 | +| `role_classifier.py` | `classify_role()` function | `_classify_role`, `_spec_has_unit_fields` | ✅ | |
| 51 | +| `descriptor_support.py` | `DescriptorSupport` | 11 descriptor methods | Deferred — low value, methods are 1-liner proxies | |
| 52 | +| `special_values.py` | `SpecialValueHandler` | special value methods | Deferred — already uses SpecialValueResolver | |
| 53 | + |
| 54 | +### 2.3 What stays on `BaseCharacteristic` |
| 55 | + |
| 56 | +- `__init__`/`__post_init__` (composition wiring) |
| 57 | +- Properties: `uuid`, `info`, `spec`, `name`, `description`, `display_name`, `unit`, `size`, `value_type_resolved`, `role`, `get_byte_order_hint` |
| 58 | +- Abstract: `_decode_value`, `_encode_value` (Template Method hooks for subclasses) |
| 59 | +- Thin delegation: `parse_value` → `ParsePipeline.run()`, `build_value` → `EncodePipeline.run()`, `encode_special*` → `EncodePipeline` |
| 60 | +- Class-level UUID resolution (5 classmethods) |
| 61 | +- Dependency resolution (5 methods) |
| 62 | +- YAML metadata accessors (5 methods) |
| 63 | +- Descriptor methods (kept in base, 1-liner proxies to descriptor_utils) |
| 64 | +- Special value properties (kept in base, delegate to SpecialValueResolver) |
| 65 | +- YAML metadata accessors (5 methods) |
| 66 | +- Proxy methods for backward compat (delegate to composed objects) |
| 67 | + |
| 68 | +--- |
| 69 | + |
| 70 | +## Step 3: Split `templates.py` (1,488 lines → package) — ✅ COMPLETE |
| 71 | + |
| 72 | +No circular dependencies. Pure file reorganisation + re-export. Pattern: **Module → Package promotion**. |
| 73 | + |
| 74 | +### 3.1 New `templates/` package |
| 75 | + |
| 76 | +| New file | Classes | Approx lines | |
| 77 | +|---|---|---| |
| 78 | +| `templates/__init__.py` | Re-exports everything via explicit imports + `__all__` | ~60 | |
| 79 | +| `templates/base.py` | `CodingTemplate[T_co]` (ABC), resolution constants | ~100 | |
| 80 | +| `templates/data_structures.py` | `VectorData`, `Vector2DData`, `TimeData` | ~40 | |
| 81 | +| `templates/numeric.py` | `Uint8Template`, `Sint8Template`, `Uint16Template`, `Sint16Template`, `Uint24Template`, `Uint32Template` | ~200 | |
| 82 | +| `templates/scaled.py` | `ScaledTemplate` (abstract) + 8 `Scaled*Template` variants + `PercentageTemplate` | ~400 | |
| 83 | +| `templates/domain.py` | `TemperatureTemplate`, `ConcentrationTemplate`, `PressureTemplate` | ~200 | |
| 84 | +| `templates/ieee_float.py` | `IEEE11073FloatTemplate`, `Float32Template` | ~80 | |
| 85 | +| `templates/string.py` | `Utf8StringTemplate`, `Utf16StringTemplate` | ~150 | |
| 86 | +| `templates/complex.py` | `TimeDataTemplate`, `VectorTemplate`, `Vector2DTemplate` | ~200 | |
| 87 | +| `templates/enum.py` | `EnumTemplate[T]` | ~240 | |
| 88 | + |
| 89 | +### 3.2 Backward compat |
| 90 | + |
| 91 | +Python resolves `from .templates import X` identically whether `templates` is a module or package — as long as `X` is in the package `__init__.py`. Zero characteristic files need changing. |
| 92 | + |
| 93 | +--- |
| 94 | + |
| 95 | +## Step 4: Split `Device` (1,172 lines → ~818 lines) — ✅ COMPLETE |
| 96 | + |
| 97 | +13/40 methods are pure delegation. Substantial logic in dependency resolution and characteristic I/O. Pattern: **Composition with explicit dependencies (no back-references)**. |
| 98 | + |
| 99 | +### 4.1 New modules under `src/bluetooth_sig/device/` |
| 100 | + |
| 101 | +| New file | Class | Methods extracted | |
| 102 | +|---|---|---| |
| 103 | +| `dependency_resolver.py` | `DependencyResolver` + `DependencyResolutionMode` enum | `_resolve_single_dependency`, `_ensure_dependencies_resolved` | |
| 104 | +| `characteristic_io.py` | `CharacteristicIO` | `read` (+overloads), `write` (+overloads), `start_notify` (+overloads), `stop_notify`, `read_multiple`, `write_multiple`, `_resolve_characteristic_name` | |
| 105 | + |
| 106 | +### 4.2 Device composes |
| 107 | + |
| 108 | +```python |
| 109 | +self._dep_resolver = DependencyResolver(connection_manager, translator, self.connected) |
| 110 | +self._char_io = CharacteristicIO(connection_manager, translator, self.connected, self._dep_resolver) |
| 111 | +``` |
| 112 | + |
| 113 | +Remaining on Device: delegation one-liners, discovery, advertising, properties, service queries. |
| 114 | + |
| 115 | +--- |
| 116 | + |
| 117 | +## Verification (after each step) |
| 118 | + |
| 119 | +1. `python -m pytest tests/ -v` — all existing tests pass |
| 120 | +2. `./scripts/lint.sh --all` — zero errors |
| 121 | +3. `./scripts/format.sh --check` — formatting valid |
| 122 | +4. Backward compat imports still work |
| 123 | + |
| 124 | +## Design Principles |
| 125 | + |
| 126 | +| Principle | Application | |
| 127 | +|---|---| |
| 128 | +| **Single Responsibility** | Each delegate/composed class owns one concern | |
| 129 | +| **DRY** | Each method exists in exactly one place; facade only delegates | |
| 130 | +| **Composition over Inheritance** | Translator: 5 delegates. Base: internal composition. Templates: domain grouping | |
| 131 | +| **Single Source of Truth** | Registry access per delegate. Validation in one validator. Pipeline in one orchestrator | |
| 132 | +| **Open/Closed** | BaseCharacteristic open for extension (override hooks), closed for modification (pipeline internal) | |
| 133 | +| **Dependency Inversion** | Delegates take abstractions (registries, protocols), not concretions | |
| 134 | +| **Interface Segregation** | QueryEngine separate from Parser — consumers depend only on what they use | |
0 commit comments