fix(gsm): follow-up review fixes + restore the connections/gsm package

[yozik04]

Follow-up to #613 and #614, from the post-merge critical review.

⚠️ This also restores #614

#614's content never reached dev. It was stacked on fix/611-gsm-line-framing and GitHub marked it MERGED when it merged into that branch — but #613 was merged to dev beforehand, so paradox/connections/gsm/ does not exist on dev today. The first commit here is a cherry-pick of 9803043 to bring it back.

git merge-base --is-ancestor 9803043 origin/dev   # false

Fixes

1. send_message was async but awaited nothing — the root cause of item 7

ConnectionProtocol.send_message is a synchronous abstract method. GsmSerialProtocol overrode it as async def around a bare transport.write(). That LSP violation is why it collided with serial.protocol.SerialConnectionProtocol in the first place: #613 renamed the class to dodge the collision, and #614 then had to override Connection.write() to raise NotImplementedError so nobody could reach the un-awaited coroutine.

Making it synchronous deletes both workarounds. send_command() now uses the inherited Connection.write(), and the missing check_active() guard is in place.

2. LineFramer emitted the tail of an over-long line as a valid frame

Before, after discarding an overrun the framer would treat whatever followed up to the next terminator as a complete line:

f = LineFramer(terminator=b"\r\n", max_line_length=8)
f.feed(b"X" * 12)          # [] - discarded, good
f.feed(b"TAIL\r\nOK\r\n")  # [b"TAIL", b"OK"]  <- b"TAIL" is corrupt

It now resynchronises: bytes up to and including the next terminator are dropped, so only b"OK" comes through. reset() clears the resync state.

3. drop_blank_lines → drop_whitespace_lines

The old name conflated two things. Empty payloads are now always skipped; skipping whitespace-only lines is the opt-in behaviour the name now describes. PRT3 is the only caller and keeps its existing semantics.

4. connect() did not actually bound the port open

open_timeout was scheduled with call_later, but it only resolves connected_future — useless if create_serial_connection is what hangs, since connect() is not awaiting that future yet. The open is now wrapped in asyncio.wait_for.

Tests

1633 passing. New coverage for the overflow resync, reset() clearing it, the bounded open, and write() reaching the transport (replacing the NotImplementedError test).

5. GSM outbound SMS never worked at all

Not a regression — the code at d347e0b, before any of these PRs, has all three of these defects verbatim. The test for it was left commented out in tests/interfaces/test_gsm.py:

# level = EventLevel.INFO
# await interface.send_message("bla", level)

a. send_message was async over a synchronous base. GSMTextInterface was the codebase's only async override of AbstractTextInterface.send_message, which handle_notify and handle_panel_event call directly. Every notification built a coroutine that nobody awaited and dropped it — the source of the RuntimeWarning: coroutine 'GSMTextInterface.send_message' was never awaited seen throughout the test suite. It is now synchronous and schedules the send as a task.

b. The response queue was starved after init. connect() finishes with set_recv_callback(self.data_received), after which on_message routed every line to the callback and none to the queue — so send_command()'s queue.get() could never be satisfied. Even a correctly awaited SMS could only time out.

set_recv_callback is typed Callable[[bytes], bool], but the result was discarded. Honouring it gives URC demultiplexing for free:

if self.recv_callback is not None and self.recv_callback(message):
    return
self.queue.put_nowait(message)

data_received now returns True only for lines it consumed (+CMT and its body, +CUSD); OK, ERROR and +CMGS: fall through to the waiting command.

c. AT+CMGS was sent as one blob. Text-mode SMS is a two-stage exchange: the modem answers with a > entry prompt and only then accepts the body, terminated by Ctrl-Z rather than CRLF. Sending both at once leaves the modem sitting in entry mode with nothing sent.

The prompt is \r\n> — unterminated, so LineFramer can never surface it. GsmSerialProtocol gains a one-shot expect_prompt() that a caller arms when it knows a prompt is due; unarmed, > is correctly treated as a line that has not finished arriving. Failure paths send ESC so the modem does not swallow the next command as message text.

Tests

1654 passing, up from 1633 on dev. Each fix was verified to be genuinely covered by reverting it and confirming the new tests fail:

Reverted	Failing tests
callback return value ignored	5, incl. test_unsolicited_lines_do_not_satisfy_a_pending_command
prompt detection removed	5, incl. test_sms_is_a_two_stage_exchange

test_sms_is_a_two_stage_exchange drives real bytes through the real protocol and asserts the full command → prompt → body → +CMGS:/OK sequence, which is what the commented-out test was reaching for.

---

Follow-up: rubber-duck review of the above

Reviewing the SMS work turned up five more defects. Three were confirmed by running them, not just by reading.

6. The command channel had no mutual exclusion (blocking)

Two notifications arriving together each schedule a send. Observed: both commands reach the modem before either reply comes back.

>>> writes before any reply: [b'AT+A\r\n', b'AT+B\r\n']

For AT+CMGS that is worse than mispaired replies — between the > prompt and the Ctrl-Z the modem treats everything written as message text, so the second command becomes the body of the first SMS.

The lock lives in GSMTextInterface rather than the transport: an SMS exchange spans send_command + write_raw + read, so guarding it inside the connection would require a reentrant lock. It is also held across the whole init sequence, so a half-initialised modem cannot see an SMS interleaved between ATE0 and AT+CMGF=1.

7. A stale reply satisfied the next command (blocking)

The late answer to a timed-out command sat in the queue and was handed to whatever came next:

>>> AT+B received: b'LATE-REPLY-TO-A'

send_command now drains before writing — anything already queued predates the command. clear() drains in place instead of replacing the queue object, which previously stranded any coroutine already blocked in read().

8. expect_prompt() stayed armed after a timeout

Nothing disarmed it on the error paths, leaving the next unterminated line primed to be read as an SMS prompt. Now cleared in a finally.

9. A modem drop was fatal until restart

run() looped while not self.modem_connected and returned once connected. on_connection_loss clears the transport's flag but nothing ever reset the interface's, so an unplugged or reset modem meant no more alerts until PAI restarted — on an alarm interface, silently. run() now watches the transport and rebuilds the port, and connect() closes the previous one first (a failed retry used to leak the last attempt).

10. The init helper was dead code around a real bug

while r != expected:
    r = await self.port.send_command(message)
    data = b""
    if r == b"ERROR": raise ...
    while r != expected:
        r = await self.port.read()
        data += r + b"\n"

The outer loop can never iterate twice (the inner one only exits when r == expected), data is accumulated and thrown away, and an ERROR arriving in the inner loop is ignored — so a rejected init command cost a 5 s timeout instead of reporting itself. Replaced by _at_command(), which shares the final-result reader with the SMS path.

Also: in-flight send tasks are now tracked, so stop() cancels them and the event loop keeps a strong reference (asyncio only holds a weak one).

Tests

1664 passing, up from 1633 on dev. Every fix was verified by reverting it and confirming the new tests fail:

Reverted	Failing test
command lock	test_concurrent_sends_are_serialised
queue drain	test_a_stale_reply_does_not_satisfy_the_next_command
prompt disarm	test_a_timed_out_command_disarms_the_prompt
reconnect check	test_run_reconnects_after_the_modem_drops
callback return value	5 tests, incl. test_unsolicited_lines_do_not_satisfy_a_pending_command
prompt detection	5 tests, incl. test_sms_is_a_two_stage_exchange

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud