feat: add ConnectionStatus enum for device connection state#54
Merged
Conversation
- Create ConnectionStatus IntEnum with DISCONNECTED (1) and CONNECTED (2) values - Update DeviceInfo.connected field to use ConnectionStatusField with proper type validation - Add enum_validator converter for automatic integer-to-enum conversion - Set default value to ConnectionStatus.DISCONNECTED This provides type-safe handling of device connection status based on reverse-engineered Navien protocol documentation (REVERSE_ENGINEERED_FIELDS.md). BREAKING CHANGE: DeviceInfo.connected is now an enum instead of int. Use .value to get the integer representation or compare directly with ConnectionStatus values. Related to: #95
- Add public auth_response property to NavienAuthClient to avoid accessing private _auth_response attribute - Fix type annotation in OutputFormatter.__init__ to prevent no-redef mypy error - Keep necessary mypy overrides for click and cli modules due to untyped decorators This resolves all CI failures while maintaining the ConnectionStatus enum feature.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces type-safe connection status handling by replacing the integer-based connected field with an IntEnum type. The change provides explicit semantic meaning to connection states (DISCONNECTED=1, CONNECTED=2) based on reverse-engineered protocol documentation. Additionally, the PR includes several type checking improvements: exposing the auth response property for cleaner access patterns, fixing type annotations in the CLI output formatter, and adding mypy configuration to ignore CLI-related type errors.
Key Changes:
- Adds
ConnectionStatusenum with DISCONNECTED (1) and CONNECTED (2) values - Updates
DeviceInfo.connectedto use type-validated enum field with proper default - Exposes
auth_responseproperty onNavienAuthClientto eliminate unsafe casting
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/nwp500/enums.py | Introduces ConnectionStatus IntEnum for device connection state representation |
| src/nwp500/models.py | Updates DeviceInfo to use ConnectionStatusField with enum validation and proper default |
| src/nwp500/auth.py | Exposes auth_response property and refactors authenticate() to use it instead of cast(Any) |
| src/nwp500/cli/rich_output.py | Fixes type annotation style in OutputFormatter constructor |
| pyproject.toml | Adds mypy overrides to ignore CLI module type errors |
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 provides type-safe handling of device connection status based on reverse-engineered Navien protocol documentation (REVERSE_ENGINEERED_FIELDS.md).
BREAKING CHANGE: DeviceInfo.connected is now an enum instead of int. Use .value to get the integer representation or compare directly with ConnectionStatus values.
Related to: #95