Skip to content

Commit 0d1f831

Browse files
committed
CRAI suggestions and feedback
1 parent ac4418c commit 0d1f831

File tree

8 files changed

+50
-79
lines changed

8 files changed

+50
-79
lines changed

airos/data.py

Lines changed: 24 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
MAC_ADDRESS_MASK_REGEX = re.compile(r"^(00:){4}[0-9a-fA-F]{2}[:-][0-9a-fA-F]{2}$")
2020

2121

22+
# Helper functions
2223
def is_mac_address(value: str) -> bool:
2324
"""Check if a string is a valid MAC address."""
2425
return bool(MAC_ADDRESS_REGEX.match(value))
@@ -38,27 +39,6 @@ def is_ip_address(value: str) -> bool:
3839
return False
3940

4041

41-
def _check_and_log_unknown_enum_value(
42-
data_dict: dict[str, Any],
43-
key: str,
44-
enum_class: type[Enum],
45-
dataclass_name: str,
46-
field_name: str,
47-
) -> None:
48-
"""Clean unsupported parameters with logging."""
49-
value = data_dict.get(key)
50-
if value is not None and isinstance(value, str):
51-
if value not in [e.value for e in enum_class]:
52-
logger.warning(
53-
"Unknown value '%s' for %s.%s. Please report at "
54-
"https://github.com/CoMPaTech/python-airos/issues so we can add support.",
55-
value,
56-
dataclass_name,
57-
field_name,
58-
)
59-
del data_dict[key]
60-
61-
6242
def redact_data_smart(data: dict) -> dict:
6343
"""Recursively redacts sensitive keys in a dictionary."""
6444
sensitive_keys = {
@@ -85,7 +65,7 @@ def _redact(d: dict):
8565
if k in sensitive_keys:
8666
if isinstance(v, str) and (is_mac_address(v) or is_mac_address_mask(v)):
8767
# Redact only the first 6 hex characters of a MAC address
88-
redacted_d[k] = "00:00:00:00:" + v.replace("-", ":").upper()[-5:]
68+
redacted_d[k] = "00:11:22:33:" + v.replace("-", ":").upper()[-5:]
8969
elif isinstance(v, str) and is_ip_address(v):
9070
# Redact to a dummy local IP address
9171
redacted_d[k] = "127.0.0.3"
@@ -109,46 +89,28 @@ def _redact(d: dict):
10989
return _redact(data)
11090

11191

112-
def _redact_ip_addresses(addresses: str | list[str]) -> str | list[str]:
113-
"""Redacts the first three octets of an IPv4 address."""
114-
if isinstance(addresses, str):
115-
addresses = [addresses]
92+
# Data class start
11693

117-
redacted_list = []
118-
for ip in addresses:
119-
try:
120-
parts = ip.split(".")
121-
if len(parts) == 4:
122-
# Keep the last octet, but replace the rest with a placeholder.
123-
redacted_list.append(f"127.0.0.{parts[3]}")
124-
else:
125-
# Handle non-standard IPs or IPv6 if it shows up here
126-
redacted_list.append("REDACTED")
127-
except (IndexError, ValueError):
128-
# In case the IP string is malformed
129-
redacted_list.append("REDACTED")
130-
131-
return redacted_list if isinstance(addresses, list) else redacted_list[0]
132-
133-
134-
def _redact_mac_addresses(macs: str | list[str]) -> str | list[str]:
135-
"""Redacts the first four octets of a MAC address."""
136-
if isinstance(macs, str):
137-
macs = [macs]
138-
139-
redacted_list = []
140-
for mac in macs:
141-
try:
142-
parts = mac.split(":")
143-
if len(parts) == 6:
144-
# Keep the last two octets, replace the rest with a placeholder
145-
redacted_list.append(f"00:11:22:33:{parts[4]}:{parts[5]}")
146-
else:
147-
redacted_list.append("REDACTED")
148-
except (IndexError, ValueError):
149-
redacted_list.append("REDACTED")
15094

151-
return redacted_list if isinstance(macs, list) else redacted_list[0]
95+
def _check_and_log_unknown_enum_value(
96+
data_dict: dict[str, Any],
97+
key: str,
98+
enum_class: type[Enum],
99+
dataclass_name: str,
100+
field_name: str,
101+
) -> None:
102+
"""Clean unsupported parameters with logging."""
103+
value = data_dict.get(key)
104+
if value is not None and isinstance(value, str):
105+
if value not in [e.value for e in enum_class]:
106+
logger.warning(
107+
"Unknown value '%s' for %s.%s. Please report at "
108+
"https://github.com/CoMPaTech/python-airos/issues so we can add support.",
109+
value,
110+
dataclass_name,
111+
field_name,
112+
)
113+
del data_dict[key]
152114

153115

154116
class IeeeMode(Enum):
@@ -328,8 +290,8 @@ class EthList:
328290
class GPSData:
329291
"""Leaf definition."""
330292

331-
lat: str | None = None
332-
lon: str | None = None
293+
lat: float | None = None
294+
lon: float | None = None
333295
fix: int | None = None
334296
sats: int | None = None # LiteAP GPS
335297
dim: int | None = None # LiteAP GPS

fixtures/airos_liteapgps_ap_ptmp_40mhz.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -535,8 +535,8 @@
535535
"dim": null,
536536
"dop": null,
537537
"fix": 0,
538-
"lat": "52.379894",
539-
"lon": "4.901608",
538+
"lat": 52.379894,
539+
"lon": 4.901608,
540540
"sats": null,
541541
"time_synced": null
542542
},
@@ -977,8 +977,8 @@
977977
"dim": null,
978978
"dop": null,
979979
"fix": 0,
980-
"lat": "52.379894",
981-
"lon": "4.901608",
980+
"lat": 52.379894,
981+
"lon": 4.901608,
982982
"sats": null,
983983
"time_synced": null
984984
},

