Skip to content

Conversation

@CoMPaTech
Copy link
Owner

@CoMPaTech CoMPaTech commented Aug 12, 2025

Summary by CodeRabbit

  • Bug Fixes

    • More consistent error handling across login, status, stakick, and provmode; login now returns False when an auth token is missing. Improved discovery transport setup and resilience.
  • Refactor

    • Tightened type hints and clarified method signatures for more predictable return types.
  • Tests

    • Substantially expanded and hardened test coverage, especially for discovery TLV edge cases and error paths.
  • Chores

    • Enabled type checking in CI and made coverage depend on it; added a mypy pre-commit hook, an environment helper script, test deps, and bumped version to 0.2.9.

@coderabbitai
Copy link

coderabbitai bot commented Aug 12, 2025

Walkthrough

Enables the mypy CI job and makes coverage depend on it; adds a local pre-commit mypy hook and an env bootstrap script; tightens typing and exception usage across airos modules; updates tests and scripts for new types/returns; bumps project version and adds mypy test overrides and test deps.

Changes

Cohort / File(s) Summary of Changes
CI workflow
.github/workflows/verify.yml
Unblocked the mypy job (removed if: false) and added - mypy to coverage job needs.
Pre-commit and env tooling
.pre-commit-config.yaml, script/run-in-env.sh
Added a local mypy pre-commit hook that runs via script/run-in-env.sh; added script/run-in-env.sh to activate Python envs before running commands.
Typing config & test deps
pyproject.toml, requirements-test.txt
Bumped project.version to 0.2.9 and added [[tool.mypy.overrides]] for tests.*; added mypy and types-aiofiles to test requirements.
Core: AirOS 8
airos/airos8.py
Tightened type hints and annotations (e.g., `current_csrf_token: str
Core: Data utilities
airos/data.py
Broadened function signatures to use dict[str, Any]; removed non-dict guard in helper; added type: ignore where needed to satisfy typing; redaction logic unchanged.
Core: Discovery
airos/discovery.py
Widened discovery callback type to Callable[[dict[str, Any]], Any] (allow async); connection_made uses provided transport.get_extra_info("socket").
Scripts
script/generate_ha_fixture.py, script/mashumaro-step-debug.py
Adjusted imports to use AirOS8Data as AirOSData; added explicit return type annotations; added a type: ignore[arg-type] on a derived_data call.
Tests: fixtures & suites
tests/conftest.py, tests/test_airos8.py, tests/test_data.py, tests/test_discovery.py, tests/test_stations.py
Extensive typing annotations and signature updates; tests updated to expect False or specific exceptions where behavior changed; discovery tests expanded for TLV edge cases and logging assertions.
Changelog
CHANGELOG.md
Added release entry 0.2.9 describing bug fixes, refactors, test additions, and chores.

Sequence Diagram(s)

sequenceDiagram
  actor Dev as Developer
  participant GA as GitHub Actions
  participant Ruff as ruff
  participant Pytest as pytest
  participant Mypy as mypy
  participant Cov as coverage

  Dev->>GA: Push/PR
  GA->>Ruff: start
  GA->>Pytest: start
  GA->>Mypy: start
  Ruff-->>GA: done
  Pytest-->>GA: done
  Mypy-->>GA: done
  GA->>Cov: start (needs: ruff, pytest, mypy)
  Cov-->>GA: done
Loading
sequenceDiagram
  participant User
  participant AirOS
  participant HTTP as aiohttp ClientSession

  User->>AirOS: login()
  AirOS->>HTTP: POST /api/login
  alt response contains CSRF header
    AirOS-->>User: True
  else missing CSRF header
    AirOS-->>User: False
  end

  User->>AirOS: status()
  AirOS->>HTTP: GET /status (include CSRF if present)
  alt 200 & valid JSON
    AirOS-->>User: dict status
  else error / invalid JSON
    AirOS-->>User: raise AirOS*Error
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

enhancement

Poem

I hopped through types and fixed a bug,
I nudged CI awake with a tiny tug.
Mypy now checks while coverage waits,
Sockets listen, tests explore edge states.
A carrot for commits — 0.2.9, snug and smug 🥕🐇

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch upd

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link

codecov bot commented Aug 12, 2025

Codecov Report

❌ Patch coverage is 99.31507% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.05%. Comparing base (475dc24) to head (58d50bf).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
airos/data.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #60      +/-   ##
==========================================
+ Coverage   94.98%   96.05%   +1.07%     
==========================================
  Files           9        9              
  Lines        1296     1345      +49     
==========================================
+ Hits         1231     1292      +61     
+ Misses         65       53      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🔭 Outside diff range comments (7)
airos/discovery.py (1)

32-42: Callback type widened but still scheduled as a coroutine — potential runtime TypeError

You changed callback to Callable[[dict[str, Any]], Any], but datagram_received schedules it with asyncio.create_task(self.callback(...)). If a caller passes a synchronous function (now allowed by the type), create_task will raise a TypeError at runtime.

Options:

  • Keep it strictly async: use Callable[[dict[str, Any]], Awaitable[None]].
  • Support both sync and async: annotate as Callable[[dict[str, Any]], Awaitable[None] | None] and only schedule if the result is awaitable.

Apply this diff to fix the type and intention:

-    def __init__(self, callback: Callable[[dict[str, Any]], Any]) -> None:
+    def __init__(self, callback: Callable[[dict[str, Any]], Awaitable[None] | None]) -> None:
         """Initialize AirOSDiscoveryProtocol.
 
         Args:
             callback: An asynchronous function to call when a device is discovered.
                       It should accept a dictionary containing device information.
 
         """
         self.callback = callback
         self.transport: asyncio.DatagramTransport | None = None

Additionally, update datagram_received to handle both sync and async callbacks:

# Add near imports:
import inspect
from typing import Awaitable, cast

# In datagram_received:
result = self.callback(parsed_data)
if inspect.isawaitable(result):
    asyncio.create_task(cast(Awaitable[None], result))  # noqa: RUF006

And import Awaitable (see snippet above).

.github/workflows/verify.yml (1)

41-45: “Ruff (with fix)” step doesn’t actually fix; commit step formats only.

  • The ruff check step runs without --fix despite the step name.
  • The commit step runs ruff format, which won’t address lint failures detected by ruff check.

Recommend enabling fixes in the check step and using the same command before committing.

Apply this diff to fix both steps:

-      - name: Ruff (with fix)
+      - name: Ruff (with fix)
         run: |
           . venv/bin/activate
-          ruff check airos/ tests/
+          ruff check --fix airos/ tests/
       - name: If needed, commit ruff changes to the pull request
         if: failure()
         run: |
           . venv/bin/activate
-          ruff format airos/ tests/
+          # Ensure we commit any automatic fixes produced above
+          ruff check --fix airos/ tests/
           git config --global user.name 'autoruff'
           git config --global user.email 'airos@users.noreply.github.com'
           git remote set-url origin https://x-access-token:${{ secrets.PAT_CT }}@github.com/$GITHUB_REPOSITORY
           git checkout $GITHUB_HEAD_REF
           git commit -am "fixup: ${GITHUB_REF##*/} Python code fixed using ruff"
           git push origin ${GITHUB_REF##*/}

Also applies to: 46-55

.pre-commit-config.yaml (1)

83-92: Ensure mypy runs holistically under pre-commit.

Two tweaks improve stability:

  • Use pass_filenames: false so mypy runs against the configured targets instead of a possibly incomplete file list.
  • types_or is unnecessary with files regex; if you keep it, it’s harmless.

Apply this diff:

       - id: mypy
         name: mypy
         entry: script/run-in-env.sh mypy
         language: script
         require_serial: true
-        types_or: [python, pyi]
+        pass_filenames: false
         files: ^(airos|tests|scripts)/.+\.(py|pyi)$

Optionally, pin the config explicitly:

-        entry: script/run-in-env.sh mypy
+        entry: script/run-in-env.sh mypy --config-file=pyproject.toml
airos/data.py (1)

63-66: Inconsistent MAC redaction and comment; leaks 2 bytes and mismatches mask regex.

  • The comment says “Redact only the first 6 hex characters,” but code redacts 4 bytes (8 hex chars) and keeps the last 2 bytes.
  • Using the static prefix 00:11:22:33: deviates from MAC_ADDRESS_MASK_REGEX (expects 00:00:00:00:XX:YY) and may confuse downstream expectations.

Recommend standardizing to a consistent mask that matches MAC_ADDRESS_MASK_REGEX and the docstring intention.

Apply this diff:

-                    # Redact only the first 6 hex characters of a MAC address
-                    redacted_d[k] = "00:11:22:33:" + v.replace("-", ":").upper()[-5:]
+                    # Redact the first 4 bytes and keep the last 2 bytes (matches MAC_ADDRESS_MASK_REGEX)
+                    normalized = v.replace("-", ":").upper()
+                    redacted_d[k] = "00:00:00:00:" + normalized[-5:]

If the goal is to keep only the vendor OUI (first 3 bytes) and redact the rest, invert the approach accordingly.

airos/airos8.py (2)

113-151: Cookie jar membership check is incorrect; may always re-add cookies.

morsel.key not in self.session.cookie_jar compares a string to cookie objects and will not behave as intended. Use an explicit any(...) over cookie keys, and avoid double-updating the jar.

Apply this diff:

-                    for _, morsel in response.cookies.items():
+                    for _, morsel in response.cookies.items():
                         # If the AIROS_ cookie was parsed but isn't automatically added to the jar, add it manually
-                        if (
-                            morsel.key.startswith("AIROS_")
-                            and morsel.key not in self.session.cookie_jar  # type: ignore[operator]
-                        ):
+                        if morsel.key.startswith("AIROS_") and not any(
+                            getattr(c, "key", None) == morsel.key for c in self.session.cookie_jar
+                        ):
@@
-                            self.session.cookie_jar.update_cookies(
-                                {
-                                    morsel.key: morsel.output(header="")[
-                                        len(morsel.key) + 1 :
-                                    ]
-                                    .split(";")[0]
-                                    .strip()
-                                },
-                                response.url,
-                            )
+                            self.session.cookie_jar.update_cookies(
+                                {morsel.key: morsel.value}, response.url
+                            )
@@
-                            # Let's try the direct update of the key-value
-                            self.session.cookie_jar.update_cookies(
-                                {morsel.key: morsel.value}
-                            )
+                            # Ensure only one update path is used

196-204: Mutable default argument in derived_data can leak state across calls.

Using {} as a default mutates the same dict on each invocation.

Apply this diff:

-    def derived_data(self, response: dict[str, Any] = {}) -> dict[str, Any]:
+    def derived_data(self, response: dict[str, Any] | None = None) -> dict[str, Any]:
         """Add derived data to the device response."""
-        derived: dict[str, Any] = {
+        if response is None:
+            response = {}
+        derived: dict[str, Any] = {
tests/test_discovery.py (1)

315-319: Patch asyncio.sleep in the module under test

airos_discover_devices calls asyncio.sleep via the airos.discovery module import. Patch airos.discovery.asyncio.sleep for reliability.

-        patch("asyncio.sleep", new=AsyncMock(side_effect=asyncio.CancelledError)),
+        patch("airos.discovery.asyncio.sleep", new=AsyncMock(side_effect=asyncio.CancelledError)),
🧹 Nitpick comments (14)
script/mashumaro-step-debug.py (1)

21-26: Avoid raising bare Exception for CLI argument errors

Raising a generic Exception for a missing CLI argument is noisy and unusual for CLI tools. Prefer SystemExit with a non-zero code (and log at error level) or argparse for better UX.

Apply this diff to improve the error path:

 def main() -> None:
     """Debug data."""
     if len(sys.argv) <= 1:
-        _LOGGER.info("Use with file to check")
-        raise Exception("File to check not provided.")
+        _LOGGER.info("Use with file to check")
+        _LOGGER.error("File to check not provided.")
+        raise SystemExit(2)
pyproject.toml (1)

399-404: Tighten mypy overrides: remove redundant setting

ignore_missing_imports is already true globally (Line 381), so repeating it in the tests.* override is redundant. Keep only the differences to reduce config drift.

Apply this diff:

 [[tool.mypy.overrides]]
 module = "tests.*"
-ignore_missing_imports = true # You'll likely need this for test-only dependencies
 disallow_untyped_decorators = false # The fix for your current errors
 check_untyped_defs = false 
.github/workflows/verify.yml (1)

121-129: Nit: Dropping the diagnostic pip list | grep.

The pip list | grep -i mypy line is non-deterministic and noisy; CI logs shouldn’t depend on installed package names.

Apply this diff to remove it:

       - name: Run mypy
         run: |
           . venv/bin/activate
-          pip list | grep -i mypy
           mypy airos/
script/run-in-env.sh (2)

2-2: Add pipefail to strict mode.

Strengthen error handling by including pipefail to avoid masked failures in pipelines.

Apply this diff:

-set -eu
+set -euo pipefail

24-29: Optional: Prefer .venv over venv to reduce accidental pollution.

Ordering .venv first is a common convention and avoids accidentally sourcing a sibling venv at the wrong level.

Apply this diff:

-  for venv in venv .venv .; do
+  for venv in .venv venv .; do
airos/data.py (2)

69-74: IPv6 addresses get redacted to an IPv4 literal.

For ip6addr and any IPv6 strings, consider redacting to a valid IPv6 dummy, e.g., ::3, to preserve type shape.

Apply this diff to distinguish families:

-                elif isinstance(v, str) and is_ip_address(v):
-                    # Redact to a dummy local IP address
-                    redacted_d[k] = "127.0.0.3"
+                elif isinstance(v, str) and is_ip_address(v):
+                    # Redact to a dummy local address (preserve IP family)
+                    redacted_d[k] = "127.0.0.3" if "." in v else "::3"

And for lists:

-                    redacted_d[k] = ["127.0.0.3"]  # type: ignore[assignment]
+                    redacted_d[k] = ["127.0.0.3" if "." in v[0] else "::3"]  # type: ignore[assignment]

59-85: Avoid type: ignore by casting the result once.

The type ignores on assignments can be dropped by declaring redacted_d with Any-valued items.

Apply this diff:

-        redacted_d = {}
+        redacted_d: dict[str, Any] = {}

Then remove the three “# type: ignore[assignment]” comments in this block.

airos/airos8.py (1)

221-247: Defensive typing and key access in derived_data.

  • addresses = {} is untyped.
  • Accesses like interface["enabled"] assume presence; if malformed, this becomes KeyError before deserialization handles it.

Apply this diff:

-        addresses = {}
+        addresses: dict[str, str] = {}
@@
-        for interface in interfaces:
-            if interface["enabled"]:  # Only consider if enabled
-                addresses[interface["ifname"]] = interface["hwaddr"]
+        for interface in interfaces:
+            if not isinstance(interface, dict):
+                continue
+            if interface.get("enabled"):
+                ifname = interface.get("ifname")
+                hwaddr = interface.get("hwaddr")
+                if isinstance(ifname, str) and isinstance(hwaddr, str):
+                    addresses[ifname] = hwaddr
script/generate_ha_fixture.py (3)

45-46: Open files with explicit encoding to avoid platform-dependent behavior

Use utf-8 for both reading and writing to prevent surprises on systems with different default encodings.

-                with open(base_fixture_path) as source:
+                with open(base_fixture_path, encoding="utf-8") as source:
                     source_data = json.loads(source.read())
@@
-                with open(new_fixture_path, "w") as new:
+                with open(new_fixture_path, "w", encoding="utf-8") as new:
                     json.dump(new_data.to_dict(), new, indent=2, sort_keys=True)

Also applies to: 51-52


32-34: Guard against missing userdata directory

os.listdir(userdata_dir) will raise if the directory doesn’t exist. Add a guard and bail early with a helpful log.

     # Iterate over all files in the userdata_dir
-    for filename in os.listdir(userdata_dir):
+    if not os.path.isdir(userdata_dir):
+        _LOGGER.warning("Userdata directory '%s' not found; skipping.", userdata_dir)
+        return
+    for filename in os.listdir(userdata_dir):

56-59: Catching broad Exception hides actionable error types

Catching Exception swallows specific errors (e.g., AirOSDataMissingError, AirOSKeyDataMissingError, KeyError). Prefer targeted exception handling with clear messages and re-raise unexpected ones.

-            except json.JSONDecodeError:
+            except json.JSONDecodeError:
                 _LOGGER.error("Skipping '%s': Not a valid JSON file.", filename)
-            except Exception as e:
-                _LOGGER.error("Error processing '%s': %s", filename, e)
+            except (KeyError, ValueError) as e:
+                _LOGGER.error("Skipping '%s': Invalid content: %s", filename, e)
+            except Exception:
+                _LOGGER.exception("Unexpected error processing '%s'", filename)

If you want to be even more precise, import and handle airos.exceptions.AirOSDataMissingError and AirOSKeyDataMissingError explicitly here.

tests/test_stations.py (2)

105-107: Test name/docstring mismatch with behavior

This test asserts behavior for a non-200 response, not a MissingField. Rename and clarify the docstring.

-async def test_status_logs_exception_on_missing_field(
+async def test_status_logs_error_on_non_200(
     mock_logger: MagicMock, airos_device: AirOS
 ) -> None:
-    """Test that the status method correctly logs a full exception when it encounters a MissingField during deserialization."""
+    """Test that the status method logs an error on non-200 status responses."""

243-252: Comment says “returns None” but code asserts False

The login() path returns False when CSRF header is missing. Update the comment for accuracy.

-    # Test case 3: Login successful, returns None due to missing headers
+    # Test case 3: Login successful, but returns False due to missing CSRF header
tests/test_discovery.py (1)

230-237: Tighten protocol factory callback typing

Match the protocol’s callback signature for better type safety.

-def mock_protocol_factory(callback: Callable[[Any], None]) -> MagicMock:
+def mock_protocol_factory(callback: Callable[[dict[str, Any]], Any]) -> MagicMock:
     def inner_callback(device_info: dict[str, Any]) -> None:
         """Inner callback to process discovered device info."""
         mac_address = device_info.get("mac_address")
         if mac_address:
             discovered_devices[mac_address] = device_info
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2cced00 and 7cbe4bc.

📒 Files selected for processing (15)
  • .github/workflows/verify.yml (1 hunks)
  • .pre-commit-config.yaml (1 hunks)
  • airos/airos8.py (8 hunks)
  • airos/data.py (3 hunks)
  • airos/discovery.py (2 hunks)
  • pyproject.toml (2 hunks)
  • requirements-test.txt (1 hunks)
  • script/generate_ha_fixture.py (2 hunks)
  • script/mashumaro-step-debug.py (1 hunks)
  • script/run-in-env.sh (1 hunks)
  • tests/conftest.py (3 hunks)
  • tests/test_airos8.py (14 hunks)
  • tests/test_data.py (1 hunks)
  • tests/test_discovery.py (20 hunks)
  • tests/test_stations.py (6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
tests/conftest.py (2)
airos/airos8.py (1)
  • AirOS (26-385)
airos/discovery.py (1)
  • AirOSDiscoveryProtocol (18-276)
tests/test_discovery.py (3)
tests/conftest.py (1)
  • mock_datagram_endpoint (30-53)
airos/discovery.py (3)
  • AirOSDiscoveryProtocol (18-276)
  • airos_discover_devices (279-327)
  • parse_airos_packet (88-276)
airos/exceptions.py (2)
  • AirOSEndpointError (36-37)
  • AirOSListenerError (32-33)
script/generate_ha_fixture.py (3)
airos/airos8.py (2)
  • AirOS (26-385)
  • derived_data (196-247)
airos/data.py (1)
  • AirOS8Data (544-561)
airos/exceptions.py (1)
  • AirOSDataMissingError (16-17)
airos/airos8.py (1)
airos/exceptions.py (1)
  • AirOSDeviceConnectionError (24-25)
tests/test_stations.py (4)
airos/airos8.py (3)
  • AirOS (26-385)
  • status (249-308)
  • login (77-194)
airos/data.py (1)
  • Wireless (421-469)
airos/exceptions.py (5)
  • AirOSConnectionAuthenticationError (12-13)
  • AirOSConnectionSetupError (8-9)
  • AirOSDataMissingError (16-17)
  • AirOSDeviceConnectionError (24-25)
  • AirOSKeyDataMissingError (20-21)
tests/conftest.py (2)
  • airos_device (21-26)
  • base_url (15-17)
tests/test_airos8.py (3)
airos/airos8.py (5)
  • AirOS (26-385)
  • status (249-308)
  • derived_data (196-247)
  • stakick (310-346)
  • provmode (348-385)
tests/conftest.py (1)
  • airos_device (21-26)
airos/exceptions.py (3)
  • AirOSDeviceConnectionError (24-25)
  • AirOSKeyDataMissingError (20-21)
  • AirOSDataMissingError (16-17)
🔇 Additional comments (25)
requirements-test.txt (1)

10-11: Verified stub availability and CI installation

Both types-aiofiles==24.1.0.20250809 and mypy==1.17.1 exist on PyPI, and the mypy job in .github/workflows/verify.yml installs requirements-test.txt before running the checks. No further action needed.

script/mashumaro-step-debug.py (1)

21-21: LGTM: explicit return type annotation

Annotating main() with -> None is consistent with the project’s typing push. No functional change introduced.

tests/test_data.py (1)

10-10: LGTM: explicit return type for async test

The annotation -> None is consistent with the mypy settings for tests and improves clarity without altering behavior.

pyproject.toml (1)

7-7: Version bump looks fine

Pre-release 0.2.9a0 aligns with the mypy rollout and test updates. Ensure downstream consumers are OK with pre-release semantics if they pin with ~=.

Please confirm changelog and release notes reflect the typing-related changes to avoid surprises for integrators.

.github/workflows/verify.yml (1)

137-137: Good: Coverage waits on mypy now.

Making coverage depend on mypy aligns quality gates and prevents publishing/reporting with type errors.

airos/data.py (1)

42-44: Signature now enforces dict input — verify all callers.

The early non-dict guard is gone. If any existing caller passed non-dicts (e.g., lists or None), this will raise. Based on usages in airos/airos8.py, it’s fine, but please confirm broader call sites.

Would you like a quick repo scan script to detect non-dict calls to redact_data_smart?

airos/airos8.py (3)

57-57: Good: Publicly typed CSRF token.

Explicit typing of current_csrf_token clarifies header handling across methods.


249-306: Status error handling is solid; redaction on deserialization failures is a good addition.

Consistent use of AirOS exceptions and redact_data_smart in the error paths improves resiliency and PII safety.


310-344: stakick typing and error handling improvements look good.

  • Type change to Optional[str] with explicit error on missing MAC is clearer.
  • Consistent exception classes across aiohttp paths.
script/generate_ha_fixture.py (2)

18-19: Imports align with the refactor to airos.data

Switching AirOSData to the AirOS8Data alias from airos.data is consistent with the PR’s typing changes. No issues.


22-22: Explicit return type is good

Annotating generate_airos_fixtures with -> None improves clarity and mypy signal. No issues.

tests/conftest.py (3)

15-16: Fixture typing: looks good

Explicit return type for base_url aids mypy and readability.


21-26: Async fixture typing: looks good

AsyncGenerator[AirOS, None] for airos_device is precise and aligns with your broader mypy enforcement.


30-32: Datagram endpoint fixture typing and setup are appropriate

The Generator tuple type is correct. Pre-seeding mocks before patching avoids scoping mishaps. All good.

tests/test_stations.py (3)

25-36: Fixture reader improvements are solid

Returning Any with explicit error handling via pytest.fail gives clearer failures and works well with mypy.


157-193: Nice addition: assert wireless.mode is present

Ensures Wireless.mode deserializes as expected; aligns with data model updates (WirelessMode | None).


226-274: Patch boolean attribute with new=True, not return_value

All occurrences of patching the boolean attribute _use_json_for_login_post are currently using return_value=True, which assigns a MagicMock instead of a plain boolean. Update each call to use new=True:

Locations to update in tests/test_stations.py:

  • Line 229
  • Line 237
  • Line 248
  • Line 258
  • Line 269

Replacements (example diff):

-with (
-    patch.object(airos_device.session, "post", return_value=mock_login_response),
-    patch.object(airos_device, "_use_json_for_login_post", return_value=True),
-):
+with (
+    patch.object(airos_device.session, "post", return_value=mock_login_response),
+    patch.object(airos_device, "_use_json_for_login_post", new=True),
+):

Apply this change across all five test blocks.

Likely an incorrect or invalid review comment.

tests/test_discovery.py (4)

4-4: Correct: use collections.abc.Callable

This aligns with modern typing practices and avoids typing.Callable for runtime use.


6-7: Good: import socket and cast for tighter assertions

Using socket for setsockopt assertions and cast for MagicMock close() improves test rigor and typing.


26-31: Helper uses asyncio.to_thread correctly

Reading binary fixtures via to_thread is appropriate; added docstring and explicit typing improve clarity. Good.


163-177: connection_made test is strong

Asserting socket options via get_extra_info("socket") mirrors the production code and guards regressions. Nicely done.

tests/test_airos8.py (4)

17-33: Login no-CSRF behavior asserted correctly

Asserting False return instead of None is consistent with the updated login() semantics.


55-68: Non-200 status path validated

Ensures AirOSDeviceConnectionError on HTTP errors. Good negative-path coverage.


106-125: derived_data fallback coverage is good

Covers absence of preferred interfaces and validates fallback to first enabled interface.


231-262: Cause-chaining asserted for MissingField

Verifying the original MissingField as the exception cause is great for debugging fidelity; logging assertion is precise.

@sonarqubecloud
Copy link

@CoMPaTech CoMPaTech merged commit df44ddf into main Aug 12, 2025
13 of 14 checks passed
@CoMPaTech CoMPaTech deleted the upd branch August 12, 2025 18:42
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7cbe4bc and 58d50bf.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • pyproject.toml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • pyproject.toml
⏰ 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 (1)
CHANGELOG.md (1)

5-6: Release entry added with correct versioning and date — looks good.
Consistent date format and version bracket syntax align with prior entries.


All notable changes to this project will be documented in this file.

## [0.2.9] - 2025-08-12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Add/verify compare-link reference for [0.2.9] at the bottom of the file.
Keeps bracketed version tags clickable and consistent with common changelog practice.

Run this to check if a compare link is present; if missing, add one like “[0.2.9]: https://github.com///compare/v0.2.8...v0.2.9”:


🏁 Script executed:

#!/bin/bash
# Verify that a compare link for [0.2.9] exists somewhere in the repo (typically at the end of CHANGELOG.md)
rg -n $'^\\[0\\.2\\.9\\]:' || echo "Missing [0.2.9] compare link reference"

Length of output: 110


I’ll fetch the repo’s origin URL and the existing [0.2.8] link so we can craft the exact compare URL for [0.2.9].


🏁 Script executed:

#!/bin/bash
set -e
# Get the GitHub org/repo from origin
remote_url=$(git remote get-url origin)
echo "Remote URL: $remote_url"

# Show the existing [0.2.8] compare link
echo "Existing [0.2.8] compare link reference:"
rg -n '^\[0\.2\.8\]:' CHANGELOG.md || echo "Missing [0.2.8] compare link reference"

Length of output: 436


Add compare-link reference for [0.2.9] in CHANGELOG.md

Please append the following line at the bottom of CHANGELOG.md to enable clickable version tags:

+[0.2.9]: https://github.com/CoMPaTech/python-airos/compare/v0.2.8...v0.2.9

• File: CHANGELOG.md
• Location: end of file

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## [0.2.9] - 2025-08-12
[0.2.9]: https://github.com/CoMPaTech/python-airos/compare/v0.2.8...v0.2.9
🤖 Prompt for AI Agents
In CHANGELOG.md around line 5 (end of file), add a Markdown reference link for
the new release tag [0.2.9] so the version header becomes clickable; append a
single line of the form "[0.2.9]:
https://github.com/<owner>/<repo>/compare/v0.2.8...v0.2.9" (replace
<owner>/<repo> and previous tag as needed) as the last line of the file.

Comment on lines +7 to +16
### Changed

- Bug Fixes
- More consistent error handling across login, status, stakick, and provmode; login now returns False when an auth token is missing. Improved discovery transport setup and resilience.
- Refactor
- Tightened type hints and clarified method signatures for predictable return types and safer usage.
- Tests
- Substantially expanded coverage, especially for discovery edge cases and error paths.
- Chores
- Enabled type checking in CI and gated coverage on it; added pre-commit hook and supporting environment script; updated test dependencies.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Restructure section to follow Keep a Changelog categories (avoid nesting under “Changed”).
Use top-level sections like Fixed/Changed/Tests/Chore; split combined items; fix “provmode” naming; format identifiers as code.

Apply this diff:

-### Changed
-
-- Bug Fixes
-  - More consistent error handling across login, status, stakick, and provmode; login now returns False when an auth token is missing. Improved discovery transport setup and resilience.
-- Refactor
-  - Tightened type hints and clarified method signatures for predictable return types and safer usage.
-- Tests
-  - Substantially expanded coverage, especially for discovery edge cases and error paths.
-- Chores
-  - Enabled type checking in CI and gated coverage on it; added pre-commit hook and supporting environment script; updated test dependencies.
+### Fixed
+- More consistent error handling across `login`, `status`, `stakick`, and provisioning mode; `login` now returns `False` when an auth token is missing.
+- Improved discovery transport setup and resilience.
+
+### Changed
+- Tightened type hints and clarified method signatures for predictable return types and safer usage.
+
+### Tests
+- Substantially expanded coverage, especially for discovery edge cases and error paths.
+
+### Chore
+- Enabled type checking in CI and gated coverage on it.
+- Added pre-commit hook and supporting environment script.
+- Updated test dependencies.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
### Changed
- Bug Fixes
- More consistent error handling across login, status, stakick, and provmode; login now returns False when an auth token is missing. Improved discovery transport setup and resilience.
- Refactor
- Tightened type hints and clarified method signatures for predictable return types and safer usage.
- Tests
- Substantially expanded coverage, especially for discovery edge cases and error paths.
- Chores
- Enabled type checking in CI and gated coverage on it; added pre-commit hook and supporting environment script; updated test dependencies.
### Fixed
- More consistent error handling across `login`, `status`, `stakick`, and provisioning mode; `login` now returns `False` when an auth token is missing.
- Improved discovery transport setup and resilience.
### Changed
- Tightened type hints and clarified method signatures for predictable return types and safer usage.
### Tests
- Substantially expanded coverage, especially for discovery edge cases and error paths.
### Chore
- Enabled type checking in CI and gated coverage on it.
- Added pre-commit hook and supporting environment script.
- Updated test dependencies.
🤖 Prompt for AI Agents
In CHANGELOG.md around lines 7 to 16, the release notes currently nest multiple
items under a single "Changed" header and use inconsistent naming/formatting
(e.g., "provmode" and plain identifiers); restructure the section to follow Keep
a Changelog by creating top-level headers such as "Fixed", "Changed", "Tests",
and "Chores", split the combined bullet points into these appropriate top-level
sections, correct the name "provmode" to `prov-mode`, and format identifiers
like `login`, `status`, `stakick`, `auth token`, and `prov-mode` as inline code;
ensure each bullet is placed under the correct header and remove the nested "Bug
Fixes" subheading so entries are directly under their respective top-level
categories.

This was referenced Aug 13, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant