Introduce basic v6 support #83
Conversation
WalkthroughIntroduces a shared AirOS base class and per‑firmware subclasses (AirOS6/AirOS8), adds AirOS6 data models and helpers, migrates scripts/tests/fixtures for multi‑version support, adds AirOSNotSupportedError, and bumps project version to 0.5.1. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Base as AirOS (base)
participant Device as Device
User->>Base: login(host, creds)
Base->>Device: POST /login (cookies + CSRF)
Device-->>Base: 200 OK (Set-Cookie, X-CSRF-ID)
User->>Base: status()
Base->>Device: GET /status (auth headers)
Device-->>Base: 200 OK (JSON)
Base->>Base: _derived_data_helper(response, derived_wireless_data)
Base-->>User: DataModel.from_dict(derived) -> AirOS6Data | AirOS8Data
sequenceDiagram
autonumber
actor Script
participant Gen as generate_ha_fixture.py
participant Source as Source JSON
Script->>Gen: run(source)
Gen->>Gen: parse host.fwversion -> fw_major
alt fw_major == 6
Gen->>Gen: AirOS6._derived_data_helper(source, AirOS6.derived_wireless_data)
Gen->>Gen: AirOS6Data.from_dict(...)
else
Gen->>Gen: AirOS8._derived_data_helper(source, AirOS8.derived_wireless_data)
Gen->>Gen: AirOS8Data.from_dict(...)
end
Gen-->>Script: write output fixture
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
==========================================
+ Coverage 91.36% 91.85% +0.48%
==========================================
Files 4 7 +3
Lines 730 909 +179
==========================================
+ Hits 667 835 +168
- Misses 63 74 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (32)
pyproject.toml (1)
7-7: Align versioning across repo (alpha vs stable).Project version is 0.5.0a0, but README/CHANGELOG use 0.5.0 and classifier says “Production/Stable.” Pick one and align all three.
Apply one of:
- version = "0.5.0a0" + version = "0.5.0"or:
- "Development Status :: 5 - Production/Stable", + "Development Status :: 3 - Alpha",README.md (2)
28-28: Fix PyPI link path (404).“projects” → “project”.
-`python-airos` or [`airos`](https://pypi.org/projects/airos) from pypi +`python-airos` or [`airos`](https://pypi.org/project/airos) from PyPI
69-69: Update deprecated aiohttp argument.verify_ssl is deprecated; prefer ssl=False for the connector example.
- session = aiohttp.ClientSession(connector=aiohttp.TCPConnector(verify_ssl=False)) + session = aiohttp.ClientSession(connector=aiohttp.TCPConnector(ssl=False))CHANGELOG.md (1)
5-14: Synchronize version label and tighten wording.
- Header shows 0.5.0 while pyproject uses 0.5.0a0.
- Minor grammar in first bullet.
-## [0.5.0] - 2025-08-30 +## [0.5.0a0] - 2025-08-30 @@ -- Add logging redacted data on interface [issue](https://github.com/home-assistant/core/issues/151348) +- Log redacted interface data [issue](https://github.com/home-assistant/core/issues/151348)fixtures/userdata/NanoStation_M5_sta_v6.3.16.json (1)
1-113: Optional: pretty-print for reviewability.Minified blocks are harder to diff/maintain; consider storing fixtures pretty-printed.
fixtures/userdata/NanoBeam_5AC_ap-ptmp_v8.7.18.json (1)
1-1: Looks good; mirrors v8 fields well.Consider pretty-printing to aid diffs, but not required.
fixtures/airos_NanoBeam_5AC_ap-ptmp_v8.7.18.json (2)
14-21: Normalize MAC placeholders to valid colon-separated format.Some hwaddr/mac fields use varying “x”-lengths. Prefer a consistent, valid-looking placeholder (e.g., AA:BB:CC:DD:EE:FF) across all interfaces and derived.mac to avoid future parser/validator surprises.
Also applies to: 49-75, 76-103, 104-126, 127-154
224-359: Consider truncating or compressing very long EVM arrays in fixtures.These repeated constant arrays bloat the repo and slow diffs without improving test fidelity. Either trim to a short representative sample or replace with smaller numbers while preserving structure.
Also applies to: 362-497
airos/airos8.py (2)
123-126: Typo in comment (“fist” → “first”).Minor readability nit.
- # Fallback take fist alternate interface found + # Fallback: take first alternate interface found
264-281: Content-Type flag mismatches payload in update_check.When force=True you set ct_form=True but pass json_data, which advertises x-www-form-urlencoded while sending JSON. Align header and payload to avoid server-side parsing issues.
If JSON is correct for both paths, apply:
- if force: - return await self._request_json( - "POST", - self._update_check_url, - json_data={"force": True}, - authenticated=True, - ct_form=True, - ) + if force: + return await self._request_json( + "POST", + self._update_check_url, + json_data={"force": True}, + authenticated=True, + ct_json=True, + )If the endpoint truly expects form data for force, instead switch to form_data:
- json_data={"force": True}, - authenticated=True, - ct_form=True, + form_data={"force": "1"}, + authenticated=True, + ct_form=True,fixtures/airos_NanoStation_M5_sta_v6.3.16.json (2)
23-25: Remove trailing space in devmodel.Trailing whitespace can cause brittle assertions.
- "devmodel": "NanoStation M5 ", + "devmodel": "NanoStation M5",
125-127: Prefer null over empty string for timestamp if “no value”.Keeps types consistent with other nullable fields.
- "timestamp": "" + "timestamp": nullscript/generate_ha_fixture.py (1)
70-75: Don’t abort entire run on one bad file; continue to next.Improves batch processing resilience.
- except json.JSONDecodeError: - _LOGGER.error("Skipping '%s': Not a valid JSON file.", filename) - raise - except Exception as e: - _LOGGER.error("Error processing '%s': %s", filename, e) - raise + except json.JSONDecodeError: + _LOGGER.error("Skipping '%s': Not a valid JSON file.", filename) + continue + except Exception as e: + _LOGGER.error("Error processing '%s': %s", filename, e) + continuetests/conftest.py (1)
38-38: Add a backward-compat fixture alias (optional).Keeps existing tests stable while you migrate. Alias airos_device to airos8_device by default.
@@ await session.close() @pytest.fixture async def airos8_device(base_url: str) -> AsyncGenerator[AirOS8, None]: """AirOS8 device fixture.""" session = aiohttp.ClientSession(cookie_jar=aiohttp.CookieJar()) instance = AirOS8(base_url, "username", "password", session, use_ssl=False) yield instance await session.close() + +# Back-compat: default to v8 unless a test explicitly requests v6 +@pytest.fixture(name="airos_device") +async def airos_device_fixture(airos8_device: AirOS8) -> AsyncGenerator[AirOS8, None]: + yield airos8_devicetests/test_stations6.py (3)
37-37: Docstring mismatch: you force MissingField, not InvalidFieldValue.Update the message to reflect MissingField, or raise InvalidFieldValue to match.
116-121: Non-200 path won’t trigger without raising ClientResponseError.Set raise_for_status.side_effect to aiohttp.ClientResponseError to hit the error branch.
mock_status_response = MagicMock() mock_status_response.__aenter__.return_value = mock_status_response - mock_status_response.status = 500 # Non-200 status + mock_status_response.status = 500 # Non-200 status + mock_status_response.raise_for_status.side_effect = aiohttp.ClientResponseError( + request_info=MagicMock(), history=(), status=500, message="Server error" + )
179-181: Remove unused cookie code.Dead code; safe to drop.
- cookie = SimpleCookie() - cookie["session_id"] = "test-cookie" - cookie["AIROS_TOKEN"] = "abc123"tests/test_stations.py (1)
185-188: Remove unused cookie code.Leftover from earlier approach; not used.
- cookie = SimpleCookie() - cookie["session_id"] = "test-cookie" - cookie["AIROS_TOKEN"] = "abc123"tests/test_airos6.py (2)
29-33: Headers object must support getall('Set-Cookie').Login path calls response.headers.getall; a plain dict breaks this. Mock a headers object with getall instead.
- mock_login_response.headers = {} # Simulate missing X-CSRF-ID + mock_login_response.headers = MagicMock() + mock_login_response.headers.get.return_value = None # No X-CSRF-ID + mock_login_response.headers.getall.return_value = [] # No Set-Cookie
55-71: Enable and fix the non-200 status test.Uncomment and raise ClientResponseError to assert the error mapping.
-''' @pytest.mark.asyncio async def test_status_non_200_response(airos6_device: AirOS) -> None: """Test status() with a non-successful HTTP response.""" airos6_device.connected = True mock_status_response = MagicMock() mock_status_response.__aenter__.return_value = mock_status_response mock_status_response.text = AsyncMock(return_value="Error") mock_status_response.status = 500 # Simulate server error + mock_status_response.raise_for_status.side_effect = aiohttp.ClientResponseError( + request_info=MagicMock(), history=(), status=500, message="Server error" + ) with ( patch.object(airos6_device.session, "request", return_value=mock_status_response), pytest.raises(airos.exceptions.AirOSDeviceConnectionError), ): await airos6_device.status() -'''airos/airos6.py (3)
145-151: Be resilient to headers objects without getall (tests/mocks).Support both CIMultiDictProxy and plain dict-like headers.
- for set_cookie in response.headers.getall("Set-Cookie", []): - cookie.load(set_cookie) + set_cookie_values: list[str] = [] + if hasattr(response.headers, "getall"): + set_cookie_values = response.headers.getall("Set-Cookie", []) + else: + sc = response.headers.get("Set-Cookie") if response.headers else None + if sc: + set_cookie_values = [sc] + for set_cookie in set_cookie_values: + cookie.load(set_cookie)
106-109: Typo in comment.“fist” → “first”.
- # Fallback take fist alternate interface found + # Fallback: take first alternate interface found
61-67: Remove unused attributes (follow-up).current_csrf_token and _use_json_for_login_post are never read; drop to avoid confusion.
airos/data.py (4)
225-244: Fix dataclass name in Host6 pre-deserialize logThe log uses "Host" for Host6; use the correct class name for clearer diagnostics.
Apply:
- def __pre_deserialize__(cls, d: dict[str, Any]) -> dict[str, Any]: - """Pre-deserialize hook for Host.""" - _check_and_log_unknown_enum_value(d, "netrole", NetRole, "Host", "netrole") + def __pre_deserialize__(cls, d: dict[str, Any]) -> dict[str, Any]: + """Pre-deserialize hook for Host6.""" + _check_and_log_unknown_enum_value(d, "netrole", NetRole, "Host6", "netrole") return d
266-272: Consider bool for Airview6.enabledIf the v6 JSON is 0/1 for enabled, modeling as bool improves ergonomics. If integers vary beyond 0/1, keep int.
- enabled: int + enabled: bool
545-586: Wireles6 pre-deserialize: fix dataclass name; consider tolerant typing for frequency
- Logging passes "Wireless" for security; use "Wireless6".
- frequency is str; several dumps expose numeric channel center as int. Consider Union[int, str] for robustness.
class Wireless6(AirOSDataClass): @@ - frequency: str + frequency: int | str @@ def __pre_deserialize__(cls, d: dict[str, Any]) -> dict[str, Any]: """Pre-deserialize hook for Wireless6.""" _check_and_log_unknown_enum_value(d, "mode", Wireless6Mode, "Wireless6", "mode") _check_and_log_unknown_enum_value( - d, "security", Security, "Wireless", "security" + d, "security", Security, "Wireless6", "security" ) return d
18-20: MAC mask regex doesn’t accept hyphen-separated masksMAC_ADDRESS_MASK_REGEX enforces colons in the first four octets, so "00-00-00-00-89-AB" won’t match. Either broaden to support "-" or harmonize input before matching.
- MAC_ADDRESS_MASK_REGEX = re.compile(r"^(00:){4}[0-9a-fA-F]{2}[:-][0-9a-fA-F]{2}$") + MAC_ADDRESS_MASK_REGEX = re.compile( + r"^(?:00([:-])){4}[0-9a-fA-F]{2}\1[0-9a-fA-F]{2}$" + )tests/test_airos8.py (5)
1-1: Docstring nitConsider “AirOS8” capitalization for consistency with module/class names.
59-72: Replace commented-out test with skip markerRather than a triple-quoted block, keep the test live and explicitly skipped for visibility in reports.
-''' -@pytest.mark.asyncio -async def test_status_non_200_response(airos8_device: AirOS) -> None: +@pytest.mark.skip(reason="pending: non-200 status handling semantics") +@pytest.mark.asyncio +async def test_status_non_200_response(airos8_device: AirOS) -> None: @@ - with ( - patch.object(airos8_device.session, "request", return_value=mock_status_response), - pytest.raises(airos.exceptions.AirOSDeviceConnectionError), - ): - await airos8_device.status() -''' + with ( + patch.object(airos8_device.session, "request", return_value=mock_status_response), + pytest.raises(airos.exceptions.AirOSDeviceConnectionError), + ): + await airos8_device.status()
155-167: Skipped non-200 stakick testOnce behavior is decided, consider converting to xfail with a reason to keep it executing.
191-237: Skipped provmode happy/negative pathsMocks look correct; when unskipping, ensure response.text() isn’t awaited twice and headers/CSRF are exercised.
290-307: warnings(): local instance with mocked sessionWorks; alternatively, reuse airos8_device fixture for uniformity. Optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
CHANGELOG.md(1 hunks)README.md(1 hunks)airos/airos6.py(1 hunks)airos/airos8.py(1 hunks)airos/data.py(8 hunks)fixtures/airos_NanoBeam_5AC_ap-ptmp_v8.7.18.json(1 hunks)fixtures/airos_NanoStation_M5_sta_v6.3.16.json(1 hunks)fixtures/userdata/NanoBeam_5AC_ap-ptmp_v8.7.18.json(1 hunks)fixtures/userdata/NanoStation_M5_sta_v6.3.16.json(1 hunks)pyproject.toml(1 hunks)script/generate_ha_fixture.py(2 hunks)script/mashumaro-step-debug.py(2 hunks)tests/conftest.py(2 hunks)tests/test_airos6.py(1 hunks)tests/test_airos8.py(10 hunks)tests/test_stations.py(8 hunks)tests/test_stations6.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
airos/airos8.py (1)
airos/exceptions.py (1)
AirOSKeyDataMissingError(20-21)
tests/test_stations6.py (3)
airos/exceptions.py (2)
AirOSDeviceConnectionError(24-25)AirOSKeyDataMissingError(20-21)tests/test_stations.py (4)
_read_fixture(18-28)test_status_logs_redacted_data_on_invalid_value(34-94)test_status_logs_exception_on_missing_field(100-140)test_ap_object(156-187)tests/conftest.py (2)
airos6_device(24-29)base_url(18-20)
airos/airos6.py (1)
airos/exceptions.py (4)
AirOSConnectionSetupError(8-9)AirOSDeviceConnectionError(24-25)AirOSConnectionAuthenticationError(12-13)AirOSDataMissingError(16-17)
tests/test_airos6.py (4)
tests/test_airos8.py (8)
test_login_no_csrf_token(18-34)test_login_connection_error(38-44)test_status_when_not_connected(49-53)test_status_invalid_json_response(76-90)test_status_missing_interface_key_data(94-111)test_derived_data_no_interfaces_key(115-119)test_derived_data_no_br0_eth0_ath0(123-133)test_status_missing_required_key_in_json(251-282)tests/conftest.py (1)
airos6_device(24-29)airos/exceptions.py (3)
AirOSDeviceConnectionError(24-25)AirOSDataMissingError(16-17)AirOSKeyDataMissingError(20-21)tests/test_airos_request.py (1)
test_request_json_connection_error(65-78)
tests/conftest.py (1)
tests/test_airos_request.py (1)
mock_airos_device(26-33)
tests/test_stations.py (3)
tests/conftest.py (2)
airos8_device(33-38)base_url(18-20)airos/exceptions.py (2)
AirOSKeyDataMissingError(20-21)AirOSDeviceConnectionError(24-25)tests/test_stations6.py (1)
test_ap_object(150-181)
tests/test_airos8.py (3)
tests/test_airos6.py (8)
test_login_no_csrf_token(17-33)test_login_connection_error(37-43)test_status_when_not_connected(48-52)test_status_invalid_json_response(75-89)test_status_missing_interface_key_data(93-110)test_derived_data_no_interfaces_key(114-118)test_derived_data_no_br0_eth0_ath0(122-132)test_status_missing_required_key_in_json(137-168)tests/conftest.py (1)
airos8_device(33-38)airos/exceptions.py (3)
AirOSDeviceConnectionError(24-25)AirOSDataMissingError(16-17)AirOSKeyDataMissingError(20-21)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~11-~11: There might be a mistake here.
Context: ... ### Added - Add logging redacted data on interface [issue](https://github.com/ho...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run mypy
🔇 Additional comments (37)
pyproject.toml (1)
22-22: Confirm Python floor is intentional (>=3.13).Requiring 3.13 only is strict; if unintentional, consider lowering to match your supported matrix.
README.md (2)
116-117: Good clarity on v6 scope.The note sets correct expectations for firmware 6.
34-34: Verify My Home Assistant redirect slug.The path includes “/_change/…config_flow_start”; typical slugs omit “_change”. Please confirm the correct redirect.
fixtures/userdata/NanoStation_M5_sta_v6.3.16.json (1)
1-113: Type consistency check for v6 model.Fixture mixes ints/strings/bools (e.g., "dfs": "0", services 0/1 vs true/false). Ensure airos6 dataclasses and deserializers accept these variations or normalize them in tests.
airos/airos8.py (1)
115-118: Good addition: log before raising on missing interfaces.The error log helps diagnosis while keeping the same failure semantics. LGTM.
tests/conftest.py (2)
24-30: LGTM: AirOS6 fixture.Creates/tears down session correctly; parameters align with constructor.
32-38: LGTM: AirOS8 fixture.Mirrors the v6 fixture appropriately.
tests/test_stations.py (1)
152-152: LGTM: additional AP-PTMP v8.7.18 fixture case.tests/test_airos6.py (2)
74-90: LGTM: invalid JSON test correctly targets JSONDecodeError → AirOSDataMissingError.
121-132: LGTM: derived_data fallback to first interface validated.airos/data.py (9)
163-165: Good addition: tolerate unknown v8 remote.modeAdding WirelessMode.UNKNOWN guards against unexpected remote.mode values seen in the field. Looks good.
167-172: v6 mode enum is clear and minimalWireless6Mode with "sta"/"ap" matches v6 payloads. LGTM.
215-215: Confirm device_id presence across v8 payloadsMoving device_id within Host is fine, but please verify all v8 fixtures still include device_id (required) to avoid MissingField during deserialization.
257-264: Services6 surface looks rightSubset aligns with v6. No issues spotted.
526-526: ieeemode placement inside WirelessField reorder is fine. Ensure all v8 devices populate ieeemode; otherwise consider Optional[IeeeMode].
609-619: Interface6Status minimal set is OKMatches v6 surface without counters. LGTM.
629-629: MTU after statusReordering MTU under Interface is harmless; confirm MTU always present in v8 payloads.
632-641: Interface6: optional MTUGood call making MTU optional for v6.
702-715: AirOS6Data aggregator looks consistentField set aligns with new v6 leaves. Nice.
tests/test_airos8.py (18)
18-35: CSRF-missing login test setup looks good (skipped)Mocks are correct; once fixed upstream, remove skip to restore coverage.
38-45: Connection error path exercised correctlyGood negative-path coverage for login.
49-54: Status requires connectionSolid guard test.
76-91: Invalid JSON → AirOSDataMissingErrorHappy-path for error mapping. LGTM.
94-112: Missing interfaces → key errorCovers derived_data path well. LGTM.
115-120: Derived-data guard (no interfaces)Good unit to assert the early failure.
123-134: Fallback MAC/interface selection pathValidates fallback to first interface. Nice.
138-143: stakick requires connectionGuard behavior covered.
146-151: stakick validates mac_addressError mapping is correct.
170-178: stakick connection errorGood.
182-187: provmode requires connectionGuard test is fine.
240-248: provmode connection error pathCovers negative path.
251-283: Surface MissingField as causeGreat improvement asserting the underlying mashumaro cause is preserved and logged.
316-333: warnings() invalid JSON negative pathGood.
340-361: update_check(): positive path with CSRFSolid coverage of header-requiring route.
367-385: update_check() invalid JSONGood.
392-402: warnings() when not connectedGood guard.
408-418: update_check() when not connectedGood guard.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
airos/airos8.py (1)
288-305: Content-Type/body mismatch in update_check(force=True).You set ct_form=True but send json_data, yielding an application/x-www-form-urlencoded header with a JSON body. Align the header with the payload. Simplest: always send JSON.
- async def update_check(self, force: bool = False) -> dict[str, Any]: - """Check for firmware updates.""" - if force: - return await self._request_json( - "POST", - self._update_check_url, - json_data={"force": True}, - authenticated=True, - ct_form=True, - ) - return await self._request_json( - "POST", - self._update_check_url, - json_data={}, - authenticated=True, - ct_json=True, - ) + async def update_check(self, force: bool = False) -> dict[str, Any]: + """Check for firmware updates.""" + payload = {"force": True} if force else {} + return await self._request_json( + "POST", + self._update_check_url, + json_data=payload, + authenticated=True, + ct_json=True, + )
♻️ Duplicate comments (1)
script/mashumaro-step-debug.py (1)
62-64: Replace outdated warning; recognize v6 and flag unsupported majors.The message still says “Non firmware 8” even though v6 is supported. Prefer explicit handling of supported majors and a clearer warning for others.
- if fw_major != 8: - _LOGGER.warning("Non firmware 8 detected: %s", fwversion) + if fw_major == 6: + _LOGGER.info("Firmware v6 detected: %s", fwversion) + elif fw_major == 8: + _LOGGER.debug("Firmware v8 detected: %s", fwversion) + else: + _LOGGER.warning("Unsupported firmware major detected: %s", fwversion)
🧹 Nitpick comments (13)
airos/exceptions.py (1)
40-41: Docstring grammar nit: add article.Suggest “Raised when a method is not available for the device.”
-class AirOSNotSupportedError(AirOSException): - """Raised when method not available for device.""" +class AirOSNotSupportedError(AirOSException): + """Raised when a method is not available for the device."""tests/test_stations6.py (3)
36-39: Rename test for clarity (it validates STA, not AP).-async def test_ap_object( +async def test_sta_object( airos6_device: AirOS6, base_url: str, mode: str, fixture: str ) -> None: @@ - # Assertions remain the same as they check the final result + # Assertions remain the same as they check the final resultAlso consider parametrizing an AP fixture when available to cover both modes.
Also applies to: 61-63
65-67: Remove unused cookie setup.Not used by assertions; safe to drop.
- cookie = SimpleCookie() - cookie["session_id"] = "test-cookie" - cookie["AIROS_TOKEN"] = "abc123"
16-26: Deduplicate fixture loader with v8 test helper.file-level optional: Extract _read_fixture to a shared helper to avoid duplication with tests/test_stations.py.
tests/conftest.py (1)
3-3: Use public collections.abc, not private _collections_abc.-from _collections_abc import AsyncGenerator, Generator +from collections.abc import AsyncGenerator, Generatorscript/generate_ha_fixture.py (2)
26-26: Drop debug print; prefer logger.debug to keep script output clean.- print(f"Loading AirOS6 from: {inspect.getfile(AirOS6)}") + _LOGGER.debug("Loading AirOS6 from: %s", inspect.getfile(AirOS6))
35-37: Guard missing userdata directory to avoid crashing when path is absent.- for filename in os.listdir(userdata_dir): # noqa: PTH208 + if not os.path.isdir(userdata_dir): + _LOGGER.error("Userdata directory does not exist: %s", userdata_dir) + return + for filename in os.listdir(userdata_dir): # noqa: PTH208airos/airos6.py (2)
12-12: Unused logger.Module logger isn’t used here; remove or use for future logs.
-_LOGGER = logging.getLogger(__name__) +# _LOGGER = logging.getLogger(__name__) # add when logging is needed
15-16: Inherited base_url logic drops ports; fix in base class to support host:port.Non-default ports break today because AirOS.init uses hostname only. Apply in airos/airos8.AirOS (base class) so v6 picks it up too:
- parsed_host = urlparse(host) - scheme = ( - parsed_host.scheme - if parsed_host.scheme - else ("https" if use_ssl else "http") - ) - hostname = parsed_host.hostname if parsed_host.hostname else host - - self.base_url = f"{scheme}://{hostname}" + parsed_host = urlparse(host) + if parsed_host.scheme: + netloc = parsed_host.netloc or parsed_host.path # keep :port if present + self.base_url = f"{parsed_host.scheme}://{netloc}".rstrip("/") + else: + scheme = "https" if use_ssl else "http" + self.base_url = f"{scheme}://{host}".rstrip("/")airos/airos8.py (2)
33-34: Remove unused TypeVar or wire it into the public API.AirOSDataModel is declared but unused in this file.
-from typing import Any, TypeVar +from typing import Any ... -AirOSDataModel = TypeVar("AirOSDataModel", bound=AirOSDataBaseClass)
108-156: Helper is solid; fix minor wording and consider tiny safety.
- Typo: “fist” -> “first”.
- Optional: guard index 0 access with a truthy check (already covered by the earlier not-interfaces branch).
- # Fallback take fist alternate interface found + # Fallback: take first alternate interface foundairos/data.py (2)
236-255: Nit: log dataclass name as Host6 in pre-deserialize.Current message uses “Host”, which can be confusing during v6 debugging.
- _check_and_log_unknown_enum_value(d, "netrole", NetRole, "Host", "netrole") + _check_and_log_unknown_enum_value(d, "netrole", NetRole, "Host6", "netrole")
556-597: Wireless6 mapping LGTM; fix class name in security check.The enum checks are good; adjust dataclass_name to Wireless6 for accurate logs.
- _check_and_log_unknown_enum_value( - d, "security", Security, "Wireless", "security" - ) + _check_and_log_unknown_enum_value( + d, "security", Security, "Wireless6", "security" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
airos/airos6.py(1 hunks)airos/airos8.py(7 hunks)airos/data.py(11 hunks)airos/exceptions.py(1 hunks)pyproject.toml(1 hunks)script/generate_ha_fixture.py(3 hunks)script/mashumaro-step-debug.py(2 hunks)tests/conftest.py(2 hunks)tests/test_stations6.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (7)
tests/test_stations6.py (6)
airos/airos6.py (1)
AirOS6(15-64)airos/data.py (1)
AirOS6Data(714-725)tests/test_stations.py (2)
_read_fixture(18-28)test_ap_object(156-187)tests/conftest.py (2)
airos6_device(24-29)base_url(18-20)airos/airos8.py (2)
login(254-260)status(262-286)tests/test_airos8.py (1)
test_login_no_csrf_token(18-34)
airos/airos6.py (3)
airos/airos8.py (9)
AirOS(36-374)derived_wireless_data(83-106)update_check(288-304)stakick(306-321)provmode(323-337)warnings(339-341)progress(343-352)download(354-363)install(365-374)airos/data.py (2)
AirOS6Data(714-725)DerivedWirelessRole(153-157)airos/exceptions.py (1)
AirOSNotSupportedError(40-41)
script/mashumaro-step-debug.py (2)
airos/airos6.py (2)
AirOS6(15-64)derived_wireless_data(21-36)airos/airos8.py (4)
AirOS(36-374)derived_data(157-159)_derived_data_helper(109-155)derived_wireless_data(83-106)
airos/data.py (1)
airos/airos8.py (1)
status(262-286)
script/generate_ha_fixture.py (4)
airos/airos6.py (2)
AirOS6(15-64)derived_wireless_data(21-36)airos/airos8.py (4)
AirOS(36-374)derived_data(157-159)_derived_data_helper(109-155)derived_wireless_data(83-106)airos/data.py (2)
AirOS6Data(714-725)AirOS8Data(693-710)tests/test_airos8.py (2)
test_derived_data_no_br0_eth0_ath0(123-133)test_derived_data_no_interfaces_key(115-119)
tests/conftest.py (4)
airos/airos6.py (1)
AirOS6(15-64)airos/airos8.py (1)
AirOS(36-374)tests/test_airos8.py (3)
test_update_check_correctly_parses_json(337-360)test_stakick_no_mac_address(146-150)test_derived_data_no_interfaces_key(115-119)tests/test_airos_request.py (1)
mock_airos_device(26-33)
airos/airos8.py (2)
airos/data.py (4)
AirOS8Data(693-710)AirOSDataBaseClass(113-119)DerivedWirelessRole(153-157)DerivedWirelessMode(160-164)airos/airos6.py (1)
derived_wireless_data(21-36)
🔇 Additional comments (22)
tests/test_stations6.py (1)
43-48: Good: patching the internal _request_json avoids real I/O.Solid approach with AsyncMock side_effect and forcing connected=True.
tests/conftest.py (1)
24-38: Fixtures look good and close sessions properly.Version-specific airos6_device/airos8_device split is clean; session lifecycle handled.
script/generate_ha_fixture.py (2)
51-66: Nice hardening of fwversion parsing.Safer host access + clear errors on missing/invalid fwversion.
67-67: Union type operator supported by project Python requirement The project’s pyproject.toml specifiesrequires-python = ">=3.13", so PEP 604 union syntax (AirOS6Data | AirOS8Data) is valid.airos/airos6.py (1)
38-64: Clear, explicit NotSupported errors for v6-only gaps.Consistent use of AirOSNotSupportedError with helpful messages. Good.
script/mashumaro-step-debug.py (5)
15-26: Version-aware imports look good.Public aliases and per-version data classes wired correctly for v6/v8.
48-55: Good null-safety and actionable error.Defensive host access and explicit file context in the error message are solid.
56-61: Robust fwversion parsing.Covers bad formats with clear logging and exception chaining.
96-104: Top-level Wireless deserialization per version is correct.Union typing is helpful for readers and tooling.
116-132: Derived-data selection per version looks correct.Using each class’s helper ensures consistent derived fields before deserialization.
airos/airos8.py (3)
39-40: Configurable data model default LGTM.Defaulting to AirOS8Data while allowing overrides enables clean v6 specialization.
82-106: Wireless derived-data mapping LGTM.Cases correctly set access_point/station and ptp/ptmp, including role/mode when applicable.
262-271: Status() pipeline LGTM.Version-agnostic derived_data + model-driven from_dict is the right abstraction.
airos/data.py (9)
174-176: Adding WirelessMode.UNKNOWN is appropriate.Covers observed remote.mode anomalies without breaking enums.
178-183: Wireless6Mode enum LGTM.Matches v6’s simple “ap/sta” values.
269-275: Services6 looks good.Subset aligns with v6 payloads.
277-283: Airview6 leaf LGTM.Minimal and matches v6 shape.
640-641: Interface.mtu position change: confirm presence across devices.mtu is required here. If some devices omit it, consider making it Optional[int] like Interface6, or ensure tests cover absence cases.
643-652: Interface6 definition LGTM.Optional mtu reflects v6 variance.
693-711: AirOS8Data now inherits from the base—ensure no alias side effects.Couples with the earlier alias change; rely on verification to catch regressions.
714-726: AirOS6Data container LGTM.Field set mirrors observed v6 payloads and pairs with derived data.
112-121: Verify deserialization still accepts lowercase keys
Your verification script failed due to the missingmashumarodependency. Install it (pip install mashumaro) and rerun your test to confirm thatAirOS8Data.from_dictandAirOS6Data.from_dictstill accept the lowercase JSON keys from device outputs.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_airos6.py (1)
135-169: Remove skip and update test to assert the cause
In tests/test_airos6.py, unskiptest_status_missing_required_key_in_jsonand adjust the final assertion comment:- @pytest.mark.skip(reason="broken, needs investigation") @pytest.mark.asyncio async def test_status_missing_required_key_in_json(airos6_device: AirOS6) -> None: @@ - # --- MODIFICATION START --- - # Assert that the cause of our exception is the correct type from mashumaro - assert isinstance(excinfo.value.__cause__, MissingField) + # Cause should be a mashumaro MissingField + assert isinstance(excinfo.value.__cause__, MissingField)This aligns AirOS6.status (inherited from AirOS) with AirOS8.status error handling.
🧹 Nitpick comments (7)
tests/test_airos6.py (7)
1-2: Nit: Fix module docstring capitalization.Prefer "AirOS6" for consistency with class/module names.
-"""Additional tests for airOS6 module.""" +"""Additional tests for AirOS6 module."""
3-13: Trim unused imports or unskip the dependent test.
MissingFieldis only used in a skipped test. Either unskip that test (preferred) or drop the import to keep lint clean.-from mashumaro.exceptions import MissingField +# from mashumaro.exceptions import MissingField # re-enable when unskipping related test
55-71: Replace triple-quoted disabled test with a real test or mark xfail.The quoted block dodges linters and obscures intent. Either enable it or explicitly xfail with a reason and a tracking issue.
-# pylint: disable=pointless-string-statement -''' -@pytest.mark.asyncio -async def test_status_non_200_response(airos6_device: AirOS6) -> None: +@pytest.mark.asyncio +async def test_status_non_200_response(airos6_device: AirOS6) -> None: @@ - with ( - patch.object(airos6_device.session, "request", return_value=mock_status_response), - pytest.raises(airos.exceptions.AirOSDeviceConnectionError), - ): - await airos6_device.status() -''' + with ( + patch.object(airos6_device.session, "request", return_value=mock_status_response), + pytest.raises(airos.exceptions.AirOSDeviceConnectionError), + ): + await airos6_device.status()If behavior isn’t finalized yet:
-async def test_status_non_200_response(airos6_device: AirOS6) -> None: +@pytest.mark.xfail(reason="Finalize v6 non-200 -> exception mapping", strict=False) +async def test_status_non_200_response(airos6_device: AirOS6) -> None:
75-90: Strengthen the invalid-JSON assertion by checking the cause.If
_request_jsonbubblesjson.JSONDecodeError, assert it as the__cause__for better diagnostics.- with ( - patch.object( - airos6_device.session, "request", return_value=mock_status_response - ), - pytest.raises(airos.exceptions.AirOSDataMissingError), - ): - await airos6_device.status() + with ( + patch.object(airos6_device.session, "request", return_value=mock_status_response), + pytest.raises(airos.exceptions.AirOSDataMissingError) as excinfo, + ): + await airos6_device.status() + assert isinstance(excinfo.value.__cause__, json.JSONDecodeError)
114-119: Remove brittle “line 206” reference.Inline line numbers drift quickly; prefer a neutral comment.
- # This will directly test the 'if not interfaces:' branch (line 206) + # This directly exercises the 'no interfaces' branch.
14-14: Reduce mock-response boilerplate with a small helper.Cuts repetition and improves readability across tests.
+ +def _mock_response(status: int, body: str | dict, *, cookies: SimpleCookie | None = None, headers: dict | None = None): + """Create an aiohttp-like async context manager mock response.""" + mock_resp = MagicMock() + mock_resp.__aenter__.return_value = mock_resp + mock_resp.status = status + mock_resp.text = AsyncMock(return_value=json.dumps(body) if isinstance(body, dict) else body) + mock_resp.cookies = cookies or SimpleCookie() + mock_resp.headers = headers or {} + return mock_respUsage example:
mock_status_response = _mock_response(200, {"system": {}})
1-169: Additional coverage suggestion: assert NotSupported APIs raise.Add tests that
update_check,stakick,provmode,warnings,progress,download,installraiseAirOSNotSupportedErroron v6. Say the word and I’ll draft them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
pyproject.toml(1 hunks)tests/test_airos6.py(1 hunks)tests/test_stations.py(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pyproject.toml
- tests/test_stations.py
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_airos6.py (5)
airos/airos6.py (1)
AirOS6(15-64)tests/test_airos8.py (1)
test_status_missing_required_key_in_json(251-282)tests/conftest.py (1)
airos6_device(24-29)airos/airos8.py (3)
status(262-286)login(254-260)derived_data(157-159)airos/exceptions.py (3)
AirOSDeviceConnectionError(24-25)AirOSDataMissingError(16-17)AirOSKeyDataMissingError(20-21)
🔇 Additional comments (5)
tests/test_airos6.py (5)
36-44: LGTM: connection error mapping.Mapping aiohttp.ClientError to AirOSDeviceConnectionError on login looks correct.
48-53: LGTM: status guards unauthenticated access.Raising AirOSDeviceConnectionError when not connected is appropriate.
93-111: LGTM: missing interfaces => key-data error.This matches the derived_data() contract.
121-133: LGTM: fallback MAC interface selection.Asserting mac_interface/mac with only wan0 present is a good edge case.
15-34: Decide on AirOS6.login CSRF‐missing contract and update test accordingly
Remove the skip ontest_login_no_csrf_token, then assert the real behavior whenX-CSRF-IDis absent (eitherNonereturn or specific exception from the inherited AirOS.login). If it now errors, wrap the call inpytest.raises(<the actual AirOSException subclass>); otherwise, assertawait airos6_device.login() is None.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
tests/test_helpers.py (4)
108-116: Decouple test from AirOS.login internals; patch login directly.Relying on
_request_jsonto also servicelogin()is brittle. PatchAirOS.loginto a noop and make_request_jsonreturn only the status payload.Apply this diff:
- mock_request_json = AsyncMock( - side_effect=[ - {}, # First call for login() - mock_response, # Second call for the status() endpoint - ] - ) + mock_request_json = AsyncMock(return_value=mock_response) @@ - with patch.object(AirOS, "_request_json", new=mock_request_json): + with patch.object(AirOS, "login", new=AsyncMock()), patch.object(AirOS, "_request_json", new=mock_request_json):
16-20: Fixture type is fine; consider AsyncMock if you start awaiting session methods.Current tests don’t use session methods. If you later await
sessioncalls, switch toAsyncMockfor accuracy.
23-31: Name your parametrized cases for quick triage.Add
idsto parametrize for clearer failures (e.g., "success_v8", "success_v6", "missing_host", …).Example:
@pytest.mark.parametrize( (...), [...], ids=["success_v8","success_v6","missing_host","missing_fwversion","invalid_fwversion","missing_hostname","missing_mac"], )
32-58: Add edge-case versions to harden parsing.Consider extra cases like "V6.5.0" (uppercase V) and "8.7.4" (no 'v') to validate version parsing robustness.
airos/helpers.py (6)
27-27: Docstring mismatches return shape.This returns fw_major, mac, and hostname — not just the major version. Update the docstring.
Apply this diff:
- """Connect to a device and return the major firmware version.""" + """Connect to a device and return firmware metadata: fw_major, mac, and hostname."""
41-46: Make firmware parsing more robust (uppercase 'V', no dot, suffixes).Use a regex to extract the leading integer rather than
lstrip("v").split(".", 1)[0].Apply these diffs:
@@ - try: - fw_major = int(fw_version.lstrip("v").split(".", 1)[0]) - except (ValueError, AttributeError) as exc: - raise AirOSKeyDataMissingError( - f"Invalid firmware version '{fw_version}'" - ) from exc + match = re.match(r"^[vV]?(\d+)", fw_version) + if not match: + raise AirOSKeyDataMissingError(f"Invalid firmware version '{fw_version}'") + fw_major = int(match.group(1))And add the import:
@@ -import aiohttp +import aiohttp +import re
48-56: Be explicit about supported majors; avoid defaulting unknown to v8.Defaulting everything not 6 to the v8 path may mask unsupported variants. Consider handling {6, 8} explicitly and raising on others (e.g., AirOSNotSupportedError).
Apply this diff (assuming the exception exists):
@@ - if fw_major == 6: + if fw_major == 6: derived_data = AirOS6._derived_data_helper( # noqa: SLF001 raw_status, AirOS6.derived_wireless_data ) - else: # Assume AirOS 8 for all other versions + elif fw_major == 8: derived_data = AirOS._derived_data_helper( # noqa: SLF001 raw_status, AirOS.derived_wireless_data ) + else: + raise AirOSNotSupportedError(f"Unsupported AirOS major version: {fw_major}")Add the import if available:
from .exceptions import AirOSNotSupportedError # alongside AirOSKeyDataMissingError
30-35: Avoid reaching into private API if feasible.Using
_request_jsonand_status_cgi_urlcouples this helper to internals. If possible, expose a publicstatus_json()onAirOSand call that here.
61-66: Clarify missing-key messages with full paths.Make messages consistent with key paths to speed debugging (e.g., "Missing host.hostname", "Missing derived.mac").
Apply this diff:
- if not hostname: - raise AirOSKeyDataMissingError("Missing hostname") + if not hostname: + raise AirOSKeyDataMissingError("Missing host.hostname") @@ - if not mac: - raise AirOSKeyDataMissingError("Missing MAC address") + if not mac: + raise AirOSKeyDataMissingError("Missing derived.mac")
28-35: StubAirOS.login()in tests to avoid duplicate_request_jsoncalls
AirOS.login()internally invokesawait self._request_json("POST", self._login_url, json_data=payload), so a test that mocks_request_jsonfor the status call will see two invocations (login + status) and may flake. In your tests, stub outlogin()to a no-op or adjust your mock to expect both calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
airos/helpers.py(1 hunks)pyproject.toml(1 hunks)tests/test_helpers.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (2)
airos/helpers.py (4)
airos/airos6.py (1)
AirOS6(15-64)airos/airos8.py (5)
AirOS(36-374)derived_data(157-159)_derived_data_helper(109-155)AirOS(32-349)derived_data(77-134)airos/exceptions.py (1)
AirOSKeyDataMissingError(20-21)airos/data.py (4)
Host(190-213)AirOSDataClass(107-108)Derived(551-566)Wireless(448-496)
tests/test_helpers.py (1)
airos/helpers.py (2)
DetectDeviceData(12-17)async_get_firmware_data(20-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run pytest using Python 3.13
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
airos/airos6.py (3)
17-37: Base URL construction still drops ports; fix in AirOS base.AirOS6 delegates host parsing to AirOS.init, which currently uses hostname only and loses :port. Please apply the fix in airos/base.py to preserve netloc.
Proposed change in airos/base.py init:
- scheme = ( - parsed_host.scheme - if parsed_host.scheme - else ("https" if use_ssl else "http") - ) - hostname = parsed_host.hostname if parsed_host.hostname else host - - self.base_url = f"{scheme}://{hostname}" + if parsed_host.scheme: + netloc = parsed_host.netloc or parsed_host.path # keep :port if present + self.base_url = f"{parsed_host.scheme}://{netloc}".rstrip("/") + else: + scheme = "https" if use_ssl else "http" + self.base_url = f"{scheme}://{host}".rstrip("/")
17-19: Map non-login ClientResponseError to DeviceConnectionError (in base).Ensure _request_json only maps login (auth) failures to setup/auth errors; other endpoints should raise DeviceConnectionError.
If not already addressed in airos/base.py:
- If url == self._login_url:
- 401 -> AirOSConnectionAuthenticationError
- else -> AirOSConnectionSetupError
- Else: AirOSDeviceConnectionError
43-53: Set ptp=True only for stations; APs shouldn’t default to PTP.This incorrectly marks APs as PTP. Move the assignment under the "sta" branch.
- # Access Point / Station - no info on ptp/ptmp - # assuming ptp for station mode - derived["ptp"] = True + # Access Point / Station - no info on ptp/ptmp + # assume ptp for station mode only wireless_mode = response.get("wireless", {}).get("mode", "") match wireless_mode: case "ap": derived["access_point"] = True derived["role"] = DerivedWirelessRole.ACCESS_POINT case "sta": derived["station"] = True + derived["ptp"] = Truetests/conftest.py (1)
10-11: Good move: version-specific imports (AirOS6/AirOS8).This aligns tests with the new split API.
🧹 Nitpick comments (14)
pyproject.toml (1)
13-14: Alpha version but “Production/Stable” classifierIf keeping an alpha pre-release (a0), consider aligning the classifier to “4 - Beta” to set correct expectations.
Example:
- "Development Status :: 5 - Production/Stable", + "Development Status :: 4 - Beta",CHANGELOG.md (2)
5-10: Wording polish for 0.5.1 entry“to consume increasing mypy options for consumption” is awkward/redundant. Suggest a concise phrasing.
- - Created a base class based on AirOS8 for both v6 and v8 to consume increasing mypy options for consumption + - Introduced a shared base class (derived from AirOS8) used by both v6 and v8; increased mypy strictness
11-20: Section structure for 0.5.0“Initial support for firmware 6” sits outside a subsection. Move under “Added” for consistency.
-## [0.5.0] - Not released - -Initial support for firmware 6 - -### Added +## [0.5.0] - Not released + +### Added +- Initial support for firmware 6Also, if 0.5.1 is released while pyproject is 0.5.1a0, clarify status: either mark 0.5.1 as “Unreleased” or bump the version to 0.5.1. See pyproject comment.
airos/base.py (4)
67-72: Remove unused auth flags
_use_json_for_login_postandcurrent_csrf_tokenare unused.- self._use_json_for_login_post = False @@ - self.current_csrf_token: str | None = None
197-207: Return type redundancy
-> dict[str, Any] | Anyis equivalent toAny. Simplify the annotation.- ) -> dict[str, Any] | Any: + ) -> Any:
142-149: Defensive access for interface keys + typoPotential KeyError if an interface dict lacks “enabled/ifname/hwaddr”. Also fix comment typo “fist”.
- for interface in interfaces: - if interface["enabled"]: # Only consider if enabled - addresses[interface["ifname"]] = interface["hwaddr"] + for interface in interfaces: + if interface.get("enabled"): + ifname = interface.get("ifname") + hwaddr = interface.get("hwaddr") + if ifname and hwaddr: + addresses[ifname] = hwaddr @@ - # Fallback take fist alternate interface found + # Fallback: take first available interface when no preferred match is found
85-110: Station role explicitness (optional)For “sta-ptp/sta-ptmp” you rely on default role=STATION. For clarity, consider setting
derived["role"] = DerivedWirelessRole.STATIONin both branches.case "sta-ptmp": derived["station"] = True derived["ptmp"] = True derived["mode"] = DerivedWirelessMode.PTMP + derived["role"] = DerivedWirelessRole.STATION @@ case "sta-ptp": derived["station"] = True derived["ptp"] = True + derived["role"] = DerivedWirelessRole.STATIONtests/test_airos_request.py (2)
50-52: Consider testing authenticated header assemblyAdd a test exercising
authenticated=Truewith preset_csrf_id/_auth_cookieto verify headers includeX-CSRF-IDandCookie. This will catch regressions in auth handling.
146-149: Add a guard-path testAdd a test asserting
_request_jsonraisesAirOSDeviceConnectionErrorwhenconnectedis False and URL != login, ensuring the not-connected guard remains enforced.airos/airos6.py (2)
28-28: Fix class init docstring: it says AirOS8.Change to AirOS6 for accuracy.
- """Initialize AirOS8 class.""" + """Initialize AirOS6 class."""
56-82: Normalize error messages: use “AirOS 6” consistently.Minor UX polish: add a space for consistency with class docstring.
- raise AirOSNotSupportedError("Firmware update check not supported on AirOS6.") + raise AirOSNotSupportedError("Firmware update check not supported on AirOS 6.") - raise AirOSNotSupportedError("Station kick not supported on AirOS6.") + raise AirOSNotSupportedError("Station kick not supported on AirOS 6.") - raise AirOSNotSupportedError("Provisioning mode not supported on AirOS6.") + raise AirOSNotSupportedError("Provisioning mode not supported on AirOS 6.") - raise AirOSNotSupportedError("Device warnings not supported on AirOS6.") + raise AirOSNotSupportedError("Device warnings not supported on AirOS 6.") - raise AirOSNotSupportedError("Firmware progress not supported on AirOS6.") + raise AirOSNotSupportedError("Firmware progress not supported on AirOS 6.") - raise AirOSNotSupportedError("Firmware download not supported on AirOS6.") + raise AirOSNotSupportedError("Firmware download not supported on AirOS 6.") - raise AirOSNotSupportedError("Firmware install not supported on AirOS6.") + raise AirOSNotSupportedError("Firmware install not supported on AirOS 6.")tests/conftest.py (3)
24-30: Use async context manager to ensure session closure on setup errors.Prevents leaking sessions if instantiation fails before yield.
-async def airos6_device(base_url: str) -> AsyncGenerator[AirOS6, None]: - """AirOS6 device fixture.""" - session = aiohttp.ClientSession(cookie_jar=aiohttp.CookieJar()) - instance = AirOS6(base_url, "username", "password", session, use_ssl=False) - yield instance - await session.close() +async def airos6_device(base_url: str) -> AsyncGenerator[AirOS6, None]: + """AirOS6 device fixture.""" + async with aiohttp.ClientSession(cookie_jar=aiohttp.CookieJar()) as session: + instance = AirOS6(base_url, "username", "password", session, use_ssl=False) + yield instance
32-38: Same here: guard aiohttp session with async with.-async def airos8_device(base_url: str) -> AsyncGenerator[AirOS8, None]: - """AirOS8 device fixture.""" - session = aiohttp.ClientSession(cookie_jar=aiohttp.CookieJar()) - instance = AirOS8(base_url, "username", "password", session, use_ssl=False) - yield instance - await session.close() +async def airos8_device(base_url: str) -> AsyncGenerator[AirOS8, None]: + """AirOS8 device fixture.""" + async with aiohttp.ClientSession(cookie_jar=aiohttp.CookieJar()) as session: + instance = AirOS8(base_url, "username", "password", session, use_ssl=False) + yield instance
3-3: Avoid private module _collections_abc.Import from collections.abc (public API).
-from _collections_abc import AsyncGenerator, Generator +from collections.abc import AsyncGenerator, Generator
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)airos/airos6.py(1 hunks)airos/airos8.py(1 hunks)airos/base.py(1 hunks)pyproject.toml(1 hunks)tests/conftest.py(2 hunks)tests/test_airos8.py(10 hunks)tests/test_airos_request.py(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_airos8.py
🧰 Additional context used
🧬 Code graph analysis (5)
airos/base.py (3)
airos/data.py (6)
AirOSDataBaseClass(113-119)DerivedWirelessMode(160-164)DerivedWirelessRole(153-157)redact_data_smart(43-102)AirOSDataClass(107-108)AirOS8Data(570-587)airos/exceptions.py (5)
AirOSConnectionAuthenticationError(12-13)AirOSConnectionSetupError(8-9)AirOSDataMissingError(16-17)AirOSDeviceConnectionError(24-25)AirOSKeyDataMissingError(20-21)airos/airos6.py (2)
derived_wireless_data(39-54)provmode(64-66)
tests/conftest.py (2)
airos/airos6.py (1)
AirOS6(17-82)airos/airos8.py (1)
AirOS8(11-30)
tests/test_airos_request.py (3)
airos/airos8.py (1)
AirOS8(11-30)airos/base.py (1)
_request_json(197-255)airos/exceptions.py (3)
AirOSDeviceConnectionError(24-25)AirOSConnectionAuthenticationError(12-13)AirOSDataMissingError(16-17)
airos/airos6.py (3)
airos/base.py (9)
AirOS(36-377)derived_wireless_data(86-109)update_check(291-307)stakick(309-324)provmode(326-340)warnings(342-344)progress(346-355)download(357-366)install(368-377)airos/data.py (5)
AirOS6Data(714-725)DerivedWirelessRole(153-157)Derived(551-566)Wireless(448-496)AirOSDataClass(107-108)airos/exceptions.py (4)
AirOSNotSupportedError(40-41)AirOSDataMissingError(16-17)AirOSConnectionSetupError(8-9)AirOSDeviceConnectionError(24-25)
airos/airos8.py (2)
airos/base.py (1)
AirOS(36-377)airos/data.py (5)
AirOS8Data(693-710)AirOS8Data(570-587)Host(190-213)Derived(551-566)Wireless(448-496)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~17-~17: There might be a mistake here.
Context: ... ### Added - Add logging redacted data on interface [issue](https://github.com/ho...
(QB_NEW_EN)
🔇 Additional comments (3)
pyproject.toml (1)
7-7: Version vs changelog mismatchProject version is 0.5.1a0 while CHANGELOG declares 0.5.1 as released on 2025-08-31. Either publish as 0.5.1 or mark the changelog entry as 0.5.1a0/unreleased to avoid confusion.
Apply one of:
- version = "0.5.1a0" + version = "0.5.1"or adjust CHANGELOG headings accordingly (see separate comment).
tests/test_airos_request.py (1)
9-9: Import swap to AirOS8Good alignment with the new per-version class structure.
airos/airos8.py (1)
11-30: LGTM – slim wrapper over the new baseConstructor correctly binds
AirOS8Datato the generic base. No issues spotted.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_stations.py (1)
73-80: Assert the correct logger and extract redacted payload from error, not exception.Redaction path should log via
_LOGGER.error(...)(not.exception(...)).- assert mock_logger.exception.called - assert mock_logger.exception.call_count == 1 - assert mock_logger.error.called is False + assert mock_logger.error.called + assert mock_logger.error.call_count == 1 + assert mock_logger.exception.called is False @@ - logged_data = mock_logger.exception.call_args[0][1] + logged_data = mock_logger.error.call_args[0][1]
♻️ Duplicate comments (4)
script/mashumaro-step-debug.py (3)
62-64: Adjust log message now that v6 is supported.
Log info for v6; warn only for unsupported majors.- if fw_major != 8: - _LOGGER.warning("Non firmware 8 detected: %s", fwversion) + if fw_major == 6: + _LOGGER.info("Firmware v6 detected: %s", fwversion) + elif fw_major != 8: + _LOGGER.warning("Unsupported firmware major detected: %s", fwversion)
15-26: Redact potential PII in logs (import helper).
Bring in the redaction helper for safe logging of Remote data.from airos.data import ( # noqa: E402 AirOS6Data, AirOS8Data, Interface, Interface6, + redact_data_smart, Remote, Station, Wireless, Wireless6, )
78-87: Gate Station/Remote parsing strictly to v8 and redact Remote data.
Prevents mis-parsing future majors and avoids logging sensitive fields.- if fw_major >= 8: + if fw_major == 8: _LOGGER.info(" -> Checking list of Station objects...") station_list_data = wireless_data["sta"] station_obj_list = [] for i, station_data in enumerate(station_list_data): _LOGGER.info(" -> Checking Station object at index %s...", i) remote_data = station_data["remote"] _LOGGER.info(" -> Checking Remote object at index %s...", i) - _LOGGER.info("Remote data = %s", remote_data) + _LOGGER.info("Remote data = %s", redact_data_smart(remote_data)) remote_obj = Remote.from_dict(remote_data) # noqa: F841 _LOGGER.info(" Success! Remote is valid.")tests/test_stations.py (1)
60-61: Route patches through session.request (duplicate of prior feedback).Patch
session.requestonce with aside_effectlist so calls still flow through_request_jsonand avoid accidental real I/O.Apply:
- patch.object(airos8_device.session, "post", return_value=mock_login_response), - patch.object(airos8_device.session, "get", return_value=mock_status_response), + patch.object( + airos8_device.session, + "request", + side_effect=[mock_login_response, mock_status_response], + ),
🧹 Nitpick comments (7)
script/generate_ha_fixture.py (3)
26-26: Use logger instead of print for consistency.
Switch the debug print to_LOGGER.debugto keep output controllable by log level.- print(f"Loading AirOS6 from: {inspect.getfile(AirOS6)}") + _LOGGER.debug("Loading AirOS6 from: %s", inspect.getfile(AirOS6))
48-50: Minor: use json.load() directly.
Slightly simpler and avoids reading the whole file into memory.- with open(base_fixture_path, encoding="utf-8") as source: # noqa: PTH123 - source_data = json.loads(source.read()) + with open(base_fixture_path, encoding="utf-8") as source: # noqa: PTH123 + source_data = json.load(source)
85-90: Optionally continue on bad files instead of aborting.
You log “Skipping …” but re-raise, which stops processing the rest. If the intent is to process what’s valid, continue instead.- except json.JSONDecodeError: - _LOGGER.error("Skipping '%s': Not a valid JSON file.", filename) - raise - except Exception as e: - _LOGGER.error("Error processing '%s': %s", filename, e) - raise + except json.JSONDecodeError: + _LOGGER.error("Skipping '%s': Not a valid JSON file.", filename) + continue + except Exception as e: + _LOGGER.error("Error processing '%s': %s", filename, e) + continuetests/test_stations.py (4)
43-48: Model login mock on real auth: include Set-Cookie header.
_store_auth_dataparses theSet-Cookieheader; relying on.cookiesalone risks leavingconnectedfalse when this test is un-skipped.- mock_login_response.headers = {"X-CSRF-ID": "test-csrf-token"} + mock_login_response.headers = { + "Set-Cookie": "AIROS_TOKEN=abc123; Path=/; HttpOnly", + "X-CSRF-ID": "test-csrf-token", + }
172-175: Approach is fine; tiny simplification available.You can set
airos8_device.connected = Truedirectly (no need to patch attribute) since this fixture instance is disposable per test.Example:
- patch.object(airos8_device, "connected", True), + # No patch needed; direct assign is simpler + # (restore not needed since fixture instance is per-test)
185-188: Remove unused cookie setup.These lines are dead and can confuse readers.
- cookie = SimpleCookie() - cookie["session_id"] = "test-cookie" - cookie["AIROS_TOKEN"] = "abc123"
203-207: stakick test setup looks good; consider asserting call args.Optionally assert
session.requestwas called with the expected URL/method/body to tighten regression coverage.Example:
airos8_device.session.request.assert_called() method, url = airos8_device.session.request.call_args[1]["method"], airos8_device.session.request.call_args[1]["url"] assert method in ("POST", "post") assert url.endswith("/api/stakick")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
airos/helpers.py(1 hunks)script/generate_ha_fixture.py(3 hunks)script/mashumaro-step-debug.py(2 hunks)tests/test_helpers.py(1 hunks)tests/test_stations.py(10 hunks)tests/test_stations6.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_helpers.py
- tests/test_stations6.py
- airos/helpers.py
🧰 Additional context used
🧬 Code graph analysis (3)
tests/test_stations.py (4)
airos/airos8.py (2)
AirOS8(11-30)AirOS(32-349)airos/data.py (3)
AirOS8Data(693-710)Wireless(505-553)AirOS8Data(570-587)tests/conftest.py (2)
airos8_device(33-38)base_url(18-20)airos/exceptions.py (2)
AirOSKeyDataMissingError(20-21)AirOSDeviceConnectionError(24-25)
script/mashumaro-step-debug.py (3)
airos/airos6.py (2)
AirOS6(17-82)derived_wireless_data(39-54)airos/data.py (15)
AirOS6Data(714-725)AirOS8Data(693-710)Interface(633-640)Interface6(644-651)Remote(399-448)Station(466-501)Wireless(505-553)Wireless6(557-596)Wireless(448-496)AirOS8Data(570-587)Derived(551-566)Host(190-213)Remote(342-391)__pre_deserialize__(487-496)AirOSDataClass(107-108)airos/base.py (2)
derived_data(160-162)_derived_data_helper(112-158)
script/generate_ha_fixture.py (5)
airos/airos6.py (2)
AirOS6(17-82)derived_wireless_data(39-54)airos/airos8.py (2)
AirOS8(11-30)AirOS(32-349)airos/data.py (5)
AirOS6Data(714-725)AirOS8Data(693-710)Host(190-213)AirOS8Data(570-587)Derived(551-566)airos/base.py (2)
derived_data(160-162)_derived_data_helper(112-158)tests/test_airos8.py (3)
test_derived_data_no_br0_eth0_ath0(123-133)test_update_check_correctly_parses_json(337-360)test_derived_data_no_interfaces_key(115-119)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (7)
script/generate_ha_fixture.py (1)
51-66: FW version parsing hardening looks good.
Safe access, clear error messages, and guarded major parsing are correct.script/mashumaro-step-debug.py (2)
48-55: FW version presence checks: good.
Safe host access and clear error message.
56-61: FW major parsing: good.
Guarded conversion and explicit error path.tests/test_stations.py (4)
1-1: File purpose is clear.Docstring aligns with AirOS8-specific tests.
13-14: Imports updated to AirOS8/AirOS8Data — good alignment with refactor.
124-132: Good: unified on session.request with side_effect for login + status.
152-153: Fixture ‘NanoBeam_5AC_ap-ptmp_v8.7.18.json’ is present and valid.
Loaded successfully from fixtures/userdata and parses without errors.
|
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores