Modelmap #112
Modelmap #112
Conversation
WalkthroughAdds SKU mapping for airOS devices via a new model map and integrates SKU resolution into derived data. Introduces an exception for ambiguous matches, extends Derived with a sku field, updates fixtures and tests, relaxes a fixture-filter rule, adjusts coverage pragmas/logging, and bumps version to 0.5.6. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Device
participant AirOSBase as AirOS Base
participant Mapper as UispAirOSProductMapper
participant Exceptions
Device->>AirOSBase: Provide status (includes devmodel)
AirOSBase->>Mapper: get_sku_by_devmodel(devmodel)
alt Exact match
Mapper-->>AirOSBase: SKU
AirOSBase->>AirOSBase: Set Derived.sku
else Unique partial match
Mapper-->>AirOSBase: SKU (from unique prefix)
AirOSBase->>AirOSBase: Set Derived.sku
else Multiple matches
Mapper-->>Exceptions: AirOSMultipleMatchesFoundException
AirOSBase->>AirOSBase: Handle exception (no SKU)
else No match
Mapper-->>AirOSBase: KeyError
AirOSBase->>AirOSBase: Handle missing SKU
end
AirOSBase-->>Device: Derived data (incl. sku when resolved)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 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)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
==========================================
+ Coverage 87.59% 89.57% +1.97%
==========================================
Files 7 8 +1
Lines 653 681 +28
==========================================
+ Hits 572 610 +38
+ Misses 81 71 -10 ☔ 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: 1
🧹 Nitpick comments (5)
airos/data.py (1)
67-79: Coverage pragmas on redaction and IP helpersPragmas are fine if branches are hard to hit in tests, but consider covering these paths (IPv4/IPv6 list/dict redaction and is_ip_address happy-path) instead of excluding from coverage. Improves safety around future changes.
Also applies to: 40-40
airos/base.py (1)
125-135: SKU resolution integration is reasonableGraceful handling: exact/partial → SKU, KeyError → "UNKNOWN", multiple matches → warn and "AMBIGUOUS". Works with tests/fixtures.
Optionally cache UispAirOSProductMapper on self (or module-level) to avoid rebuilding the reverse map each call to status().
tests/test_model_map.py (1)
38-49: Avoid hard-coding ambiguous match countThe number of LiteBeam variants may change as MODELS evolves. Compute expected_matches dynamically from MODELS to keep the test stable.
Apply this change:
- exception_message = str(excinfo.value) - expected_matches = 4 - - match = re.search(r"matched multiple \((\d+)\) products", exception_message) + from airos.model_map import MODELS + exception_message = str(excinfo.value) + expected_matches = sum( + 1 for name in MODELS.keys() if "litebeam" in name.lower() + ) + + match = re.search(r"matched multiple \((\d+)\) products", exception_message) assert match is not None actual_matches_int = int(match.group(1)) assert actual_matches_int == expected_matchesairos/model_map.py (2)
91-95: Normalize input to reduce false negativesTrim whitespace before matching; prevents misses like "NanoBeam 5AC " (trailing space).
def get_sku_by_devmodel(self, devmodel: str) -> str: """Retrieves the SKU for a given device model name.""" + devmodel = devmodel.strip() if devmodel in MODELS: return MODELS[devmodel]Also applies to: 102-115
123-129: Minor: clarify variable naming/commentsCode matches on known-name.endswith(input). The comment says “prefix match”, which is misleading. Consider renaming best_match_is_prefix → best_match_is_suffix (or adjust the comment) for readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
CHANGELOG.md(1 hunks)airos/base.py(3 hunks)airos/data.py(3 hunks)airos/exceptions.py(1 hunks)airos/helpers.py(1 hunks)airos/model_map.py(1 hunks)fixtures/airos_LiteBeam5AC_ap-ptp_30mhz.json(1 hunks)fixtures/airos_LiteBeam5AC_sta-ptp_30mhz.json(1 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/airos_liteapgps_ap_ptmp_40mhz.json(1 hunks)fixtures/airos_loco5ac_ap-ptp.json(1 hunks)fixtures/airos_loco5ac_sta-ptp.json(1 hunks)fixtures/airos_mocked_sta-ptmp.json(4 hunks)fixtures/airos_nanobeam5ac_sta_ptmp_40mhz.json(1 hunks)fixtures/airos_nanostation_ap-ptp_8718_missing_gps.json(1 hunks)fixtures/userdata/mocked_sta-ptmp.json(1 hunks)pyproject.toml(1 hunks)script/generate_ha_fixture.py(1 hunks)script/mashumaro-step-debug.py(1 hunks)tests/test_model_map.py(1 hunks)tests/test_stations.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
airos/helpers.py (1)
airos/exceptions.py (1)
AirOSKeyDataMissingError(20-21)
airos/base.py (3)
airos/exceptions.py (2)
AirOSMultipleMatchesFoundException(48-49)AirOSUrlNotFoundError(44-45)airos/model_map.py (2)
UispAirOSProductMapper(84-144)get_sku_by_devmodel(91-138)airos/data.py (2)
DerivedWirelessRole(153-157)DerivedWirelessMode(160-164)
tests/test_model_map.py (2)
airos/exceptions.py (1)
AirOSMultipleMatchesFoundException(48-49)airos/model_map.py (3)
UispAirOSProductMapper(84-144)get_sku_by_devmodel(91-138)get_devmodel_by_sku(140-144)
tests/test_stations.py (4)
tests/test_stations6.py (1)
test_ap_object(36-67)tests/conftest.py (2)
airos8_device(33-38)base_url(18-20)airos/airos8.py (1)
AirOS8(11-30)airos/base.py (1)
status(302-326)
script/mashumaro-step-debug.py (2)
airos/data.py (1)
AirOS8Data(696-713)airos/base.py (1)
derived_data(177-179)
airos/model_map.py (1)
airos/exceptions.py (1)
AirOSMultipleMatchesFoundException(48-49)
⏰ 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 (19)
pyproject.toml (1)
7-7: LGTM! Version bump aligns with the new SKU mapping feature.The minor version increment from 0.5.5 to 0.5.6 appropriately reflects the addition of new functionality (model-to-SKU mapping) without breaking changes.
airos/helpers.py (1)
61-65: LGTM! Appropriate use of coverage pragmas.The
pragma: no coverdirectives are correctly applied to defensive exception handlers that validate API response integrity. These branches are difficult to test without mocking corrupted responses and represent edge cases that don't merit coverage requirements.script/generate_ha_fixture.py (1)
37-38: LGTM! Filter logic correctly allows specific mocked fixture.The updated condition appropriately allows
mocked_sta-ptmp.jsonto be processed while skipping other mocked files. This aligns with the test fixture changes that introduce an "unexisting" model for testing edge cases in SKU mapping.fixtures/airos_NanoStation_M5_sta_v6.3.16.json (1)
13-13: LGTM! SKU field addition is consistent.The new
skufield with value"LocoM5"appropriately extends the derived data structure for SKU mapping support. The value appears consistent with the device model.fixtures/userdata/mocked_sta-ptmp.json (1)
1-1: LGTM! Test fixture appropriately covers edge cases.The changes to use "NanoStation 5AC loco unexisting" as the device model are intentional test data for validating SKU mapping behavior with unknown models. This aligns with the exception handling introduced for ambiguous or missing matches in the model mapper.
fixtures/airos_LiteBeam5AC_sta-ptp_30mhz.json (1)
20-20: LGTM! SKU field addition is consistent.The new
skufield with value"LBE-5AC-GEN2"appropriately extends the derived data structure. The SKU value aligns with the "LiteBeam 5AC" device model.fixtures/airos_liteapgps_ap_ptmp_40mhz.json (1)
20-20: LGTM! SKU field addition is consistent.The new
skufield with value"LAP-GPS"appropriately extends the derived data structure for the access point fixture. The SKU value aligns with the "LiteAP GPS" device model.fixtures/airos_loco5ac_sta-ptp.json (1)
20-20: LGTM! SKU field addition is consistent.The new
skufield with value"Loco5AC"appropriately extends the derived data structure for the station fixture. The SKU value aligns with the "NanoStation 5AC loco" device model.CHANGELOG.md (1)
5-9: LGTM!The changelog entry clearly documents the new SKU mapping feature with appropriate detail and follows the established format.
fixtures/airos_NanoBeam_5AC_ap-ptmp_v8.7.18.json (1)
20-20: LGTM!The SKU field addition is consistent with the feature design, and the value "NBE-5AC-GEN2" appropriately maps to the "NanoBeam 5AC" device model.
fixtures/airos_loco5ac_ap-ptp.json (1)
20-20: LGTM!The SKU field correctly maps "NanoStation 5AC loco" to "Loco5AC" and follows the consistent fixture update pattern.
fixtures/airos_LiteBeam5AC_ap-ptp_30mhz.json (1)
20-20: LGTM!The SKU field appropriately maps "LiteBeam 5AC" to "LBE-5AC-GEN2", maintaining consistency with the SKU mapping design.
script/mashumaro-step-debug.py (1)
131-133: LGTM!The removal of the
noqapragma is appropriate since the variable is now used in the SKU log statement. The debug output enhancement supports verification of the new SKU mapping feature.fixtures/airos_nanostation_ap-ptp_8718_missing_gps.json (1)
20-20: LGTM!The SKU mapping for "NanoStation 5AC loco" to "Loco5AC" is consistent with other fixtures using the same device model.
airos/exceptions.py (1)
47-49: LGTM!The new exception class follows existing naming conventions and provides clear intent for handling ambiguous device lookups during SKU mapping. The placement after
AirOSUrlNotFoundErrormaintains logical grouping.fixtures/airos_nanobeam5ac_sta_ptmp_40mhz.json (1)
20-20: LGTM!The SKU field correctly maps "NanoBeam 5AC" to "NBE-5AC-GEN2", consistent with other NanoBeam 5AC fixtures regardless of device role.
airos/data.py (1)
691-693: Derived.sku addition looks goodField is correctly typed and integrated with base.py population.
fixtures/airos_mocked_sta-ptmp.json (1)
16-22: Fixture updates align with SKU mapping fallback
- derived.mode/role/sku additions align with code (sku: "UNKNOWN").
- host.devmodel and remote.platform set to a non-mapped value ensure KeyError path → UNKNOWN, as intended.
If any consumer still asserts on previous devmodel/platform strings, update them accordingly.
Also applies to: 31-39, 43-43, 552-552, 982-982
tests/test_stations.py (1)
144-153: Parametrize SKU and assert propagationGood additions: include sku in params, pass through to test, and assert status.derived.sku. Matches fixtures and base logic.
Also applies to: 157-158, 183-184
|
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests