feat: enhance diagnostics, docs, and CLI tests by eman · Pull Request #50 · eman/nwp500-python

eman · 2025-12-17T22:50:01Z

Overview

This PR enhances the nwp500 library with diagnostics capabilities, modernizes the codebase to Python 3.13+, and fixes various issues discovered during development.

Breaking Changes

⚠️ Python 3.13+ Required: Minimum Python version raised from 3.9 to 3.13

Home Assistant has deprecated Python 3.12 support
Enables use of modern Python features and performance improvements
Users on older Python versions should use version 6.1.x

Major Changes

1. MQTT Diagnostics & Monitoring

Integrated MqttDiagnosticsCollector into NavienMqttClient for connection stability analysis
Added diagnostic script (scripts/diagnose_mqtt_connection.py) for troubleshooting
Records connection drops, reconnections, and MQTT traffic patterns
Export diagnostics to JSON for analysis

2. Python Modernization (3.13+ Features)

Python 3.10: Native union syntax (X | Y instead of Union[X, Y])
Python 3.11: Self type for context manager return types
Python 3.12: PEP 695 type parameter syntax (def func[T](...))
Python 3.13: datetime.UTC constant, JIT compiler benefits
Cleaner, more maintainable type annotations throughout

3. CLI Enhancements

Comprehensive unit tests for all CLI command handlers
Refactored JSON formatting to shared utility function
Better test coverage and code organization

4. Bug Fixes

Fixed deprecated datetime.utcnow() → datetime.now(UTC) (4 instances)
Fixed type annotation: month: list[int] | None (was list[int | None])
Fixed test issues to avoid bound method docstring problems
Documentation fixes for field name inconsistencies

Technical Improvements

Type Safety

Replaced all Union[X, Y] with X | Y syntax
Replaced all Optional[X] with X | None syntax
Used Self type for methods returning self
Modern generic syntax with PEP 695

Code Quality

Consistent timezone-aware datetime usage
Removed all deprecated datetime methods
Better import organization (collections.abc)
Added strict=True to zip() calls for safety

Testing

Tests on Python 3.13 and 3.14
All CI checks passing (lint, format, tests, security)
Comprehensive CLI command test coverage

Files Changed

Core Library:

src/nwp500/mqtt_client.py - Integrated diagnostics collector
src/nwp500/mqtt_diagnostics.py - Modernized datetime usage
src/nwp500/mqtt_command_queue.py - Updated type hints
src/nwp500/api_client.py - Used Self type
src/nwp500/auth.py - Used Self type
src/nwp500/models.py - Fixed type annotation
src/nwp500/utils.py - PEP 695 generics

Tests:

tests/test_cli_*.py - New CLI unit tests
tests/test_command_queue.py - Fixed datetime usage
tests/test_mqtt_client_init.py - Fixed docstring issue

Documentation:

