refactor!: trim public API, remove dead code, deduplicate conversions#95
Merged
Conversation
added 2 commits
July 5, 2026 08:23
- Send flat raw protocol payloads for weekly reservation and recirculation schedules; the model dump double-nested the request and leaked computed display fields (incl. unit-converted temperature) into device commands. Add NavienBaseModel.to_protocol_dict() - Preserve command queue order on failed flush (deque with re-insert at front instead of tail) - Discard queued commands older than max_queued_command_age (default 300s) instead of replaying stale control commands after long outages - Use truncated remainder (firmware semantics) instead of Python's floored modulo in the ASYMMETRIC Fahrenheit formula; fixes off-by-one conversions for sub-zero temperatures - Correct freeze protection default limits to raw half-Celsius 12/20 (43F/50F documented fixed range) instead of 43/65 - Emit error_detected when the error code changes between two non-zero codes - Round TOU prices half-up via Decimal (round() banker's rounding under-encoded exact halves); encode bool prices instead of passing them through as pre-encoded ints - Fix inverted enable-flag doc, wrong week-bitfield example (158->84), and int/float doctest values
BREAKING CHANGES (major version bump required): - Remove deprecated NavienMqttClient.control property; use the delegated methods on the client directly - Remove never-raised exception classes: TokenExpiredError, MqttSubscriptionError, DeviceNotFoundError, DeviceOfflineError, DeviceOperationError (catch the parent classes instead) - Stop re-exporting internal plumbing from the top-level namespace: requires_capability, MqttDeviceInfoCache, MqttDeviceCapabilityChecker, log_performance, and the eight bit-encoding helpers (import from nwp500.encoding et al.) Cleanup: - Delete dead nwp500.cli.commands module (never imported, drifted from the real click commands) - Delete unused converters.str_enum_validator and module-level temperature conversion wrappers; simplify div_10/mul_10 - Delete no-op NavienMqttClient._on_message_received placeholder and unused CLI width calculations - Deduplicate the four Temperature classes via a _scale ClassVar on the base (behavior unchanged, ~200 lines removed); doctests pass - Deduplicate field_factory metadata-merge boilerplate - Use converters.device_bool_from_python in mqtt/control.py instead of inline 2/1 literals - Resolve auth.py version from distribution metadata, removing the order-dependent 'from . import __version__' near-circular import - Update docs and examples to import encoding helpers from nwp500.encoding; add public-API surface tests
…-cleanup # Conflicts: # CHANGELOG.rst # src/nwp500/encoding.py # src/nwp500/mqtt/client.py # src/nwp500/temperature.py # tests/test_protocol_correctness.py
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs an architecture cleanup/refactor of nwp500, trimming the top-level public API, removing dead code and never-raised exceptions, and deduplicating a few repeated conversion/field-construction implementations. It aligns the library with the documented “no backward compatibility” policy by removing a deprecated shim rather than carrying it forward.
Changes:
- Remove deprecated/unused public API elements (e.g.,
NavienMqttClient.control, never-raised exception classes, and several internal helpers previously re-exported vianwp500.__all__). - Deduplicate core implementations (temperature conversions via per-class scaling; shared
Field(..., json_schema_extra=...)construction; centralized device-bool encoding). - Update tests, docs, examples, and changelog to reflect the trimmed public surface and new import paths.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_public_api.py | Adds coverage asserting __all__ names resolve and that removed/internal names are not exported. |
| tests/test_exceptions.py | Removes tests for exception classes deleted from the library. |
| tests/test_auth.py | Removes tests that referenced the deleted TokenExpiredError. |
| src/nwp500/temperature.py | Deduplicates temperature conversions via a shared Temperature implementation with per-class _scale. |
| src/nwp500/mqtt/control.py | Uses device_bool_from_python() to centralize protocol boolean encoding. |
| src/nwp500/mqtt/client.py | Removes a no-op message callback placeholder and drops the deprecated .control property. |
| src/nwp500/field_factory.py | Extracts repeated json_schema_extra merge logic into _metadata_field(). |
| src/nwp500/exceptions.py | Removes never-raised exception classes and updates the documented hierarchy. |
| src/nwp500/converters.py | Removes dead str_enum_validator() and simplifies div_10() / mul_10() by removing redundant branches. |
| src/nwp500/cli/output_formatters.py | Removes unused line-width calculations in CLI formatting. |
| src/nwp500/cli/commands.py | Deletes an unused/never-imported CLI command registry module. |
| src/nwp500/auth.py | Resolves __version__ via distribution metadata to avoid near-circular import with __init__.py. |
| src/nwp500/init.py | Trims __all__ exports by removing re-exports of internal plumbing and deleted exception types. |
| examples/advanced/tou_openei.py | Updates imports to pull encoding helpers from nwp500.encoding instead of the top-level package. |
| docs/reference/python_api/mqtt_client.rst | Updates docs examples to import encoding helpers from nwp500.encoding. |
| docs/reference/python_api/exceptions.rst | Removes docs for deleted exceptions and updates hierarchy presentation. |
| docs/reference/python_api/auth_client.rst | Removes TokenExpiredError mentions from auth error-handling docs. |
| docs/how-to/schedule-operation.rst | Updates how-to guide imports for build_reservation_entry to use nwp500.encoding. |
| docs/how-to/optimize-tou.rst | Updates how-to guide imports for encoding helpers to use nwp500.encoding. |
| CHANGELOG.rst | Documents breaking removals and the refactor/deduplication work under “Unreleased”. |
This was referenced Jul 6, 2026
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.
Sixth batch from the comprehensive repo evaluation: architecture cleanup. Based on #94 (
fix/protocol-correctness) because both touchtemperature.py/control.py— merge #94 first, then retarget/merge this one.Contains breaking changes — requires a major version bump.
Removed (breaking)
.controlproperty onNavienMqttClient: the@warnings.deprecatedshim slated for v9.0.0 contradicts the project's documented no-backward-compat policy (remove immediately, don't deprecate). Use the delegated methods directly (client.set_power(...)).TokenExpiredError,MqttSubscriptionError,DeviceNotFoundError,DeviceOfflineError,DeviceOperationErrorhad zero raise sites in the library, so no working error handling can break. CatchAuthenticationError/MqttError/DeviceErrorinstead. Hierarchy docs updated.__all__103 → 87):requires_capability,MqttDeviceInfoCache,MqttDeviceCapabilityChecker,log_performance, and the eight bit-encoding helpers. All remain importable from their owning modules (nwp500.encoding, etc.); docs and examples updated to match.nwp500/cli/commands.py(never imported anywhere; its command metadata had drifted from the real click commands),converters.str_enum_validator, unused module-leveltemperature.half_celsius_to_fahrenheit/deci_celsius_to_fahrenheitwrappers, the no-opNavienMqttClient._on_message_receivedplaceholder (real dispatch lives in the subscription manager), duplicated-then-unused_line_widthcalculations in the CLI formatters, and identical-branchisinstanceconditionals indiv_10/mul_10.Deduplicated (behavior unchanged)
HalfCelsius/DeciCelsius/RawCelsius/DeciCelsiusDeltawere four near-identical copies differing only in a scale constant. Conversions are now implemented once onTemperaturewith a per-class_scale: ClassVar; onlyRawCelsius(formula-specific rounding) andDeciCelsiusDelta(no ±32 offset) override. Doctests all pass.json_schema_extramerge block was pasted 4×; extracted into one private_metadata_field()helper.mqtt/control.pynow usesconverters.device_bool_from_python()instead of inline2 if enabled else 1literals.auth.pyresolves the version from distribution metadata instead offrom . import __version__(an order-dependent near-circular import with__init__.py).Net: −575 lines.
Deferred follow-ups (tracked from the evaluation, too large for one PR)
mqtt/client.py(~1500 lines) into a thin façade over its helper modulesoutput_formatters.py+rich_output.py, ~2100 lines) behind a presentation-neutral layerevents.pystring emitter vs typedmqtt_events.py)computed_fielddependence on the global unit-system stateTests
New
tests/test_public_api.py: every__all__name resolves, internal plumbing absent, removed exceptions gone, encoding helpers importable fromnwp500.encoding. Removed the 7 tests that only exercised the deleted exception classes.Validation
pytest: 522 passedruff check/ruff format --check: cleanpyright src/nwp500: 0 errorsmypy src/nwp500: no issues