diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c226cd8..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:` @@ -79,6 +94,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..39fe27a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,67 @@ Changelog ========= +Version 6.0.0 (2025-11-02) +========================== + +**BREAKING CHANGES**: Removed constructor callbacks and backward compatibility re-exports + +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) + +- **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) + - Consistent API with other events (temperature_changed, mode_changed, etc.) + - Dynamic listener management (add/remove listeners at runtime) + - Async handler support + - Priority-based execution + - 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/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/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/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/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..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" @@ -25,11 +21,8 @@ _logger = logging.getLogger(__name__) -# Exception class moved to exceptions.py module -# Import it here for backward compatibility __all__ = [ "NavienAPIClient", - "APIError", ] @@ -77,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 f092fc9..f654f5d 100644 --- a/src/nwp500/auth.py +++ b/src/nwp500/auth.py @@ -23,7 +23,6 @@ from .exceptions import ( AuthenticationError, InvalidCredentialsError, - TokenExpiredError, TokenRefreshError, ) @@ -261,16 +260,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", diff --git a/src/nwp500/mqtt_client.py b/src/nwp500/mqtt_client.py index 2ee279d..b539a37 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, @@ -126,8 +124,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 +131,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 @@ -182,14 +176,10 @@ 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 (simpler than checking _connection_manager) 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 +224,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 +243,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( @@ -497,7 +470,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 @@ -587,7 +560,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 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, )