ParadoxAlarmInterface/pai

GSM interface: replace hand-rolled CRLF framing with shared LineFramer, fix echo suppression and unbounded buffer

Closed

#611 opened on Aug 10, 2026

 (0 comments) (0 reactions) (0 assignees)Python (113 forks)auto 404
bugenhancementgood first issueprotocol

Repository metrics

Stars
 (414 stars)
PR merge metrics
 (PR metrics pending)

Description

paradox/interfaces/text/gsm.py hand-rolls CRLF line framing inline in SerialConnectionProtocol.data_received, while the rest of the connections package has moved framing into dedicated framing modules behind ConnectionProtocol.reset_framing(). A recent cleanup of that loop (dead branch removal, delimiter consumption, lazy logging) left the structure and several pre-existing defects untouched. This issue tracks the follow-up.

Defects

1. Unbounded buffer growth

self.buffer grows without limit when no \r\n ever arrives — line noise at the wrong baud rate, a modem in PDU/binary mode, or a stuck AT+CMGS prompt awaiting \x1a. find returns -1 on every callback and the only escapes are reset_framing() or connection_lost, neither of which fires on a healthy-but-noisy link.

paradox/connections/prt3/framing.py already solves this with MAX_LINE_LENGTH = 512 plus a discard-and-warn path. FrameBuffer.take/compact also avoids the O(n^2) self.buffer = self.buffer[r + 2:] re-slicing on every frame.

2. \r-only echo defeats echo suppression

Many V.25ter modems echo the command terminated by a bare <CR>, then send <CR><LF>OK<CR><LF>. Given send_message(b"AT") followed by data_received(b"AT\r\r\nOK\r\n"), the extracted frame is b"AT\r", which does not equal last_message (b"AT"). The echo is forwarded to on_message, lands in SerialCommunication.queue, and GSMTextInterface.write treats it as the response — desynchronizing the request/response stream for the rest of the session.

Fix: frame.rstrip(b"\r") before the comparison, or compare against last_message.rstrip().

3. last_message is sticky

It is cleared only on an exact match or on connection_lost. Since connect() issues ATE0, echo is normally off, so last_message retains the last command indefinitely (e.g. b"AT+CUSD=1" after init, or the whole AT+CMGS=... blob after an SMS) and will silently swallow the first later frame that happens to equal it. Echo suppression should be scoped to the first frame after a write, not "any frame, ever".

4. A raising on_message aborts the drain loop

GSMTextInterface.data_received calls data.decode() and json.loads with no guard. A non-UTF-8 SMS or a malformed +CMT header raises straight out through protocol.data_received into the asyncio transport, stalling any already-buffered complete frames until the next byte arrives.

Structural cleanup

5. Promote LineFramer to paradox/connections/framing.py

LineFramer is fully generic (parameterised terminator, no PRT3 specifics) but lives in connections/prt3/. Importing it from interfaces/text/ today would couple GSM to an unrelated panel transport. Promote it first, updating PRT3's import.

6. Have GSM delegate to LineFramer(terminator=b"\r\n")

This closes item 1 for free. Note the semantic mismatch: LineFramer keeps the terminator and drops whitespace-only lines, whereas GSM's echo comparison and on_message consumers expect it stripped. Needs either a strip_terminator flag or a caller-side strip.

Echo suppression is an application concern and should stay in the protocol/interface layer — do not push last_message into LineFramer.

7. Rename GSM's SerialConnectionProtocol

It collides with paradox.connections.serial.protocol.SerialConnectionProtocol. The two are not substitutable — the connections one has a sync send_message (called by Connection.write()), GSM's is async def — so a mis-targeted import type-checks and fails only at runtime on the first data_received. Suggested: GsmSerialProtocol or ModemLineProtocol. Single-file rename plus one test import.

8. (Optional) Relocate SerialCommunication under connections/gsm/

It is a parallel hand-rolled reimplementation of the Connection abstraction (own queue, connected_future, open_timeout, make_protocol) sitting in interfaces/. The principled end-state is a paradox/connections/gsm/ package, leaving GSMTextInterface to do AT-command orchestration and SMS parsing only. Low urgency — GSM has one transport, one consumer, and no encryption or variable-length concerns.

Notes

The characterization tests in tests/interfaces/test_gsm.py (test_serial_connection_protocol_reassembles_split_frames, test_serial_connection_protocol_drops_echoed_message) pin the current framing contract and should let the refactor land with confidence. They are not regression tests — the pre-refactor loop passed them too.

Contributor guide