Fix age field in Remote for nanostation 8.7.11 #52
Conversation
WalkthroughThis update introduces a new base dataclass, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script
participant FileSystem
participant AirOS
participant JSON
User->>Script: Run script
Script->>FileSystem: List JSON files in userdata/
loop For each JSON file (excluding "mocked")
Script->>FileSystem: Read JSON file
Script->>JSON: Parse JSON
Script->>AirOS: Call derived_data()
AirOS-->>Script: Return derived data
Script->>JSON: Convert to AirOSData
Script->>FileSystem: Write fixture to fixtures/
Script->>User: Log progress
end
Script->>User: Log completion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #52 +/- ##
=======================================
Coverage 95.70% 95.71%
=======================================
Files 9 9
Lines 1281 1283 +2
=======================================
+ Hits 1226 1228 +2
Misses 55 55 ☔ 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: 2
🔭 Outside diff range comments (1)
airos/data.py (1)
185-190: Potential deserialization failure on unknown Host.netrolepre_deserialize deletes an unknown netrole, but Host.netrole is non-optional. If a device returns a new/unknown value, mashumaro will error due to a missing required field. Recommend making it optional with a sane default to degrade gracefully.
@dataclass -class Host(AirOSDataClass): +class Host(AirOSDataClass): @@ - netrole: NetRole + netrole: NetRole | None = NoneAlternatively, adapt the sanitizer to set None instead of deleting the key when the target field is optional.
🧹 Nitpick comments (3)
pyproject.toml (1)
7-7: Pre-release version vs Production/Stable classifierYou bumped to a pre-release (0.2.7a0) while classifiers still say "Development Status :: 5 - Production/Stable". If this is an alpha, consider either using a stable version (e.g., 0.2.7) or downgrading the classifier to reflect pre-release status. This avoids confusing downstream consumers.
Also applies to: 13-18
script/generate_ha_fixture.py (2)
43-52: File encodings and context managers — minor hardeningSpecify UTF-8 explicitly and prefer json.load. Same for writes.
- try: - with open(base_fixture_path) as source: - source_data = json.loads(source.read()) + try: + with open(base_fixture_path, "r", encoding="utf-8") as source: + source_data = json.load(source) @@ - with open(new_fixture_path, "w") as new: - json.dump(new_data.to_dict(), new, indent=2, sort_keys=True) + with open(new_fixture_path, "w", encoding="utf-8") as new: + json.dump(new_data.to_dict(), new, indent=2, sort_keys=True)
55-58: Include stack traces for easier debuggingUse _LOGGER.exception to include traceback on unexpected failures.
- 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 json.JSONDecodeError: + _LOGGER.error("Skipping '%s': Not a valid JSON file.", filename) + except Exception: + _LOGGER.exception("Error processing '%s'", filename)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
airos/data.py(19 hunks)pyproject.toml(1 hunks)script/generate_ha_fixture.py(2 hunks)
🔇 Additional comments (8)
airos/data.py (7)
95-99: Good foundation: shared base for mashumaro dataclassesIntroducing AirOSDataClass centralizes the mashumaro mixin and keeps leaf classes clean. No issues spotted.
158-166: Consistent inheritance migration to AirOSDataClassAll dataclasses now consistently inherit from AirOSDataClass. This unifies serialization behavior and reduces boilerplate. Looks good.
Also applies to: 193-205, 214-223, 230-244, 247-256, 259-266, 268-280, 283-293, 296-307, 310-315, 318-360, 369-381, 383-419, 421-471, 474-492, 495-503, 506-517, 520-526, 529-542, 545-561
169-185: Host: relax numeric types for load/cpu — LGTMChanging loadavg and cpuload to float | int | None will deserialise more real-world values. No functional issues.
Also applies to: 178-183
299-306: GPSData numeric unions — LGTMAllowing lat/lon/dop/alt to be float | int | None improves robustness against varying firmware outputs. No concerns.
523-525: GPSMain numeric unions — LGTMlat/lon relaxed to float | int | None aligns with GPSData. Looks correct.
326-327: Remote.cpuload relaxed to float | int | None — LGTMMatches Host behavior and real-world variability.
359-360: Remote.age optionality verified – safe to approve
- airos/data.py:359 –
age: int | None = None- No usages of
Remote.agefound elsewhere in the codebaseMarking
ageas optional aligns with NanoStation 8.7.11 and introduces no downstream impact.script/generate_ha_fixture.py (1)
61-65: Main guard — LGTMLogging config and entry point are fine. No changes needed.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
script/mashumaro-step-debug.py (5)
1-1: Add a shebang to make the script directly executable.Helps running via ./script/mashumaro-step-debug.py.
+#!/usr/bin/env python3 """Debug userdata json to see where things don't add up."""
12-13: Remove duplicate sys.path manipulation and ensure import precedence.The project_root_dir is added twice; also prefer insert(0) to ensure it precedes site-packages.
- sys.path.append(project_root_dir) + sys.path.insert(0, project_root_dir)- current_script_dir = os.path.dirname(os.path.abspath(__file__)) - project_root_dir = os.path.abspath(os.path.join(current_script_dir, os.pardir)) - - if project_root_dir not in sys.path: - sys.path.append(project_root_dir) + # (duplicate sys.path manipulation removed)Also applies to: 27-32
41-45: Use the preprocessed Wireless data for subsequent steps.Currently the result is computed but ignored; using it ensures consistent deserialization if pre-processing mutates values (e.g., enum normalization).
- wireless_data_prepped = Wireless.__pre_deserialize__(wireless_data.copy()) # noqa: F841 + wireless_data = Wireless.__pre_deserialize__(wireless_data.copy())
49-59: Avoid accumulating unused objects and suppressing F841; just construct to validate.This reduces memory for large sta lists and removes the need for noqa pragmas.
- station_obj_list = [] for i, station_data in enumerate(station_list_data): @@ - remote_obj = Remote.from_dict(remote_data) # noqa: F841 + Remote.from_dict(remote_data) @@ - station_obj = Station.from_dict(station_data) - station_obj_list.append(station_obj) # noqa: F841 + Station.from_dict(station_data)
53-56: Downgrade verbose/PII-prone logging to DEBUG and avoid dumping full payloads at INFO.Remote data may include identifiers (MAC/IP). Keep it for debugging but not at INFO.
- _LOGGER.info(" -> Checking Remote object at index %s...", i) - _LOGGER.info("Remote data = %s", remote_data) + _LOGGER.info(" -> Checking Remote object at index %s...", i) + _LOGGER.debug("Remote data = %s", remote_data)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pyproject.toml(1 hunks)script/mashumaro-step-debug.py(1 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: Process test coverage
🔇 Additional comments (1)
script/mashumaro-step-debug.py (1)
39-39: No concerns: project targets Python ≥ 3.13—builtin generics are supported
Thepyproject.tomldeclaresrequires-python = ">=3.13", so usingdict[str, Any](added in 3.9) is fully supported. No changes needed here.
1 participant
Summary by CodeRabbit
Refactor
Style
Chores