-
Notifications
You must be signed in to change notification settings - Fork 1
Discovery fixes #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Discovery fixes #34
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,17 @@ | |||||||||||||||||||||||||||||||||||||||||||
| from airos.exceptions import AirosDiscoveryError, AirosEndpointError | ||||||||||||||||||||||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| expected_parsed_data_for_mock_packet = { | ||||||||||||||||||||||||||||||||||||||||||||
| "ip_address": "192.168.1.3", # This comes from the host_ip passed to datagram_received | ||||||||||||||||||||||||||||||||||||||||||||
| "mac_address": "01:23:45:67:89:CD", # From 0x06 TLV | ||||||||||||||||||||||||||||||||||||||||||||
| "hostname": "name", # From 0x0B TLV | ||||||||||||||||||||||||||||||||||||||||||||
| "model": "NanoStation 5AC loco", # From 0x0C TLV | ||||||||||||||||||||||||||||||||||||||||||||
| "firmware_version": "WA.V8.7.17", # From 0x03 TLV | ||||||||||||||||||||||||||||||||||||||||||||
| "uptime_seconds": 3231, # From 0x0A TLV (0x0C9F in hex) | ||||||||||||||||||||||||||||||||||||||||||||
| "ssid": "DemoSSID", # From 0x0D TLV | ||||||||||||||||||||||||||||||||||||||||||||
| "full_model_name": "NanoStation 5AC loco", # From 0x14 TLV | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify uptime_seconds value consistency. There's a discrepancy in the 🏁 Script executed: #!/bin/bash
# Description: Check the actual uptime value in the binary fixture
# Expected: Verify which uptime value is correct by examining the fixture
# Look for the fixture file and examine its contents
fd -t f "airos_sta_discovery_packet.bin" fixtures/
if [ $? -eq 0 ]; then
echo "Found fixture file. Checking TLV 0x0A (uptime) content..."
# Use hexdump to examine the binary content
hexdump -C fixtures/airos_sta_discovery_packet.bin | grep -A5 -B5 "0a"
fiLength of output: 403 I couldn’t inspect the TLV parsing logic with 🏁 Script executed: #!/bin/bash
# 1) Find and show the parse_airos_packet function with context
rg -n "def parse_airos_packet" -tpy
rg -A20 -B5 "def parse_airos_packet" -tpy
# 2) Within that function (or nearby), look for handling of TLV type 0x0A / "uptime"
rg -n "0x0A" -C5 -tpy
rg -n "uptime" -C5 -tpyLength of output: 9942 Update expected The mock‐packet dict in tests/test_discovery.py
@@ Lines 12–21
- "uptime_seconds": 3231, # From 0x0A TLV (0x0C9F in hex)
+ "uptime_seconds": 265375, # From 0x0A TLV (0x00040C9F in hex)• Location: tests/test_discovery.py, around line 18. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| # Helper to load binary fixture | ||||||||||||||||||||||||||||||||||||||||||||
| async def _read_binary_fixture(fixture_name: str) -> bytes: | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -119,21 +130,48 @@ async def test_datagram_received_calls_callback(mock_airos_packet): | |||||||||||||||||||||||||||||||||||||||||||
| protocol = AirosDiscoveryProtocol(mock_callback) | ||||||||||||||||||||||||||||||||||||||||||||
| host_ip = "192.168.1.3" # Sender IP | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| with patch("asyncio.create_task") as mock_create_task: | ||||||||||||||||||||||||||||||||||||||||||||
| protocol.datagram_received(mock_airos_packet, (host_ip, DISCOVERY_PORT)) | ||||||||||||||||||||||||||||||||||||||||||||
| protocol.datagram_received(mock_airos_packet, (host_ip, DISCOVERY_PORT)) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| # Verify the task was created and get the coroutine | ||||||||||||||||||||||||||||||||||||||||||||
| mock_create_task.assert_called_once() | ||||||||||||||||||||||||||||||||||||||||||||
| task_coro = mock_create_task.call_args[0][0] | ||||||||||||||||||||||||||||||||||||||||||||
| # Assert that the mock_callback was called exactly once | ||||||||||||||||||||||||||||||||||||||||||||
| mock_callback.assert_called_once() | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| # Manually await the coroutine to test the callback | ||||||||||||||||||||||||||||||||||||||||||||
| await task_coro | ||||||||||||||||||||||||||||||||||||||||||||
| # Get the arguments it was called with | ||||||||||||||||||||||||||||||||||||||||||||
| args, kwargs = mock_callback.call_args | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| mock_callback.assert_called_once() | ||||||||||||||||||||||||||||||||||||||||||||
| called_args, _ = mock_callback.call_args | ||||||||||||||||||||||||||||||||||||||||||||
| parsed_data = called_args[0] | ||||||||||||||||||||||||||||||||||||||||||||
| assert parsed_data["ip_address"] == "192.168.1.3" | ||||||||||||||||||||||||||||||||||||||||||||
| assert parsed_data["mac_address"] == "01:23:45:67:89:CD" # Verify scrubbed MAC | ||||||||||||||||||||||||||||||||||||||||||||
| # Assert that the callback was called with a single argument (the parsed data) | ||||||||||||||||||||||||||||||||||||||||||||
| assert len(args) == 1 | ||||||||||||||||||||||||||||||||||||||||||||
| assert not kwargs # No keyword arguments expected | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| # Get the parsed data from the call | ||||||||||||||||||||||||||||||||||||||||||||
| actual_parsed_data = args[0] | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| # Assert that the parsed data matches the expected structure and values | ||||||||||||||||||||||||||||||||||||||||||||
| # We need to make sure the IP address in the expected data reflects the host_ip | ||||||||||||||||||||||||||||||||||||||||||||
| expected_parsed_data_with_current_ip = expected_parsed_data_for_mock_packet.copy() | ||||||||||||||||||||||||||||||||||||||||||||
| expected_parsed_data_with_current_ip["ip_address"] = host_ip | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| # For robust testing, you might want to only check the critical fields, | ||||||||||||||||||||||||||||||||||||||||||||
| # or ensure your `parse_airos_packet` is separately tested to be correct. | ||||||||||||||||||||||||||||||||||||||||||||
| # Here, we'll assert a subset of key fields for simplicity. | ||||||||||||||||||||||||||||||||||||||||||||
| assert ( | ||||||||||||||||||||||||||||||||||||||||||||
| actual_parsed_data.get("mac_address") | ||||||||||||||||||||||||||||||||||||||||||||
| == expected_parsed_data_with_current_ip["mac_address"] | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
| assert ( | ||||||||||||||||||||||||||||||||||||||||||||
| actual_parsed_data.get("hostname") | ||||||||||||||||||||||||||||||||||||||||||||
| == expected_parsed_data_with_current_ip["hostname"] | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
| assert ( | ||||||||||||||||||||||||||||||||||||||||||||
| actual_parsed_data.get("ip_address") | ||||||||||||||||||||||||||||||||||||||||||||
| == expected_parsed_data_with_current_ip["ip_address"] | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
| assert ( | ||||||||||||||||||||||||||||||||||||||||||||
| actual_parsed_data.get("model") == expected_parsed_data_with_current_ip["model"] | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
| assert ( | ||||||||||||||||||||||||||||||||||||||||||||
| actual_parsed_data.get("firmware_version") | ||||||||||||||||||||||||||||||||||||||||||||
| == expected_parsed_data_with_current_ip["firmware_version"] | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| @pytest.mark.asyncio | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Async callback not properly invoked.
The callback is documented as asynchronous (line 36) but is now called directly without awaiting, causing the coroutine to never execute. The pipeline failure confirms this with "RuntimeWarning: coroutine was never awaited."
Choose one of these solutions:
Option 1: Restore async task scheduling (recommended)
Option 2: Await the callback (blocks the listener)
Option 3: Make callback synchronous (breaking change)
The first option maintains the original responsive behavior while properly handling async callbacks.
📝 Committable suggestion
🧰 Tools
🪛 GitHub Actions: Latest commit
[warning] 62-62: RuntimeWarning: coroutine 'AsyncMockMixin._execute_mock_call' was never awaited. This warning indicates a coroutine was not awaited properly.
🤖 Prompt for AI Agents