Correct the one-controller claim: the drone takes over, it does not refuse
Tested on the aircraft. A second ARSDK handshake is accepted and telemetry is redirected to it; the first session's frames stop while it still reports connected = True. So the claim inherited from pyparrot's error text, which had reached our error messages, tool descriptions, simulator behaviour and a test name, was wrong in the most misleading direction: a refusal would be loud, and this is silent. The simulator now models the takeover by default; refusal stays available because a client must handle a non-zero status anyway.
This commit is contained in:
@@ -21,9 +21,14 @@ def already_connected(target: str) -> ToolError:
|
|||||||
|
|
||||||
|
|
||||||
def handshake_refused(status: int) -> ToolError:
|
def handshake_refused(status: int) -> ToolError:
|
||||||
|
# Tested 2026-10-02: a Bebop 2 does NOT refuse a second controller, it
|
||||||
|
# accepts it and redirects telemetry to it. So a non-zero status means
|
||||||
|
# something other than "already taken", and guessing at it would send the
|
||||||
|
# reader looking in the wrong place.
|
||||||
return ToolError(
|
return ToolError(
|
||||||
f"The drone refused the connection (status {status}). It serves one controller at a time, "
|
f"The drone refused the connection (status {status}). That is unusual: this aircraft accepts "
|
||||||
"so close FreeFlight on any phone or tablet that is holding the link."
|
"a second controller rather than refusing one, so the cause is more likely the drone still "
|
||||||
|
"booting, or a firmware that differs from 4.7.1. Wait a few seconds and try again."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+10
-3
@@ -184,7 +184,10 @@ class FakeBebop:
|
|||||||
host: str = "127.0.0.1"
|
host: str = "127.0.0.1"
|
||||||
discovery_port: int = 0 # 0 asks the OS, which is what parallel tests want
|
discovery_port: int = 0 # 0 asks the OS, which is what parallel tests want
|
||||||
c2d_port: int = 0
|
c2d_port: int = 0
|
||||||
single_controller: bool = True
|
# False by default because that is what the aircraft does: it accepts a
|
||||||
|
# second controller and redirects telemetry to it. Set True to exercise a
|
||||||
|
# client's handling of a refusal, which some other firmware may still give.
|
||||||
|
single_controller: bool = False
|
||||||
stream_hz: float = 5.0
|
stream_hz: float = 5.0
|
||||||
battery_start: int = 87
|
battery_start: int = 87
|
||||||
|
|
||||||
@@ -295,8 +298,12 @@ class FakeBebop:
|
|||||||
continue
|
continue
|
||||||
self.handshakes.append(request)
|
self.handshakes.append(request)
|
||||||
if self.single_controller and self.occupied:
|
if self.single_controller and self.occupied:
|
||||||
# What the aircraft does to a second controller, and the
|
# NOT what a real Bebop 2 does. Tested on the aircraft
|
||||||
# only way to test that path without two phones.
|
# 2026-10-02: it accepts a second controller and silently
|
||||||
|
# redirects telemetry to it, leaving the first starved but
|
||||||
|
# still believing it is connected. This refusal path is kept
|
||||||
|
# because a client must handle a non-zero status anyway, but
|
||||||
|
# the default models the takeover. See docs network notes.
|
||||||
conn.sendall(json.dumps({"status": 1}).encode() + b"\x00")
|
conn.sendall(json.dumps({"status": 1}).encode() + b"\x00")
|
||||||
continue
|
continue
|
||||||
self._d2c = (addr[0], int(request["d2c_port"]))
|
self._d2c = (addr[0], int(request["d2c_port"]))
|
||||||
|
|||||||
@@ -45,8 +45,14 @@ def register(mcp: FastMCP, settings: Settings) -> None:
|
|||||||
) -> ConnectionInfo:
|
) -> ConnectionInfo:
|
||||||
"""Open a session with the drone. Nothing else works until this succeeds.
|
"""Open a session with the drone. Nothing else works until this succeeds.
|
||||||
|
|
||||||
The drone serves one controller at a time, so close any phone app first.
|
|
||||||
This machine must already have joined the drone's own Wi-Fi network.
|
This machine must already have joined the drone's own Wi-Fi network.
|
||||||
|
|
||||||
|
Connecting while something else is already controlling the drone takes
|
||||||
|
the link over rather than failing: the aircraft accepts the newcomer and
|
||||||
|
stops sending telemetry to whoever had it, without telling them. So if a
|
||||||
|
phone app is flying it, connect here and the phone goes quiet. Watch
|
||||||
|
last_update_age rather than the connected flag to tell a live link from
|
||||||
|
one that has been taken.
|
||||||
"""
|
"""
|
||||||
state = app()
|
state = app()
|
||||||
async with state.lock:
|
async with state.lock:
|
||||||
|
|||||||
@@ -119,7 +119,14 @@ async def test_handshake_describes_this_controller(sim, session):
|
|||||||
assert request["d2c_port"] == session.d2c_port != 0
|
assert request["d2c_port"] == session.d2c_port != 0
|
||||||
|
|
||||||
|
|
||||||
async def test_second_controller_is_refused(sim, session):
|
async def test_second_controller_takes_the_link_over(sim, session):
|
||||||
|
"""The aircraft accepts a newcomer and stops talking to whoever had it.
|
||||||
|
|
||||||
|
Measured on the real drone 2026-10-02: the first session's frame count
|
||||||
|
froze while the second received, and the first went on reporting
|
||||||
|
connected = True. That silent failure is why a client should judge a link
|
||||||
|
by telemetry freshness rather than by its connected flag.
|
||||||
|
"""
|
||||||
other = DroneSession(
|
other = DroneSession(
|
||||||
"127.0.0.1",
|
"127.0.0.1",
|
||||||
discovery_port=sim.discovery_port,
|
discovery_port=sim.discovery_port,
|
||||||
@@ -128,9 +135,13 @@ async def test_second_controller_is_refused(sim, session):
|
|||||||
encoder=fake_encode,
|
encoder=fake_encode,
|
||||||
decoder=fake_decode,
|
decoder=fake_decode,
|
||||||
)
|
)
|
||||||
with pytest.raises(HandshakeError, match="one controller"):
|
await other.connect()
|
||||||
await other.connect()
|
try:
|
||||||
assert not other.connected
|
assert other.connected
|
||||||
|
# The loser keeps its socket and its optimism; only the data stops.
|
||||||
|
assert session.connected
|
||||||
|
finally:
|
||||||
|
await other.disconnect()
|
||||||
|
|
||||||
|
|
||||||
async def test_unreachable_address_fails_fast_rather_than_hanging():
|
async def test_unreachable_address_fails_fast_rather_than_hanging():
|
||||||
|
|||||||
@@ -55,3 +55,19 @@ def test_acknowledgement_is_decided_by_data_type_not_buffer():
|
|||||||
frame = Frame(DataType.DATA_WITH_ACK, buffer_id, 9, b"\x00\x05\x00\x00")
|
frame = Frame(DataType.DATA_WITH_ACK, buffer_id, 9, b"\x00\x05\x00\x00")
|
||||||
assert Frame.decode_all(frame.encode())[0].buffer_id == buffer_id
|
assert Frame.decode_all(frame.encode())[0].buffer_id == buffer_id
|
||||||
assert 0 <= BufferId.ack_for(buffer_id) <= 255
|
assert 0 <= BufferId.ack_for(buffer_id) <= 255
|
||||||
|
|
||||||
|
|
||||||
|
def test_simulator_models_takeover_not_refusal_by_default():
|
||||||
|
"""The aircraft accepts a second controller; it does not refuse one.
|
||||||
|
|
||||||
|
Tested on the real drone 2026-10-02: session A's frames froze at 264 while
|
||||||
|
session B took over, and A went on reporting connected = True. The
|
||||||
|
simulator defaulted to refusing, which is the same unverified assumption
|
||||||
|
the client carried, so no test could catch the difference.
|
||||||
|
"""
|
||||||
|
import dataclasses
|
||||||
|
|
||||||
|
from mcbebop.sim import FakeBebop
|
||||||
|
|
||||||
|
field = next(f for f in dataclasses.fields(FakeBebop) if f.name == "single_controller")
|
||||||
|
assert field.default is False, "the default must model the aircraft, not the folklore"
|
||||||
|
|||||||
+31
-2
@@ -74,6 +74,16 @@ def sim():
|
|||||||
yield fake
|
yield fake
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def sim_factory():
|
||||||
|
"""For tests that need the refusing simulator rather than the default.
|
||||||
|
|
||||||
|
The default models the aircraft: a second controller is accepted and the
|
||||||
|
first is starved. Refusal is the opt-in mode.
|
||||||
|
"""
|
||||||
|
return FakeBebop
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def controller(sim):
|
def controller(sim):
|
||||||
client = Controller(sim)
|
client = Controller(sim)
|
||||||
@@ -165,15 +175,34 @@ def test_handshake_is_accepted_and_names_a_c2d_port(sim, controller):
|
|||||||
assert sim.handshakes[0]["controller_name"] == "test"
|
assert sim.handshakes[0]["controller_name"] == "test"
|
||||||
|
|
||||||
|
|
||||||
def test_a_second_controller_is_refused(sim, controller):
|
def test_a_second_controller_is_accepted_by_default(sim, controller):
|
||||||
|
"""Matching the aircraft, which accepts a newcomer and starves the first.
|
||||||
|
|
||||||
|
Tested on the real drone 2026-10-02. The refusal this test used to assert
|
||||||
|
was folklore inherited from pyparrot's error text.
|
||||||
|
"""
|
||||||
second = Controller(sim)
|
second = Controller(sim)
|
||||||
try:
|
try:
|
||||||
assert second.reply["status"] == 1
|
assert second.reply["status"] == 0
|
||||||
finally:
|
finally:
|
||||||
second.close()
|
second.close()
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_second_controller_can_be_refused_when_asked(sim_factory):
|
||||||
|
"""Kept because a client must still handle a non-zero status."""
|
||||||
|
with sim_factory(single_controller=True) as strict:
|
||||||
|
first = Controller(strict)
|
||||||
|
second = Controller(strict)
|
||||||
|
try:
|
||||||
|
assert first.reply["status"] == 0
|
||||||
|
assert second.reply["status"] == 1
|
||||||
|
finally:
|
||||||
|
second.close()
|
||||||
|
first.close()
|
||||||
|
|
||||||
|
|
||||||
def test_the_slot_is_free_again_after_release(sim, controller):
|
def test_the_slot_is_free_again_after_release(sim, controller):
|
||||||
|
"""Only meaningful in the strict mode; the default never withholds a slot."""
|
||||||
assert sim.occupied
|
assert sim.occupied
|
||||||
sim.release()
|
sim.release()
|
||||||
third = Controller(sim)
|
third = Controller(sim)
|
||||||
|
|||||||
Reference in New Issue
Block a user