Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors singleton registry access across the codebase to use explicit getter/class methods (instead of module-level global instances), adds a centralized “prewarm” catalogue to deterministically load registries, and updates tests/docs/benchmarks to reflect the new lifecycle and sticky-failure loading behavior.
Changes:
- Replaced module-level singleton registry variables with explicit getter functions (e.g.,
get_uuid_registry(),get_*_registry()) across library code, tests, and documentation. - Introduced a discovery-based prewarm catalogue (
prewarm_catalog.py) and updatedprewarm_registries()to execute registered loaders. - Added lifecycle + performance benchmarks/tests for
UuidRegistrylazy-load + sticky-failure semantics; updated docs to match.
Reviewed changes
Copilot reviewed 81 out of 82 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/utils/test_prewarm.py | Adds coverage that prewarm catalog includes and loads the UUID registry. |
| tests/test_units_registry.py | Updates tests to use get_units_registry() getter. |
| tests/test_service_classes_registry.py | Updates tests to use get_service_classes_registry() getter. |
| tests/test_sdo_uuids_registry.py | Updates tests to use get_sdo_uuids_registry() getter. |
| tests/test_object_types_registry.py | Updates tests to use get_object_types_registry() getter. |
| tests/test_mesh_profiles_registry.py | Updates tests to use get_mesh_profiles_registry() getter. |
| tests/test_members_registry.py | Updates tests to use get_members_registry() getter. |
| tests/test_declarations_registry.py | Updates tests to use get_declarations_registry() getter. |
| tests/test_browse_groups_registry.py | Updates tests to use get_browse_groups_registry() getter. |
| tests/static_analysis/test_yaml_implementation_coverage.py | Switches UUID registry access to get_uuid_registry() for static-analysis tests. |
| tests/static_analysis/test_service_registry_completeness.py | Switches UUID registry access to get_uuid_registry() for service completeness checks. |
| tests/static_analysis/test_characteristic_registry_completeness.py | Switches UUID registry access to get_uuid_registry() for characteristic completeness checks. |
| tests/registry/test_yaml_units.py | Uses get_uuid_registry() in YAML unit parsing tests. |
| tests/registry/test_uri_schemes.py | Uses get_uri_schemes_registry() getter. |
| tests/registry/test_registry_validation.py | Uses get_uuid_registry() for registry validation checks. |
| tests/registry/test_protocol_identifiers.py | Uses get_protocol_identifiers_registry() getter; removes unnecessary cast. |
| tests/registry/test_ad_types.py | Uses get_ad_types_registry() getter; updates singleton test (but currently asserts function identity). |
| tests/registry/core/test_uri_schemes.py | Updates fixture to use get_uri_schemes_registry(). |
| tests/registry/core/test_namespace_description.py | Updates fixture to use get_namespace_description_registry(). |
| tests/registry/core/test_formattypes.py | Updates fixture to use get_format_types_registry(); updates singleton test (but currently asserts function identity). |
| tests/registry/core/test_coding_format.py | Updates fixture to use get_coding_format_registry(). |
| tests/integration/test_format_types_integration.py | Uses get_format_types_registry() in integration tests; singleton test currently asserts function identity. |
| tests/integration/test_custom_registration.py | Switches custom registration tests to get_uuid_registry(). |
| tests/gatt/test_uuid_registry_lifecycle.py | Adds lifecycle tests for lazy-load, single-load, and sticky-failure semantics. |
| tests/conftest.py | Updates test fixtures to clear custom registrations via get_uuid_registry(). |
| tests/benchmarks/test_performance.py | Adds benchmarks for UUID registry cold vs warm lifecycle paths. |
| tests/benchmarks/conftest.py | Updates benchmark input payloads to better match expected wire formats. |
| src/bluetooth_sig/utils/prewarm.py | Refactors prewarm to execute loaders from the new prewarm catalogue. |
| src/bluetooth_sig/utils/prewarm_catalog.py | New discovery-based catalogue that finds registries and returns prewarm loaders. |
| src/bluetooth_sig/types/uri.py | Switches to get_uri_schemes_registry() for URI scheme lookup. |
| src/bluetooth_sig/types/company.py | Switches to get_company_identifiers_registry() for company ID lookup. |
| src/bluetooth_sig/types/appearance.py | Switches to get_appearance_values_registry() for appearance lookup. |
| src/bluetooth_sig/registry/uuids/units.py | Replaces module global with get_units_registry() and updates helper usage. |
| src/bluetooth_sig/registry/uuids/service_classes.py | Replaces module global with get_service_classes_registry(). |
| src/bluetooth_sig/registry/uuids/sdo_uuids.py | Replaces module global with get_sdo_uuids_registry(). |
| src/bluetooth_sig/registry/uuids/protocol_identifiers.py | Replaces module global with get_protocol_identifiers_registry() and updates __all__. |
| src/bluetooth_sig/registry/uuids/object_types.py | Replaces module global with get_object_types_registry(). |
| src/bluetooth_sig/registry/uuids/mesh_profiles.py | Replaces module global with get_mesh_profiles_registry(). |
| src/bluetooth_sig/registry/uuids/members.py | Replaces module global with get_members_registry(). |
| src/bluetooth_sig/registry/uuids/declarations.py | Replaces module global with get_declarations_registry(). |
| src/bluetooth_sig/registry/uuids/browse_groups.py | Replaces module global with get_browse_groups_registry(). |
| src/bluetooth_sig/registry/uuids/init.py | Re-exports getters instead of module-level instances. |
| src/bluetooth_sig/registry/service_discovery/attribute_ids.py | Replaces module global with get_service_discovery_attribute_registry(). |
| src/bluetooth_sig/registry/service_discovery/init.py | Re-exports getter for service discovery attribute registry. |
| src/bluetooth_sig/registry/profiles/profile_lookup.py | Replaces module global with get_profile_lookup_registry(). |
| src/bluetooth_sig/registry/profiles/permitted_characteristics.py | Replaces module global with get_permitted_characteristics_registry(). |
| src/bluetooth_sig/registry/profiles/init.py | Re-exports profile registry getters. |
| src/bluetooth_sig/registry/gss.py | Removes custom singleton plumbing; adds get_gss_registry(). |
| src/bluetooth_sig/registry/core/uri_schemes.py | Replaces module global with get_uri_schemes_registry() and updates examples. |
| src/bluetooth_sig/registry/core/namespace_description.py | Replaces module global with get_namespace_description_registry() and updates examples. |
| src/bluetooth_sig/registry/core/formattypes.py | Replaces module global with get_format_types_registry(). |
| src/bluetooth_sig/registry/core/coding_format.py | Replaces module global with get_coding_format_registry() and updates examples. |
| src/bluetooth_sig/registry/core/class_of_device.py | Replaces module global with get_class_of_device_registry(). |
| src/bluetooth_sig/registry/core/appearance_values.py | Replaces module global with get_appearance_values_registry(). |
| src/bluetooth_sig/registry/core/ad_types.py | Replaces module global with get_ad_types_registry(). |
| src/bluetooth_sig/registry/core/init.py | Re-exports core registry getters instead of instances. |
| src/bluetooth_sig/registry/company_identifiers/company_identifiers_registry.py | Replaces module global with get_company_identifiers_registry(). |
| src/bluetooth_sig/registry/company_identifiers/init.py | Re-exports getter for company identifiers registry. |
| src/bluetooth_sig/registry/base.py | Updates singleton typing/locking primitives and generic return typing for get_instance(). |
| src/bluetooth_sig/gatt/uuid_registry.py | Introduces explicit singleton getter, lazy-load on first access, and sticky failure state. |
| src/bluetooth_sig/gatt/services/registry.py | Switches service registry lookups to get_uuid_registry(). |
| src/bluetooth_sig/gatt/services/base.py | Switches service metadata lookups to get_uuid_registry(). |
| src/bluetooth_sig/gatt/resolver.py | Switches registry lookups to get_uuid_registry(). |
| src/bluetooth_sig/gatt/descriptors/characteristic_presentation_format.py | Switches dependent registry lookups to getter-based access. |
| src/bluetooth_sig/gatt/descriptors/base.py | Switches descriptor resolution to get_uuid_registry(). |
| src/bluetooth_sig/gatt/characteristics/registry.py | Switches characteristic registry lookups to get_uuid_registry(). |
| src/bluetooth_sig/gatt/characteristics/preferred_units.py | Switches units lookup/validation to get_units_registry(). |
| src/bluetooth_sig/gatt/characteristics/context_lookup.py | Switches characteristic UUID lookup to get_uuid_registry(). |
| src/bluetooth_sig/gatt/characteristics/characteristic_meta.py | Switches YAML/spec and units lookups to getter-based access. |
| src/bluetooth_sig/gatt/characteristics/appearance.py | Switches appearance value lookup to get_appearance_values_registry(). |
| src/bluetooth_sig/gatt/init.py | Re-exports get_uuid_registry instead of uuid_registry. |
| src/bluetooth_sig/core/registration.py | Switches registration helpers to get_uuid_registry(). |
| src/bluetooth_sig/core/query.py | Switches query helpers to get_uuid_registry(). |
| src/bluetooth_sig/advertising/registry.py | Adds thread-safe singleton accessor get_payload_interpreter_registry() and updates defaults. |
| src/bluetooth_sig/advertising/pdu_parser.py | Switches registry access to getter-based APIs for AD types/appearance/CoD. |
| src/bluetooth_sig/advertising/base.py | Registers interpreters via get_payload_interpreter_registry(). |
| src/bluetooth_sig/advertising/init.py | Re-exports get_payload_interpreter_registry instead of the module-level registry. |
| docs/source/performance/performance-data.md | Adds cold vs warm UUID registry lifecycle performance section. |
| docs/source/how-to/advertising-parsing.md | Updates examples to use get_payload_interpreter_registry(). |
| docs/source/explanation/architecture/registry-system.md | Updates docs to describe sticky-failure deterministic loading. |
| docs/source/explanation/architecture/decisions.md | Adds ADR note requiring side-effect-free imports and getter-based singleton access. |
| .gitignore | Ignores .tmp/ directory. |
Comment on lines
150
to
+155
| def test_singleton_instance(self) -> None: | ||
| """Test that ad_types_registry is a singleton.""" | ||
| from bluetooth_sig.registry.core.ad_types import ad_types_registry as imported_registry | ||
| """Test that get_ad_types_registry is a singleton.""" | ||
| from bluetooth_sig.registry.core.ad_types import get_ad_types_registry as imported_registry | ||
|
|
||
| # Should be the same instance | ||
| assert ad_types_registry is imported_registry | ||
| assert get_ad_types_registry is imported_registry |
There was a problem hiding this comment.
This test currently asserts that the getter function object is identical across imports, which does not validate the singleton instance behavior. Update the test to call both getters and assert they return the same registry instance (e.g., compare get_ad_types_registry() from both imports).
Comment on lines
169
to
+174
| def test_singleton_instance(self) -> None: | ||
| """Test that format_types_registry is a singleton.""" | ||
| from bluetooth_sig.registry.core.formattypes import format_types_registry as imported_registry | ||
| """Test that get_format_types_registry is a singleton.""" | ||
| from bluetooth_sig.registry.core.formattypes import get_format_types_registry as imported_registry | ||
|
|
||
| # Should be the same instance | ||
| assert format_types_registry is imported_registry | ||
| assert get_format_types_registry is imported_registry |
There was a problem hiding this comment.
This test compares the imported getter function objects rather than the registry singleton instances. To validate singleton semantics, call the getter(s) and assert the returned FormatTypesRegistry objects are identical (same id).
Comment on lines
53
to
+60
| def test_registry_singleton(self) -> None: | ||
| """Test that format_types_registry is a proper singleton.""" | ||
| from bluetooth_sig.registry.core import format_types_registry as imported_registry | ||
| from bluetooth_sig.registry.core import format_types_registry as main_imported_registry | ||
| """Test that get_format_types_registry is a proper singleton.""" | ||
| from bluetooth_sig.registry.core import get_format_types_registry as imported_registry | ||
| from bluetooth_sig.registry.core import get_format_types_registry as main_imported_registry | ||
|
|
||
| # All imports should reference the same instance | ||
| assert format_types_registry is imported_registry | ||
| assert format_types_registry is main_imported_registry | ||
| assert get_format_types_registry is imported_registry | ||
| assert get_format_types_registry is main_imported_registry |
There was a problem hiding this comment.
This singleton test currently checks that the getter function objects are the same across imports, which doesn't prove the registry is a singleton. Change this to compare the returned instances from get_format_types_registry() across the import paths.
Comment on lines
162
to
+165
| # Global singleton instance | ||
| uri_schemes_registry = UriSchemesRegistry() | ||
| def get_uri_schemes_registry() -> UriSchemesRegistry: | ||
| """Return the process-wide get_uri_schemes_registry singleton instance.""" | ||
| return UriSchemesRegistry.get_instance() |
There was a problem hiding this comment.
The docstring says "Return the process-wide get_uri_schemes_registry singleton instance" (repeats the function name). Consider rewording to describe the returned object (e.g., "Return the process-wide URI schemes registry singleton instance") to avoid confusion in generated docs.
Comment on lines
169
to
+172
| # Global singleton instance | ||
| namespace_description_registry = NamespaceDescriptionRegistry() | ||
| def get_namespace_description_registry() -> NamespaceDescriptionRegistry: | ||
| """Return the process-wide get_namespace_description_registry singleton instance.""" | ||
| return NamespaceDescriptionRegistry.get_instance() |
There was a problem hiding this comment.
The docstring currently says "Return the process-wide get_namespace_description_registry singleton instance" (repeats the function name). Reword to describe the returned singleton registry instance for clarity in documentation.
Comment on lines
144
to
+147
| # Global singleton instance | ||
| coding_format_registry = CodingFormatRegistry() | ||
| def get_coding_format_registry() -> CodingFormatRegistry: | ||
| """Return the process-wide get_coding_format_registry singleton instance.""" | ||
| return CodingFormatRegistry.get_instance() |
There was a problem hiding this comment.
The getter docstring says "Return the process-wide get_coding_format_registry singleton instance" (repeats the function name). Reword to describe the returned CodingFormatRegistry singleton instance to keep docs clear.
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
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 pull request refactors the way singleton registries are accessed throughout the codebase, replacing direct module-level global variables with explicit getter/class methods. This improves encapsulation, thread safety, and makes registry lifecycle management more predictable. Additionally, the documentation and performance data are updated to reflect these changes and clarify registry loading behavior.
Registry singleton access and API changes:
Replaced all direct usage of module-level singleton registries (e.g.,
payload_interpreter_registry,uuid_registry,appearance_values_registry, etc.) with explicit getter methods likeget_payload_interpreter_registry(),get_uuid_registry(), and similar for all relevant registries. This includes updates in both library code and documentation examples. [1] [2] [3] [4] [5] [6] [7]Implemented thread-safe singleton pattern for
PayloadInterpreterRegistryand similar registries using a class method and lock, ensuring only one instance exists per process.Updated all relevant
__init__.pyfiles and public APIs to export the new getter functions instead of the previous singleton variables. [1] [2] [3]Documentation and example updates:
Updated documentation and code examples to use the new getter-based access for registries, ensuring consistency and clarity for users. [1] [2] [3]
Added an implementation note to the architecture decision records specifying that registry modules must be imported side-effect free and accessed only via explicit getter methods.
Registry loading and performance:
Changed registry loading failure behavior to be deterministic and sticky, rather than silently degrading with partial data. Updated documentation to reflect this and to clarify that registry loading is performed only once per application lifetime.
Added a new section to the performance documentation detailing registry lifecycle paths and the impact of cold vs. warm registry loads.