From b24dfc749c991f7086a9d86968382808532f4554 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Sun, 2 Nov 2025 10:50:55 -0800 Subject: [PATCH 1/7] BREAKING: Remove constructor callbacks in favor of event emitter - Remove on_connection_interrupted and on_connection_resumed parameters from NavienMqttClient - Update examples to use event emitter pattern (mqtt_client.on('connection_interrupted', ...)) - Update documentation to reflect new pattern - Add backward compatibility policy to Copilot instructions - Update CHANGELOG.rst with migration guide BREAKING CHANGE: Constructor callbacks removed. Use event emitter pattern instead: # Old (removed) mqtt_client = NavienMqttClient(auth, on_connection_interrupted=handler) # New mqtt_client = NavienMqttClient(auth) mqtt_client.on('connection_interrupted', handler) Benefits: - Multiple listeners per event - Consistent API with other events - Dynamic listener management - Async handler support --- .github/copilot-instructions.md | 40 +++++++++++++++++++++++++++++++++ CHANGELOG.rst | 38 +++++++++++++++++++++++++++++++ docs/python_api/mqtt_client.rst | 24 +++++++------------- examples/command_queue_demo.py | 14 +++++++----- examples/reconnection_demo.py | 27 +++++++++++----------- src/nwp500/mqtt_client.py | 25 --------------------- 6 files changed, 108 insertions(+), 60 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c226cd8..72e804c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -79,6 +79,46 @@ Report the results of these checks in your final summary. - `asyncio.CancelledError` - Task cancellation - Only catch exceptions you can handle; let unexpected exceptions propagate +## Backward Compatibility Policy + +**DO NOT maintain backward compatibility.** This library is young and has no external clients. + +- **Breaking changes are acceptable**: Make the best design decisions without worrying about breaking existing code +- **Remove deprecated code immediately**: Don't add deprecation warnings or transitional code - just remove it +- **Remove duplicate functionality**: If there are two ways to do the same thing, remove one +- **Clean up legacy patterns**: Remove old patterns, helper variables, or compatibility shims +- **Update documentation**: When making breaking changes: + 1. Document the change in `CHANGELOG.rst` under the appropriate version + 2. Explain what was removed/changed and why + 3. Provide clear migration guidance showing the old way vs. new way + 4. Update affected examples to use the new pattern + 5. Update relevant documentation files +- **Version bumping**: Breaking changes require a major version bump (see Version Management section) + +**Example changelog entry for breaking changes:** +```rst +Version X.0.0 (YYYY-MM-DD) +========================== + +**BREAKING CHANGES**: Description of what broke + +Removed +------- +- **Old Pattern**: Removed `old_function()` in favor of cleaner `new_function()` + + .. code-block:: python + + # OLD (removed) + result = client.old_function(arg) + + # NEW + result = client.new_function(arg) + +- **Duplicate Functionality**: Removed constructor callbacks in favor of event emitter pattern + - Removed `on_connection_interrupted` constructor parameter + - Use `client.on('connection_interrupted', handler)` instead +``` + ## Integration Points - **AWS IoT Core**: MQTT client uses `awscrt` and `awsiot` libraries for connection and messaging - **aiohttp**: Used for async HTTP requests to the Navien API diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 11f44d3..5a4692f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,44 @@ Changelog ========= +Version 6.0.0 (2025-11-02) +========================== + +**BREAKING CHANGES**: Removed constructor callbacks in favor of event emitter pattern + +Removed +------- + +- **Constructor Callbacks**: Removed ``on_connection_interrupted`` and ``on_connection_resumed`` constructor parameters from ``NavienMqttClient`` + + .. code-block:: python + + # OLD (removed in v6.0.0) + mqtt_client = NavienMqttClient( + auth_client, + on_connection_interrupted=on_interrupted, + on_connection_resumed=on_resumed, + ) + + # NEW (use event emitter pattern) + mqtt_client = NavienMqttClient(auth_client) + mqtt_client.on("connection_interrupted", on_interrupted) + mqtt_client.on("connection_resumed", on_resumed) + +- **Migration Benefits**: + + - Multiple listeners per event (not just one callback) + - Consistent API with other events (temperature_changed, mode_changed, etc.) + - Dynamic listener management (add/remove listeners at runtime) + - Async handler support + - Priority-based execution + +Changed +------- + +- Updated ``examples/command_queue_demo.py`` to use event emitter pattern +- Updated ``examples/reconnection_demo.py`` to use event emitter pattern + Version 5.0.2 (2025-10-31) ========================== diff --git a/docs/python_api/mqtt_client.rst b/docs/python_api/mqtt_client.rst index 1beca42..cbdd950 100644 --- a/docs/python_api/mqtt_client.rst +++ b/docs/python_api/mqtt_client.rst @@ -91,7 +91,7 @@ API Reference NavienMqttClient ---------------- -.. py:class:: NavienMqttClient(auth_client, config=None, on_connection_interrupted=None, on_connection_resumed=None) +.. py:class:: NavienMqttClient(auth_client, config=None) MQTT client for real-time device communication via AWS IoT Core. @@ -99,10 +99,6 @@ NavienMqttClient :type auth_client: NavienAuthClient :param config: Connection configuration (optional) :type config: MqttConnectionConfig or None - :param on_connection_interrupted: Callback for connection loss - :type on_connection_interrupted: Callable or None - :param on_connection_resumed: Callback for connection restoration - :type on_connection_resumed: Callable or None :raises ValueError: If auth_client not authenticated or missing AWS credentials **Example:** @@ -124,18 +120,15 @@ NavienMqttClient ) mqtt = NavienMqttClient(auth, config=config) - # With connection callbacks + # Register event handlers def on_interrupted(error): print(f"Connection lost: {error}") def on_resumed(return_code, session_present): print("Connection restored!") - mqtt = NavienMqttClient( - auth, - on_connection_interrupted=on_interrupted, - on_connection_resumed=on_resumed - ) + mqtt.on("connection_interrupted", on_interrupted) + mqtt.on("connection_resumed", on_resumed) Connection Methods ------------------ @@ -1033,6 +1026,8 @@ Best Practices .. code-block:: python + mqtt = NavienMqttClient(auth) + def on_interrupted(error): print(f"Connection lost: {error}") # Save state, notify user, etc. @@ -1041,11 +1036,8 @@ Best Practices print("Connection restored") # Re-request status, etc. - mqtt = NavienMqttClient( - auth, - on_connection_interrupted=on_interrupted, - on_connection_resumed=on_resumed - ) + mqtt.on("connection_interrupted", on_interrupted) + mqtt.on("connection_resumed", on_resumed) 4. **Use periodic requests for long-running monitoring:** diff --git a/examples/command_queue_demo.py b/examples/command_queue_demo.py index 0dfe794..dd2e97e 100644 --- a/examples/command_queue_demo.py +++ b/examples/command_queue_demo.py @@ -70,6 +70,12 @@ async def command_queue_demo(): auto_reconnect=True, ) + mqtt_client = NavienMqttClient( + auth_client, + config=config, + ) + + # Register event handlers def on_interrupted(error): print(f" āš ļø Connection interrupted: {error}") print(f" šŸ“ Queued commands: {mqtt_client.queued_commands_count}") @@ -78,12 +84,8 @@ def on_resumed(return_code, session_present): print(" āœ… Connection resumed!") print(f" šŸ“ Queued commands: {mqtt_client.queued_commands_count}") - mqtt_client = NavienMqttClient( - auth_client, - config=config, - on_connection_interrupted=on_interrupted, - on_connection_resumed=on_resumed, - ) + mqtt_client.on("connection_interrupted", on_interrupted) + mqtt_client.on("connection_resumed", on_resumed) # Step 3: Connect print("\n3. Connecting to AWS IoT...") diff --git a/examples/reconnection_demo.py b/examples/reconnection_demo.py index fae983a..f339b95 100644 --- a/examples/reconnection_demo.py +++ b/examples/reconnection_demo.py @@ -36,16 +36,6 @@ async def main(): print("MQTT Reconnection Demo") print("=" * 70) - # Connection callbacks - def on_interrupted(error): - print(f"\nāš ļø Connection interrupted: {error}") - print(" Automatic reconnection will begin...") - - def on_resumed(return_code, session_present): - print("\nāœ… Connection resumed!") - print(f" Return code: {return_code}") - print(f" Session present: {session_present}") - # Authenticate async with NavienAuthClient(email, password) as auth_client: print(f"āœ… Authenticated as: {auth_client.current_user.full_name}") @@ -69,14 +59,25 @@ def on_resumed(return_code, session_present): reconnect_backoff_multiplier=2.0, # Double the delay each time ) - # Create MQTT client with reconnection callbacks + # Create MQTT client mqtt_client = NavienMqttClient( auth_client, config=config, - on_connection_interrupted=on_interrupted, - on_connection_resumed=on_resumed, ) + # Register event handlers + def on_interrupted(error): + print(f"\nāš ļø Connection interrupted: {error}") + print(" Automatic reconnection will begin...") + + def on_resumed(return_code, session_present): + print("\nāœ… Connection resumed!") + print(f" Return code: {return_code}") + print(f" Session present: {session_present}") + + mqtt_client.on("connection_interrupted", on_interrupted) + mqtt_client.on("connection_resumed", on_resumed) + # Connect await mqtt_client.connect() print(f"āœ… MQTT Connected: {mqtt_client.client_id}") diff --git a/src/nwp500/mqtt_client.py b/src/nwp500/mqtt_client.py index 2ee279d..255e44a 100644 --- a/src/nwp500/mqtt_client.py +++ b/src/nwp500/mqtt_client.py @@ -126,8 +126,6 @@ def __init__( self, auth_client: NavienAuthClient, config: Optional[MqttConnectionConfig] = None, - on_connection_interrupted: Optional[Callable[[Exception], None]] = None, - on_connection_resumed: Optional[Callable[[Any, Any], None]] = None, ): """ Initialize the MQTT client. @@ -135,8 +133,6 @@ def __init__( Args: auth_client: Authentication client with valid tokens config: Optional connection configuration - on_connection_interrupted: Callback for connection interruption - on_connection_resumed: Callback for connection resumption Raises: ValueError: If auth client is not authenticated or AWS @@ -186,10 +182,6 @@ def __init__( self._connection: Optional[mqtt.Connection] = None self._connected = False - # User-provided callbacks - self._on_connection_interrupted = on_connection_interrupted - self._on_connection_resumed = on_connection_resumed - _logger.info( f"Initialized MQTT client with ID: {self.config.client_id}" ) @@ -234,19 +226,6 @@ def _on_connection_interrupted_internal( # Emit event self._schedule_coroutine(self.emit("connection_interrupted", error)) - # Call user callback if provided - if self._on_connection_interrupted: - try: - self._on_connection_interrupted(error) - except TypeError: - # Fallback for callbacks expecting no arguments - try: - self._on_connection_interrupted() # type: ignore - except (TypeError, AttributeError) as e: - _logger.error( - f"Error in connection_interrupted callback: {e}" - ) - # Delegate to reconnection handler if available if self._reconnection_handler and self.config.auto_reconnect: self._reconnection_handler.on_connection_interrupted(error) @@ -266,10 +245,6 @@ def _on_connection_resumed_internal( self.emit("connection_resumed", return_code, session_present) ) - # Call user callback - if self._on_connection_resumed: - self._on_connection_resumed(return_code, session_present) - # Delegate to reconnection handler to reset state if self._reconnection_handler: self._reconnection_handler.on_connection_resumed( From 303998830be4267743b04172cfa673268d6f212b Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Sun, 2 Nov 2025 10:57:16 -0800 Subject: [PATCH 2/7] Remove misleading 'legacy state' comment These variables are not legacy or backward compatibility code - they are essential connection state tracking used 33+ times throughout the codebase. - _connected: Simpler to check than _connection_manager.is_connected - _connection: Passed to MqttSubscriptionManager which needs raw connection - Both: Handle the case before connect() when _connection_manager is None The comment was misleading as it suggested these should be removed eventually, when in fact they serve important architectural purposes. --- src/nwp500/mqtt_client.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nwp500/mqtt_client.py b/src/nwp500/mqtt_client.py index 255e44a..decac41 100644 --- a/src/nwp500/mqtt_client.py +++ b/src/nwp500/mqtt_client.py @@ -178,7 +178,9 @@ def __init__( self._reconnect_task: Optional[asyncio.Task[None]] = None self._periodic_manager: Optional[MqttPeriodicRequestManager] = None - # Legacy state (kept for backward compatibility during transition) + # Connection state tracking + # Note: These track state at the client level and are simpler to check than + # self._connection_manager.is_connected (which can be None before connect()) self._connection: Optional[mqtt.Connection] = None self._connected = False From 68da77f9190ed431a6d10178d9b31199b4c57fd5 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Sun, 2 Nov 2025 10:59:10 -0800 Subject: [PATCH 3/7] Remove remaining misleading 'legacy state' comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changed: - 'Update legacy state for backward compatibility' ā†’ 'Update connection state' - 'Update legacy state' ā†’ 'Clear connection state' These variables are essential connection state tracking, not legacy code. --- src/nwp500/mqtt_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nwp500/mqtt_client.py b/src/nwp500/mqtt_client.py index decac41..3bc78f6 100644 --- a/src/nwp500/mqtt_client.py +++ b/src/nwp500/mqtt_client.py @@ -474,7 +474,7 @@ async def connect(self) -> bool: success = await self._connection_manager.connect() if success: - # Update legacy state for backward compatibility + # Update connection state self._connection = self._connection_manager.connection self._connected = True @@ -564,7 +564,7 @@ async def disconnect(self) -> None: # Delegate disconnection to connection manager await self._connection_manager.disconnect() - # Update legacy state + # Clear connection state self._connected = False self._connection = None From 05f764736583c5861b5642760753da23dcdd8aad Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Sun, 2 Nov 2025 11:03:18 -0800 Subject: [PATCH 4/7] Remove backward compatibility re-exports BREAKING CHANGE: Exception re-exports removed from api_client and auth modules. Removed: - APIError re-export from api_client module - AuthenticationError, InvalidCredentialsError, TokenExpiredError, TokenRefreshError re-exports from auth module Migration: # Old (removed) from nwp500.api_client import APIError from nwp500.auth import AuthenticationError # New (import from exceptions or package root) from nwp500.exceptions import APIError, AuthenticationError # OR from nwp500 import APIError, AuthenticationError Updated all examples to import exceptions from correct module. Rationale: Library is young with no external clients. Removing backward compatibility prevents accumulation of legacy patterns and maintains clean architecture. --- CHANGELOG.rst | 31 +++++++++++++++++++++--- examples/device_feature_callback.py | 3 ++- examples/device_status_callback.py | 3 ++- examples/device_status_callback_debug.py | 3 ++- examples/test_api_client.py | 5 ++-- src/nwp500/api_client.py | 3 --- src/nwp500/auth.py | 6 ----- 7 files changed, 36 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5a4692f..39fe27a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,7 +5,7 @@ Changelog Version 6.0.0 (2025-11-02) ========================== -**BREAKING CHANGES**: Removed constructor callbacks in favor of event emitter pattern +**BREAKING CHANGES**: Removed constructor callbacks and backward compatibility re-exports Removed ------- @@ -26,6 +26,26 @@ Removed mqtt_client.on("connection_interrupted", on_interrupted) mqtt_client.on("connection_resumed", on_resumed) +- **Backward Compatibility Re-exports**: Removed exception re-exports from ``api_client`` and ``auth`` modules + + .. code-block:: python + + # OLD (removed in v6.0.0) + from nwp500.api_client import APIError + from nwp500.auth import AuthenticationError, TokenRefreshError + + # NEW (import from exceptions module) + from nwp500.exceptions import APIError, AuthenticationError, TokenRefreshError + + # OR (import from package root - recommended) + from nwp500 import APIError, AuthenticationError, TokenRefreshError + +- **Rationale**: Library is young with no external clients. Removing backward compatibility + allows for cleaner architecture and prevents accumulation of legacy patterns. + +Changed +------- + - **Migration Benefits**: - Multiple listeners per event (not just one callback) @@ -33,12 +53,15 @@ Removed - Dynamic listener management (add/remove listeners at runtime) - Async handler support - Priority-based execution - -Changed -------- + - Cleaner imports (exceptions from one module) - Updated ``examples/command_queue_demo.py`` to use event emitter pattern - Updated ``examples/reconnection_demo.py`` to use event emitter pattern +- Updated ``examples/device_status_callback.py`` to import exceptions from correct module +- Updated ``examples/device_status_callback_debug.py`` to import exceptions from correct module +- Updated ``examples/device_feature_callback.py`` to import exceptions from correct module +- Updated ``examples/test_api_client.py`` to import exceptions from correct module +- Removed misleading "legacy state" comments from connection tracking code Version 5.0.2 (2025-10-31) ========================== diff --git a/examples/device_feature_callback.py b/examples/device_feature_callback.py index 1e35508..afc7123 100644 --- a/examples/device_feature_callback.py +++ b/examples/device_feature_callback.py @@ -31,7 +31,8 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "src")) from nwp500.api_client import NavienAPIClient -from nwp500.auth import AuthenticationError, NavienAuthClient +from nwp500.auth import NavienAuthClient +from nwp500.exceptions import AuthenticationError from nwp500.models import DeviceFeature from nwp500.mqtt_client import NavienMqttClient diff --git a/examples/device_status_callback.py b/examples/device_status_callback.py index 3d46804..393d109 100755 --- a/examples/device_status_callback.py +++ b/examples/device_status_callback.py @@ -34,7 +34,8 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "src")) from nwp500.api_client import NavienAPIClient -from nwp500.auth import AuthenticationError, NavienAuthClient +from nwp500.auth import NavienAuthClient +from nwp500.exceptions import AuthenticationError from nwp500.models import DeviceStatus from nwp500.mqtt_client import NavienMqttClient diff --git a/examples/device_status_callback_debug.py b/examples/device_status_callback_debug.py index 8699f35..b5af78a 100644 --- a/examples/device_status_callback_debug.py +++ b/examples/device_status_callback_debug.py @@ -23,7 +23,8 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "src")) from nwp500.api_client import NavienAPIClient -from nwp500.auth import AuthenticationError, NavienAuthClient +from nwp500.auth import NavienAuthClient +from nwp500.exceptions import AuthenticationError from nwp500.models import DeviceStatus from nwp500.mqtt_client import NavienMqttClient diff --git a/examples/test_api_client.py b/examples/test_api_client.py index 1d7497b..6804fba 100755 --- a/examples/test_api_client.py +++ b/examples/test_api_client.py @@ -13,8 +13,9 @@ # Add src to path for testing sys.path.insert(0, os.path.join(os.path.dirname(__file__), "src")) -from nwp500.api_client import APIError, NavienAPIClient -from nwp500.auth import AuthenticationError, NavienAuthClient +from nwp500.api_client import NavienAPIClient +from nwp500.auth import NavienAuthClient +from nwp500.exceptions import APIError, AuthenticationError # Setup logging logging.basicConfig( diff --git a/src/nwp500/api_client.py b/src/nwp500/api_client.py index 912a3dc..0106ba2 100644 --- a/src/nwp500/api_client.py +++ b/src/nwp500/api_client.py @@ -25,11 +25,8 @@ _logger = logging.getLogger(__name__) -# Exception class moved to exceptions.py module -# Import it here for backward compatibility __all__ = [ "NavienAPIClient", - "APIError", ] diff --git a/src/nwp500/auth.py b/src/nwp500/auth.py index f092fc9..2e08c54 100644 --- a/src/nwp500/auth.py +++ b/src/nwp500/auth.py @@ -261,16 +261,10 @@ def from_dict( ) -# Exception classes moved to exceptions.py module -# Import them here for backward compatibility __all__ = [ "UserInfo", "AuthTokens", "AuthenticationResponse", - "AuthenticationError", - "InvalidCredentialsError", - "TokenExpiredError", - "TokenRefreshError", "NavienAuthClient", "authenticate", "refresh_access_token", From 644840ec25ec3e67d3bc53f7b5b416c282128c93 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Sun, 2 Nov 2025 11:17:49 -0800 Subject: [PATCH 5/7] Fix linting and type checking issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix line length in connection state comment (80 char limit) - Import exceptions from exceptions module (not auth/api_client) - Remove unused type: ignore comment All CI checks now pass: āœ… Ruff linting āœ… Ruff formatting āœ… Mypy type checking --- src/nwp500/api_client.py | 10 +++------- src/nwp500/auth.py | 1 - src/nwp500/mqtt_client.py | 12 +++++------- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/nwp500/api_client.py b/src/nwp500/api_client.py index 0106ba2..c917bd0 100644 --- a/src/nwp500/api_client.py +++ b/src/nwp500/api_client.py @@ -9,13 +9,9 @@ import aiohttp -from .auth import ( - AuthenticationError, - NavienAuthClient, - TokenRefreshError, -) +from .auth import NavienAuthClient from .config import API_BASE_URL -from .exceptions import APIError +from .exceptions import APIError, AuthenticationError, TokenRefreshError from .models import Device, FirmwareInfo, TOUInfo __author__ = "Emmanuel Levijarvi" @@ -74,7 +70,7 @@ def __init__( self.base_url = base_url.rstrip("/") self._auth_client = auth_client - self._session: aiohttp.ClientSession = session or auth_client._session # type: ignore[assignment] + self._session: aiohttp.ClientSession = session or auth_client._session if self._session is None: raise ValueError("auth_client must have an active session") self._owned_session = ( diff --git a/src/nwp500/auth.py b/src/nwp500/auth.py index 2e08c54..f654f5d 100644 --- a/src/nwp500/auth.py +++ b/src/nwp500/auth.py @@ -23,7 +23,6 @@ from .exceptions import ( AuthenticationError, InvalidCredentialsError, - TokenExpiredError, TokenRefreshError, ) diff --git a/src/nwp500/mqtt_client.py b/src/nwp500/mqtt_client.py index 3bc78f6..b799103 100644 --- a/src/nwp500/mqtt_client.py +++ b/src/nwp500/mqtt_client.py @@ -19,17 +19,15 @@ from awscrt import mqtt from awscrt.exceptions import AwsCrtError -from .auth import ( - AuthenticationError, - NavienAuthClient, - TokenRefreshError, -) +from .auth import NavienAuthClient from .events import EventEmitter from .exceptions import ( + AuthenticationError, MqttConnectionError, MqttCredentialsError, MqttNotConnectedError, MqttPublishError, + TokenRefreshError, ) from .models import ( Device, @@ -179,8 +177,8 @@ def __init__( self._periodic_manager: Optional[MqttPeriodicRequestManager] = None # Connection state tracking - # Note: These track state at the client level and are simpler to check than - # self._connection_manager.is_connected (which can be None before connect()) + # Note: Simpler to check than _connection_manager.is_connected + # (which can be None before connect()) self._connection: Optional[mqtt.Connection] = None self._connected = False From 42c3bcadadd53f07781c2b5fc0bc47041e32f9a8 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Sun, 2 Nov 2025 11:41:28 -0800 Subject: [PATCH 6/7] Fix test imports and update copilot validation requirements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix test_auth.py to import exceptions from exceptions module - Update copilot instructions to make validation checks REQUIRED before completing any task - All checks now passing: āœ… Tests: 123/123 passed āœ… Linting: All checks passed āœ… Type checking: No errors --- .github/copilot-instructions.md | 27 +++++++++++++++++++++------ tests/test_auth.py | 8 +++++--- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 72e804c..9beeeb2 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -55,13 +55,28 @@ This prevents "passes locally but fails in CI" issues. **Important**: When updating CHANGELOG.rst or any file with dates, always use `date +"%Y-%m-%d"` to get the correct current date. Never hardcode or guess dates. -### After Completing a Task -Always run these checks after completing a task to validate your changes: -1. **Type checking**: `python3 -m mypy src/nwp500 --config-file pyproject.toml` - Verify no type errors were introduced -2. **Linting**: `make ci-lint` - Verify code style compliance -3. **Tests** (if applicable): `pytest` - Verify functionality works as expected +### Before Completing a Task - REQUIRED VALIDATION + +**ALWAYS run these checks before considering a task complete:** + +1. **Linting**: `make ci-lint` - MUST pass before completion +2. **Type checking**: `python3 -m mypy src/nwp500 --config-file pyproject.toml` - MUST pass before completion +3. **Unit tests**: `pytest` - MUST pass before completion (unless tests don't exist for the feature) + +**Do not mark a task as complete or create a PR without running all three checks.** -Report the results of these checks in your final summary. +These checks prevent "works locally but fails in CI" issues and catch integration problems early. + +Report the results of these checks in your final summary, including: +- Number of tests passed/failed +- Any linting errors fixed +- Any type errors resolved + +### After Completing a Task +Document validation results: +- āœ… **Linting**: All checks passed +- āœ… **Type checking**: No errors found +- āœ… **Tests**: X/X passed (or "N/A - no existing tests for this feature") ## Patterns & Conventions - **Async context managers** for authentication: `async with NavienAuthClient(email, password) as auth_client:` diff --git a/tests/test_auth.py b/tests/test_auth.py index b32ef1e..c2a4daf 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -7,14 +7,16 @@ import pytest from nwp500.auth import ( - AuthenticationError, AuthenticationResponse, AuthTokens, - InvalidCredentialsError, NavienAuthClient, + UserInfo, +) +from nwp500.exceptions import ( + AuthenticationError, + InvalidCredentialsError, TokenExpiredError, TokenRefreshError, - UserInfo, ) From 7800cd3b08012afa88ecdae11ec5c6d28b5caa3a Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Sun, 2 Nov 2025 11:49:39 -0800 Subject: [PATCH 7/7] Condense connection state comment to single line Address PR feedback: comment was unnecessarily verbose for simple state tracking variables. Reduced from 3 lines to 1 concise line. --- src/nwp500/mqtt_client.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/nwp500/mqtt_client.py b/src/nwp500/mqtt_client.py index b799103..b539a37 100644 --- a/src/nwp500/mqtt_client.py +++ b/src/nwp500/mqtt_client.py @@ -176,9 +176,7 @@ def __init__( self._reconnect_task: Optional[asyncio.Task[None]] = None self._periodic_manager: Optional[MqttPeriodicRequestManager] = None - # Connection state tracking - # Note: Simpler to check than _connection_manager.is_connected - # (which can be None before connect()) + # Connection state (simpler than checking _connection_manager) self._connection: Optional[mqtt.Connection] = None self._connected = False