GSM interface: replace hand-rolled CRLF framing with shared LineFramer, fix echo suppression and unbounded buffer
#611 建立於 2026年8月10日
倉庫指標
- 星標
- (414 顆星)
- PR 合併指標
- (PR 指標待抓取)
描述
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.