docs/python_api/*.rst - Fixed field name examples
README.rst - Updated Python version requirement
CHANGELOG.rst - Consolidated all changes into v6.2.0

Configuration:

setup.cfg - Python 3.13+ requirement
pyproject.toml - Updated target version to py313
.github/workflows/ci.yml - Test on Python 3.13 and 3.14

Verification

✅ All linting checks pass (ruff)
✅ All formatting checks pass
✅ All tests pass on Python 3.13 and 3.14
✅ Type checking passes (mypy)
✅ Security scanning passes (CodeQL)
✅ Build distribution succeeds

Migration Guide

For users upgrading from v6.1.x:

Update Python version to 3.13 or 3.14
No API changes - all existing code continues to work
Performance improvements automatic from Python 3.13 JIT compiler

For users who need Python 3.9-3.12:

Stay on version 6.1.x
All features except diagnostics are available

- Integrate MqttDiagnosticsCollector into NavienMqttClient - Add diagnose_mqtt_connection.py script - Fix documentation inconsistencies (camelCase vs snake_case, units) - Add CLI unit tests (basic coverage and command mocking) - Fix deprecation warnings in utils and tests

In Python 3.9, asyncio.Queue() calls get_event_loop() during __init__, which fails when there's no running event loop. Since the queue is accessed synchronously (using put_nowait/get_nowait), we can use collections.deque instead, which doesn't require an event loop. This fixes CI failures on Python 3.9 while maintaining the same functionality - FIFO queue with automatic overflow dropping when full. Fixes: 6 test failures in test_mqtt_client_init.py

Copilot

Pull request overview

This PR enhances the nwp500 project by integrating diagnostics collection, fixing documentation inconsistencies, adding CLI unit tests, and addressing deprecation warnings. The changes improve observability of MQTT connections and expand test coverage while modernizing deprecated datetime usage.

Key changes:

Integrated MqttDiagnosticsCollector into NavienMqttClient for connection monitoring
Added diagnostic script for troubleshooting MQTT connections
Fixed documentation examples to match actual API field names
Replaced deprecated datetime.utcnow() with timezone-aware alternatives

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/nwp500/mqtt_client.py	Integrated diagnostics collector and added connection event recording
src/nwp500/mqtt_diagnostics.py	Updated type hints to use PEP 604 union syntax
src/nwp500/mqtt_command_queue.py	Replaced asyncio.Queue with deque for simpler synchronous queue management
src/nwp500/mqtt_*.py	Standardized type hints across MQTT modules using modern union syntax
src/nwp500/utils.py	Replaced asyncio.iscoroutinefunction with inspect.iscoroutinefunction
tests/test_mqtt_client_init.py	Fixed test to avoid bound method docstring issues
tests/test_command_queue.py	Replaced deprecated datetime.utcnow() with timezone-aware datetime.now(timezone.utc)
tests/test_cli_*.py	Added comprehensive unit tests for CLI command handlers
scripts/diagnose_mqtt_connection.py	Added diagnostic tool for monitoring MQTT connection stability
scripts/lint.py	Updated to use sys.executable instead of hardcoded python3
scripts/extract_changelog.py	Removed redundant file mode and added noqa comment for long line
docs/python_api/*.rst	Fixed field name inconsistencies in documentation examples

Comments suppressed due to low confidence (3)

src/nwp500/mqtt_diagnostics.py:181

Use datetime.now(timezone.utc) instead of deprecated datetime.utcnow(). The current approach creates a timezone-naive datetime and manually appends 'Z', when timezone-aware datetimes handle this properly.

        now = datetime.utcnow().isoformat() + "Z"

src/nwp500/mqtt_diagnostics.py:271

Use datetime.now(timezone.utc) instead of deprecated datetime.utcnow(). Same issue as Comment 2 - creates timezone-naive datetime and manually appends 'Z'.

        now = datetime.utcnow().isoformat() + "Z"

src/nwp500/mqtt_diagnostics.py:372

Use datetime.now(timezone.utc) instead of deprecated datetime.utcnow(). Consistent with previous comments - this deprecated usage should be replaced for consistency with test_command_queue.py changes.

            "timestamp": datetime.utcnow().isoformat() + "Z",

src/nwp500/mqtt_command_queue.py

tests/test_cli_commands.py

…utc) Using datetime.now(timezone.utc) instead of the deprecated utcnow() to create timezone-aware datetime objects and avoid subtle bugs. Consistent with fixes in test_command_queue.py.

- Use native union syntax (X | Y instead of Union[X, Y]) - Use native optional syntax (X | None instead of Optional[X]) - Import Callable from collections.abc instead of typing - Add strict=True to zip() calls for safety These changes leverage Python 3.10+ features now that we've dropped Python 3.9 support.

- Remove unused Optional and Union imports - Consolidate collections.abc imports on single line - Fix import ordering per isort rules - Fix line length on zip() call with strict parameter

The return type in Callable must be separate from args. Changed Callable[[Any, Any | None, None]] to Callable[[Any, Any | None], None]

Changed Callable[..., None | None] to Callable[..., None] | None The return type was incorrectly specified as None | None which is invalid.

All changes in this PR are part of version 6.2.0. Removed artificial version 7.0.0 and 8.0.0 entries and consolidated all breaking changes and features into the single upcoming release.

Test on both Python 3.13 and 3.14 to ensure forward compatibility.

Copilot

Pull request overview

Copilot reviewed 33 out of 34 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

src/nwp500/models.py:1

The type annotation list[int | None] should be list[int] | None because month is an optional list of integers, not a list that can contain None values. The current annotation incorrectly suggests the list can contain None as an element.

"""Data models for Navien NWP500 water heater communication.

Copilot · 2025-12-18T01:11:59Z

src/nwp500/models.py

    @staticmethod
    def _convert_enums_to_names(
-        data: Any, visited: Optional[set[int]] = None
+        data: Any, visited: set[int | None] = None


The type annotation set[int | None] should be set[int] | None because visited is an optional set of integers, not a set that can contain None values. The current annotation incorrectly suggests the set can contain None as an element.

Suggested change

data: Any, visited: set[int | None] = None

data: Any, visited: set[int] | None = None

Copilot · 2025-12-18T01:11:59Z

src/nwp500/models.py

-        context: Optional[dict[str, Any]] = None,
+        strict: bool | None = None,
+        from_attributes: bool | None = None,
+        context: dict[str, Any | None] = None,


The type annotation dict[str, Any | None] should be dict[str, Any] | None because context is an optional dictionary, not a dictionary with optional values. The current annotation incorrectly suggests the dictionary values can be None rather than the entire parameter being optional.

Suggested change

context: dict[str, Any | None] = None,

context: dict[str, Any] | None = None,

Copilot · 2025-12-18T01:11:59Z

src/nwp500/mqtt_connection.py

        self,
        topic: str,
-        payload: Union[str, dict[str, Any], Any],
+        payload: str | dict[str, Any, Any],


The type annotation dict[str, Any, Any] is invalid syntax. Dictionary type hints should be dict[KeyType, ValueType] with only two type parameters. This should be dict[str, Any] instead.

Suggested change

payload: str | dict[str, Any, Any],

payload: str | bytes | dict[str, Any],

Copilot · 2025-12-18T01:12:00Z

src/nwp500/exceptions.py

-        error_code: Optional[str] = None,
-        details: Optional[dict[str, Any]] = None,
+        error_code: str | None = None,
+        details: dict[str, Any | None] = None,


The type annotation dict[str, Any | None] should be dict[str, Any] | None because details is an optional dictionary parameter, not a dictionary with optional values. The current annotation incorrectly suggests the dictionary values can be None rather than the entire parameter being optional.

Suggested change

details: dict[str, Any | None] = None,

details: dict[str, Any] | None = None,

Copilot · 2025-12-18T01:12:00Z

src/nwp500/exceptions.py

-        status_code: Optional[int] = None,
-        response: Optional[dict[str, Any]] = None,
+        status_code: int | None = None,
+        response: dict[str, Any | None] = None,


The type annotation dict[str, Any | None] should be dict[str, Any] | None because response is an optional dictionary parameter, not a dictionary with optional values. The current annotation incorrectly suggests the dictionary values can be None rather than the entire parameter being optional.

Copilot · 2025-12-18T01:12:00Z

src/nwp500/exceptions.py

-        code: Optional[int] = None,
-        response: Optional[dict[str, Any]] = None,
+        code: int | None = None,
+        response: dict[str, Any | None] = None,


The type annotation dict[str, Any | None] should be dict[str, Any] | None because response is an optional dictionary parameter, not a dictionary with optional values. The current annotation incorrectly suggests the dictionary values can be None rather than the entire parameter being optional.

Copilot · 2025-12-18T01:12:00Z

src/nwp500/events.py


    def off(
-        self, event: str, callback: Optional[Callable[..., Any]] = None
+        self, event: str, callback: Callable[..., Any | None] = None


The type annotation Callable[..., Any | None] should be Callable[..., Any] | None because callback is an optional callable parameter, not a callable that returns an optional value. The current annotation incorrectly suggests the callable returns an optional value rather than the entire parameter being optional.

Suggested change

self, event: str, callback: Callable[..., Any | None] = None

self, event: str, callback: Callable[..., Any] | None = None

Copilot · 2025-12-18T01:12:01Z

src/nwp500/api_client.py

+        json_data: dict[str, Any | None] = None,
+        params: dict[str, Any | None] = None,


The type annotations dict[str, Any | None] should be dict[str, Any] | None because these are optional dictionary parameters, not dictionaries with optional values. The current annotations incorrectly suggest the dictionary values can be None rather than the entire parameters being optional.

Suggested change

json_data: dict[str, Any | None] = None,

params: dict[str, Any | None] = None,

json_data: dict[str, Any] | None = None,

params: dict[str, Any] | None = None,

- Replace deprecated datetime.utcnow() with datetime.now(UTC) in mqtt_diagnostics.py (3 instances) - Fix type annotation: month: list[int] | None (not list[int | None]) The field is an optional list of integers, not a list that can contain None values

Replace quoted forward references with Self type (PEP 673): - NavienAPIClient.__aenter__ now returns Self instead of 'NavienAPIClient' - NavienAuthClient.__aenter__ now returns Self instead of 'NavienAuthClient' Benefits: - Cleaner type annotations - Better type inference - More maintainable (no need to update quoted class names)

Updated documentation to reflect Python 3.13+ requirement: - docs/installation.rst: Prerequisites section - docs/quickstart.rst: Prerequisites section - docs/development/history.rst: Current status references Historical references to Python 3.9 migration remain unchanged as they document past milestones.

- Updated from 1.26.0 to latest version 1.27.0 - Made version consistent across all documentation - Removed incorrect dependencies from README (websockets, cryptography) These are transitive dependencies, not direct requirements All documentation now shows consistent dependency versions.

eman added 11 commits December 17, 2025 14:35

fix: resolve linting errors

f856047

fix: make lint script respect active python environment

90d9b15

test: fix datetime.UTC attribute error in command queue tests

ef09bcf

test: fix docstring check to be more robust

6a03f5e

fix: add future annotations to diagnostics and utils for py39 compat

4c18aa4

fix: resolve remaining lint/type errors (UP045, whitepace)

8767ece

fix: add future annotations to remaining modules for py39 compat

d69ca26

style: fix formatting in mqtt_periodic.py

795f44c

fix: Remove unused asyncio import

678f4a5

eman requested a review from Copilot December 17, 2025 23:45

Copilot AI reviewed Dec 17, 2025

View reviewed changes

src/nwp500/mqtt_command_queue.py Outdated Show resolved Hide resolved

tests/test_cli_commands.py Outdated Show resolved Hide resolved

eman added 13 commits December 17, 2025 15:50

refactor: Condense verbose test comment

c9d8d19

fix: Replace deprecated datetime.utcnow() with datetime.now(timezone.…

e4b1400

…utc) Using datetime.now(timezone.utc) instead of the deprecated utcnow() to create timezone-aware datetime objects and avoid subtle bugs. Consistent with fixes in test_command_queue.py.

fix: Fix line length issues for linting

319892d

feat: Drop Python 3.9, require Python 3.10+

24d75a3

fix: Clean up imports and fix linting issues

ae43327

- Remove unused Optional and Union imports - Consolidate collections.abc imports on single line - Fix import ordering per isort rules - Fix line length on zip() call with strict parameter

fix: Correct Callable type annotation syntax

8b6a632

The return type in Callable must be separate from args. Changed Callable[[Any, Any | None, None]] to Callable[[Any, Any | None], None]

style: Auto-format models.py with ruff

973b930

fix: Correct invalid None | None type annotation

7e35665

Changed Callable[..., None | None] to Callable[..., None] | None The return type was incorrectly specified as None | None which is invalid.

feat: Drop Python 3.10-3.11, require Python 3.12+

a722dc7

fix: Consolidate changelog into single version 6.2.0

a459304

All changes in this PR are part of version 6.2.0. Removed artificial version 7.0.0 and 8.0.0 entries and consolidated all breaking changes and features into the single upcoming release.

feat: Drop Python 3.12, require Python 3.13+

9f04ab0

ci: Add Python 3.14 to test matrix

28e5931

Test on both Python 3.13 and 3.14 to ensure forward compatibility.

eman requested a review from Copilot December 18, 2025 01:09

Copilot AI reviewed Dec 18, 2025

View reviewed changes

eman added 2 commits December 17, 2025 17:14

fix: Address PR review comments

617997b

- Replace deprecated datetime.utcnow() with datetime.now(UTC) in mqtt_diagnostics.py (3 instances) - Fix type annotation: month: list[int] | None (not list[int | None]) The field is an optional list of integers, not a list that can contain None values

eman added 4 commits December 17, 2025 17:43

update the version to 7.0.0

485cf6e

remove unintentional file

be8e8ea

eman merged commit e61f7f2 into main Dec 18, 2025
7 checks passed

eman deleted the improvement/diagnostics-and-docs branch December 18, 2025 02:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enhance diagnostics, docs, and CLI tests#50

feat: enhance diagnostics, docs, and CLI tests#50
eman merged 30 commits intomainfrom
improvement/diagnostics-and-docs

eman commented Dec 17, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	data: Any, visited: set[int \| None] = None
	data: Any, visited: set[int] \| None = None

	context: dict[str, Any \| None] = None,
	context: dict[str, Any] \| None = None,

	payload: str \| dict[str, Any, Any],
	payload: str \| bytes \| dict[str, Any],

	details: dict[str, Any \| None] = None,
	details: dict[str, Any] \| None = None,

	self, event: str, callback: Callable[..., Any \| None] = None
	self, event: str, callback: Callable[..., Any] \| None = None

		json_data: dict[str, Any \| None] = None,
		params: dict[str, Any \| None] = None,

Conversation

eman commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Breaking Changes

Major Changes

1. MQTT Diagnostics & Monitoring

2. Python Modernization (3.13+ Features)

3. CLI Enhancements

4. Bug Fixes

Technical Improvements

Type Safety

Code Quality

Testing

Files Changed

Verification

Migration Guide

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eman commented Dec 17, 2025 •

edited

Loading