Improvements #37
Improvements #37
Conversation
WalkthroughThis update standardizes exception and class names across the codebase to use the "AirOS" prefix, introduces a new provisioning mode API and asynchronous device discovery function, updates enum member names for clarity, adds new tests and fixtures for discovery, and slightly lowers the test coverage threshold. The project version is incremented. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AirOS
participant API_Server
User->>AirOS: provmode(active: bool)
AirOS->>API_Server: POST /api/provmode {"action": "start"/"stop"}
API_Server-->>AirOS: HTTP 200/Failure
AirOS-->>User: True/False or raise AirOSDeviceConnectionError
sequenceDiagram
participant User
participant async_discover_devices
participant EventLoop
participant AirOSDiscoveryProtocol
User->>async_discover_devices: call with timeout
async_discover_devices->>EventLoop: create_datagram_endpoint(AirOSDiscoveryProtocol)
EventLoop-->>async_discover_devices: (transport, protocol)
AirOSDiscoveryProtocol-->>async_discover_devices: discovered device(s)
async_discover_devices-->>User: {mac: device_info, ...}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/verify.yml (1)
154-154: Ensure the coverage threshold reduction is tracked and reverted.The coverage threshold was lowered from 85% to 80%, which aligns with the PR objectives mentioning this as a temporary adjustment. Consider adding a TODO comment in the workflow file itself to ensure this reduction is reverted once the new functionality is fully tested.
coverage combine coverage*/.coverage* + # TODO: Revert coverage threshold back to 85% once new discovery functionality is fully tested coverage report --fail-under=80airos/discovery.py (1)
85-86: Consider adding context to the raised exception.When re-raising
AirOSDiscoveryErrorinconnection_lost, consider including the original exception or a descriptive message for better debugging.- raise AirOSDiscoveryError from None + raise AirOSDiscoveryError("Connection lost due to error") from exc
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/verify.yml(1 hunks)airos/airos8.py(10 hunks)airos/data.py(1 hunks)airos/discovery.py(9 hunks)airos/exceptions.py(1 hunks)pyproject.toml(1 hunks)tests/conftest.py(2 hunks)tests/test_discovery.py(9 hunks)tests/test_stations.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
airos/data.py (1)
airos/airos8data.py (5)
WirelessMode(18-23)Wireless(265-301)check_for_warnings(394-425)Station(227-261)AirOSData(367-431)
tests/conftest.py (2)
airos/airos8.py (1)
AirOS(25-345)airos/discovery.py (1)
AirOSDiscoveryProtocol(18-275)
tests/test_stations.py (3)
airos/exceptions.py (4)
AirOSConnectionSetupError(8-9)AirOSDataMissingError(16-17)AirOSConnectionAuthenticationError(12-13)AirOSDeviceConnectionError(24-25)airos/airos8.py (2)
status(227-268)login(76-193)tests/conftest.py (1)
airos_device(20-25)
airos/airos8.py (3)
airos/exceptions.py (5)
AirOSConnectionAuthenticationError(12-13)AirOSConnectionSetupError(8-9)AirOSDataMissingError(16-17)AirOSDeviceConnectionError(24-25)AirOSKeyDataMissingError(20-21)tests/conftest.py (1)
base_url(14-16)airos/airos8data.py (3)
AirOSData(367-431)check_for_warnings(394-425)ProvisioningMode(337-340)
airos/discovery.py (2)
airos/exceptions.py (3)
AirOSDiscoveryError(28-29)AirOSEndpointError(36-37)AirOSListenerError(32-33)airos/airos8data.py (2)
AirOSData(367-431)check_for_warnings(394-425)
🔇 Additional comments (11)
airos/data.py (1)
45-46: PTP enum renaming is consistent and complete– Searched for
AccessPoint_PointToPointandStation_PointToPoint: no occurrences found
– VerifiedPTP_ACCESSPOINTandPTP_STATIONonly exist inairos/data.pyAll references have been updated; this change is ready to merge.
pyproject.toml (1)
7-7: LGTM! Appropriate version increment.The patch version bump from 0.2.1 to 0.2.2 is appropriate for the improvements, refactoring, and new discovery functionality introduced in this PR.
tests/test_stations.py (1)
108-108: LGTM! Exception names updated consistently.The exception names have been updated to use the new "AirOS" prefixed naming convention, which aligns with the standardization effort across the codebase. The test logic remains intact while properly catching the renamed exception types.
Also applies to: 127-127, 138-138, 146-146
tests/conftest.py (2)
3-4: LGTM! Appropriate imports for new discovery functionality.The new imports support the discovery testing functionality with proper asyncio and mocking capabilities.
Also applies to: 7-7
28-50: LGTM! Well-designed test fixture for discovery functionality.The
mock_datagram_endpointfixture is well-implemented for testing the new UDP discovery functionality:
- Properly mocks
asyncio.DatagramTransportandAirOSDiscoveryProtocolwith appropriate specs- Uses context managers for clean patch management
- Pre-defines mock objects to avoid scope issues
- Returns both transport and protocol instances for comprehensive testing
The fixture follows testing best practices and will enable reliable testing of the discovery protocol.
airos/exceptions.py (1)
8-29: Consistent naming convention applied successfully.The renaming of all exception classes to include the "AirOS" prefix improves consistency across the codebase. The inheritance hierarchy is properly maintained with all exceptions deriving from the base
AirOSExceptionclass.airos/discovery.py (1)
278-325: Well-implemented asynchronous discovery function.The new
async_discover_devicesfunction provides a clean, high-level interface for device discovery with proper error handling and resource cleanup. The use of a closure to capture discovered devices is appropriate, and the transport is correctly closed in the finally block.airos/airos8.py (2)
15-19: Exception renaming applied consistently throughout the module.All exception references have been properly updated to use the "AirOS" prefix, maintaining the same error handling behavior while improving naming consistency across the codebase.
Also applies to: 107-345
308-345: New provisioning mode method follows established patterns.The
provmodemethod is well-implemented, following the same structure and error handling patterns as other methods in the class. The boolean parameter provides a clear interface for enabling/disabling provisioning mode.tests/test_discovery.py (2)
8-211: Test updates maintain coverage while reflecting naming changes.All existing tests have been properly updated to use the new "AirOS" prefixed class and exception names. The test logic and coverage remain unchanged, ensuring backward compatibility of the test suite.
217-306: Comprehensive test coverage for the new async discovery function.The new tests thoroughly cover all aspects of
async_discover_devices:
- Success scenarios with device discovery
- Empty results when no devices are found
- Error handling for network issues (OSError)
- Proper cleanup during cancellation
The tests follow async best practices and ensure proper resource cleanup in all scenarios.
1 participant
Note: temporarily adjust minimal coverage (TODO)
Improvements:
Airosoccasions to AirOSSummary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores