-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add AiPort support #330
Conversation
WalkthroughThis pull request introduces support for a new device type called Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/uiprotect/api.py (1)
1397-1404
: Fix typo in docstringThere's a typo in the docstring:
self.bootstrap.aiport[device_id]
should beself.bootstrap.aiports[device_id]
.- The websocket is connected and running, you likely just want to use `self.bootstrap.aiport[device_id]` + The websocket is connected and running, you likely just want to use `self.bootstrap.aiports[device_id]`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/uiprotect/api.py
(3 hunks)src/uiprotect/data/__init__.py
(2 hunks)src/uiprotect/data/bootstrap.py
(2 hunks)src/uiprotect/data/convert.py
(2 hunks)src/uiprotect/data/devices.py
(2 hunks)src/uiprotect/data/types.py
(2 hunks)src/uiprotect/test_util/__init__.py
(3 hunks)tests/conftest.py
(1 hunks)tests/sample_data/sample_bootstrap.json
(12 hunks)tests/sample_data/sample_camera.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/sample_data/sample_camera.json
👮 Files not reviewed due to content moderation or server errors (2)
- src/uiprotect/data/devices.py
- tests/sample_data/sample_bootstrap.json
🔇 Additional comments (14)
tests/conftest.py (1)
784-784
: Adding AiPort to object comparison in tests
Including ModelType.AIPORT.value
in the condition ensures that AiPort
devices are correctly handled in the compare_objs
function alongside cameras.
src/uiprotect/data/convert.py (2)
11-11
: Importing AiPort for model conversion
Adding AiPort
to the import statement allows it to be used in the model conversion mappings.
44-44
: Mapping ModelType.AIPORT to AiPort class
Including ModelType.AIPORT: AiPort
in the MODEL_TO_CLASS
mapping integrates the AiPort
model into the conversion system, enabling proper instantiation from data dictionaries.
src/uiprotect/data/__init__.py (2)
13-13
: Importing AiPort into the data module
Adding AiPort
to the list of imports ensures it is accessible within the data package.
89-89
: Exposing AiPort in the public API
Including AiPort
in the __all__
list makes it part of the module's public interface, allowing users to import it directly.
src/uiprotect/data/types.py (2)
127-127
: Adding AiPort to ModelType enum
Defining AIPORT = "aiport"
in the ModelType
enum ensures the new device type is recognized throughout the codebase.
177-177
: Including AiPort in bootstrap model types
Adding ModelType.AIPORT
to the _bootstrap_model_types
method includes AiPort
devices in the bootstrap process, ensuring they are properly initialized during startup.
src/uiprotect/test_util/__init__.py (3)
134-134
: LGTM: Device count tracking is consistent
The addition of "aiport" count follows the established pattern for tracking device counts in the bootstrap data.
287-287
: LGTM: Device data generation is properly integrated
The addition of aiport data generation maintains consistency with other device types and preserves the alphabetical ordering.
474-488
: LGTM: AiPort data generation implementation is well-structured
The implementation follows the established pattern for device data generation, including:
- Proper error handling for when no aiport is found
- Appropriate logging
- Consistent API endpoint usage
src/uiprotect/data/bootstrap.py (2)
26-26
: LGTM: Import statement follows conventions
The addition of AiPort to the imports maintains consistency with the existing import style and alphabetical ordering.
185-185
: LGTM: Bootstrap class attribute is properly defined
The aiports attribute is correctly typed and follows the established pattern for device storage in the Bootstrap class.
src/uiprotect/api.py (2)
60-60
: LGTM: Import statement is consistent
The addition of AiPort to the imports follows the established pattern.
1271-1278
: LGTM: get_aiports method is well-implemented
The implementation follows the established pattern for device retrieval methods and includes appropriate type safety through casting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/uiprotect/cli/aiports.py (1)
44-44
: Enhance error message for invalid device IDThe error message could be more descriptive to help users understand the issue.
- typer.secho("Invalid aiport ID", fg="red") + typer.secho(f"Invalid AiPort ID: {device_id}", fg="red")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/uiprotect/cli/__init__.py
(2 hunks)src/uiprotect/cli/aiports.py
(1 hunks)src/uiprotect/data/devices.py
(2 hunks)tests/conftest.py
(7 hunks)tests/sample_data/sample_aiport.json
(1 hunks)tests/test_api.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/conftest.py
🔇 Additional comments (7)
src/uiprotect/cli/aiports.py (2)
17-21
: LGTM: AiPortContext class implementation
The class correctly extends base.CliContext and provides appropriate type hints for devices dictionary and device instance.
26-57
: LGTM: Main callback implementation
The implementation follows the established pattern for CLI modules:
- Proper handling of device selection
- Consistent use of base printing utilities
- Appropriate error handling and exit codes
src/uiprotect/cli/__init__.py (1)
20-20
: LGTM: AiPort CLI integration
The integration of the AiPort CLI module follows the established pattern for sub-application registration.
tests/sample_data/sample_aiport.json (1)
1-757
: LGTM: Overall configuration structure
The configuration provides a comprehensive representation of an AiPort device with appropriate settings for:
- Device capabilities and feature flags
- Video and audio configuration
- Smart detection settings
- Network and connection state
- Security and privacy settings
tests/test_api.py (3)
908-913
: LGTM! Test follows established patterns.
The test case follows the same pattern as other device tests, properly validating the object creation and API response matching.
916-924
: LGTM! Error handling test is well implemented.
The test properly validates the error handling for non-adopted devices, following the established pattern.
926-934
: LGTM! Test validates ignore_unadopted flag behavior.
The test properly validates the retrieval of non-adopted devices when the ignore flag is disabled, following the established pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/uiprotect/cli/aiports.py
(1 hunks)tests/conftest.py
(8 hunks)tests/test_api.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/uiprotect/cli/aiports.py
🔇 Additional comments (10)
tests/conftest.py (6)
56-56
: LGTM: Constant declaration follows existing pattern
The constant TEST_AIPORT_EXISTS
is correctly defined following the same pattern as other device existence checks.
93-100
: LGTM: Function implementation matches existing patterns
The read_aiport_json_file()
function follows the same pattern as other device-specific JSON file readers, including the handling of global recording settings.
162-163
: LGTM: Mock API request handler updated correctly
The mock_api_request()
function is properly updated to handle AiPort-related requests, maintaining consistency with other device types.
Also applies to: 202-203
474-480
: LGTM: Fixture follows established patterns
The aiport_obj
fixture follows the same pattern as other device object fixtures, correctly handling the case when test data doesn't exist.
583-589
: LGTM: List fixture follows established patterns
The aiports
fixture follows the same pattern as other device list fixtures, correctly handling empty lists when test data doesn't exist.
822-822
: LGTM: Object comparison updated for new device type
The compare_objs()
function is correctly updated to handle AiPort objects with the same field exclusions as cameras.
tests/test_api.py (4)
908-914
: LGTM: Basic device retrieval test
The test follows the established pattern for device retrieval tests, correctly using the fixture and comparing the results.
916-924
: LGTM: Not adopted error test
The test correctly verifies that attempting to retrieve a non-adopted device raises an NvrError when ignore_unadopted is true.
926-935
: LGTM: Not adopted allowed test
The test correctly verifies that non-adopted devices can be retrieved when ignore_unadopted is false.
966-971
: LGTM: Device list retrieval test
The test correctly verifies the retrieval of multiple AiPort devices, following the established pattern for device list tests.
Description of change
Pull-Request Checklist
main
branchFixes #0000
Summary by CodeRabbit
New Features
AiPort
, enhancing device management capabilities.AiPort
devices and their details.AiPort
data.AiPort
properties and relationships with cameras.AiPort
management into the command-line interface.AiPort
functionality within the API client.Bug Fixes
AiPort
type.Documentation
AiPort
.