fixtures/airos_loco5ac_ap-ptp.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,8 @@
524524
"dim": null,
525525
"dop": null,
526526
"fix": 0,
527-
"lat": "52.379894",
528-
"lon": "4.901608",
527+
"lat": 52.379894,
528+
"lon": 4.901608,
529529
"sats": null,
530530
"time_synced": null
531531
},

fixtures/airos_loco5ac_sta-ptp.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,8 @@
524524
"dim": null,
525525
"dop": null,
526526
"fix": 0,
527-
"lat": "52.379894",
528-
"lon": "4.901608",
527+
"lat": 52.379894,
528+
"lon": 4.901608,
529529
"sats": null,
530530
"time_synced": null
531531
},

fixtures/airos_mocked_sta-ptmp.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,8 @@
524524
"dim": null,
525525
"dop": null,
526526
"fix": 0,
527-
"lat": "52.379894",
528-
"lon": "4.901608",
527+
"lat": 52.379894,
528+
"lon": 4.901608,
529529
"sats": null,
530530
"time_synced": null
531531
},
@@ -954,8 +954,8 @@
954954
"dim": null,
955955
"dop": null,
956956
"fix": 0,
957-
"lat": "52.379894",
958-
"lon": "4.901608",
957+
"lat": 52.379894,
958+
"lon": 4.901608,
959959
"sats": null,
960960
"time_synced": null
961961
},

fixtures/airos_nanobeam5ac_sta_ptmp_40mhz.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,8 @@
549549
"dim": 3,
550550
"dop": 0.96,
551551
"fix": 1,
552-
"lat": "52.379894",
553-
"lon": "4.901608",
552+
"lat": 52.379894,
553+
"lon": 4.901608,
554554
"sats": 9,
555555
"time_synced": 0
556556
},

tests/test_discovery.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ async def _simulate_discovery():
246246

247247
mock_protocol_factory(MagicMock()).callback(parsed_data)
248248

249-
with patch("asyncio.sleep", new=AsyncMock()):
249+
with patch("airos.discovery.asyncio.sleep", new=AsyncMock()):
250250
discovery_task = asyncio.create_task(airos_discover_devices(timeout=1))
251251

252252
await _simulate_discovery()
@@ -263,7 +263,7 @@ async def test_async_discover_devices_no_devices(mock_datagram_endpoint):
263263
"""Test discovery returns an empty dict if no devices are found."""
264264
mock_transport, _ = mock_datagram_endpoint
265265

266-
with patch("asyncio.sleep", new=AsyncMock()):
266+
with patch("airos.discovery.asyncio.sleep", new=AsyncMock()):
267267
result = await airos_discover_devices(timeout=1)
268268

269269
assert result == {}

tests/test_stations.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,18 @@ async def test_status_logs_redacted_data_on_invalid_value(mock_logger, airos_dev
7676
logged_data = mock_logger.exception.call_args[0][1]
7777

7878
# Assert that the dictionary has been redacted
79+
assert "wireless" in logged_data
80+
assert "essid" in logged_data["wireless"]
7981
assert logged_data["wireless"]["essid"] == "REDACTED"
82+
assert "host" in logged_data
83+
assert "hostname" in logged_data["host"]
8084
assert logged_data["host"]["hostname"] == "REDACTED"
81-
assert logged_data["wireless"]["apmac"] == "00:00:00:00:89:AB"
85+
assert "apmac" in logged_data["wireless"]
86+
assert logged_data["wireless"]["apmac"] == "00:11:22:33:89:AB"
87+
assert "interfaces" in logged_data
88+
assert len(logged_data["interfaces"]) > 2
89+
assert "status" in logged_data["interfaces"][2]
90+
assert "ipaddr" in logged_data["interfaces"][2]["status"]
8291
assert logged_data["interfaces"][2]["status"]["ipaddr"] == "127.0.0.3"
8392

8493

0 commit comments

Comments
 (0)