Dissolve call.py: SessionInitiationProtocol is the session, RealtimeTransportProtocol is the call handler#12
Conversation
- IncomingCall is now a pure RTP handler (no SIP knowledge) - answer() and reject() moved to IncomingCallProtocol (SIP session) - IncomingCallProtocol/RegisterProtocol accept call_class parameter - invite_received() signature now includes request for SIP context - WhisperCall is now a pure RTP handler (no SIP params) - CLI creates RegisterProtocol with call_class=TranscribingCall Co-authored-by: codingjoe <[email protected]>
Co-authored-by: codingjoe <[email protected]>
codingjoe
left a comment
codingjoe
left a comment
There was a problem hiding this comment.
@copilot better but not quite there yet. I want the SIP handler to implement handling different incoming messages and have a function for outgoing messages. Like registering, dialing, accepting, or rejecting calls. It should also have a method for an incoming call, that can be implemented by subclasses to either accept or decline.
Dissolve the call.py. The logical separation should be SIP is the session, while RTP is the individual call.
At the end I want a public API in the readme similar to this:
from voip.sip import SIP
class TransribeSession(SIP):
def call_received(self, request):
self.answer(request=request, class_class=TransribeCallToStdOut)
class TransribeCallToStdOut(RTP):
def audio_received(self, data):
# transcripe implementation goes here- voip/sip/protocol.py: SIP class expanded with call_received(), answer(), reject(), keepalive handling, addr tracking by Call-ID - voip/sip/session.py: new RegisterSIP(STUNProtocol, SIP) with REGISTER + digest auth + STUN; no call_class ctor arg (passed to answer() instead) - voip/sip/__init__.py: exports SIP, RegisterSIP, SessionInitiationProtocol - voip/rtp.py: adds RTP plain base class (caller + audio_received); keeps RealtimeTransportProtocol as internal asyncio protocol - voip/call.py: dissolved to compat shim re-exporting from new locations - voip/whisper.py: WhisperCall now extends RTP instead of IncomingCall - voip/__init__.py: exports RTP, SIP, RegisterSIP - voip/__main__.py: TranscribeSession(RegisterSIP) with call_received() calling self.answer(request=request, call_class=TranscribingCall) - tests/test_call.py: rewritten to test SIP/RegisterSIP/RTP at new paths - tests/test_whisper.py: updated to use RTP base class - tests/test_main.py: updated for call_received() API - README.md: updated with new SIP/RTP public API examples Co-authored-by: codingjoe <[email protected]>
Done in commit d0b6564. The new architecture now matches the desired API: from voip.sip import SIP # or RegisterSIP for carrier registration
from voip.rtp import RTP
class TransribeSession(SIP):
def call_received(self, request):
self.answer(request=request, call_class=TransribeCallToStdOut)
class TransribeCallToStdOut(RTP):
def audio_received(self, data):
# transcription implementation goes hereKey changes:
|
voip/sip/protocol.py
Outdated
|
|
||
| class SessionInitiationProtocol(asyncio.DatagramProtocol): | ||
| """An asyncio protocol handler for the Session Initiation Protocol (RFC 3261).""" | ||
| class _RTPProtocol(RealtimeTransportProtocol): |
There was a problem hiding this comment.
Fixed in 30e1e77. RTP now extends asyncio.DatagramProtocol directly — it strips its own RTP header and calls audio_received(). SIP.answer() passes the call_class factory straight to create_datagram_endpoint, so there's no wrapper class and no import of voip.rtp in voip/sip/. (RTP is only referenced under TYPE_CHECKING for the type annotation.)
voip/sip/session.py
Outdated
| __all__ = ["RegisterSIP"] | ||
|
|
||
|
|
||
| class RegisterSIP(STUNProtocol, SIP): |
There was a problem hiding this comment.
Why is this a separate protocol in a session.py? Just extend the existing sip hanlder.
There was a problem hiding this comment.
Fixed in 30e1e77. RegisterSIP is now in voip/sip/protocol.py alongside SIP. voip/sip/session.py has been deleted.
voip/__init__.py
Outdated
| VERSION = _version.version_tuple | ||
|
|
||
| __all__ = ["Request", "Response", "SIP", "SessionInitiationProtocol"] | ||
| __all__ = ["RTP", "RegisterSIP", "Request", "Response", "SIP", "SessionInitiationProtocol"] |
There was a problem hiding this comment.
Fixed in 30e1e77. voip/__init__.py now contains only the version info — no class imports.
voip/call.py
Outdated
| @@ -1,390 +1,11 @@ | |||
| from __future__ import annotations | |||
| """Compatibility shim — use voip.sip and voip.rtp instead. | |||
There was a problem hiding this comment.
You don't need to worry about backward compatibility. clean up!
There was a problem hiding this comment.
Done in 30e1e77. voip/call.py has been deleted entirely.
…lete call.py - voip/rtp.py: RTP now extends asyncio.DatagramProtocol directly (strips RTP header, calls audio_received). No wrapper class needed. RealtimeTransportProtocol kept as backward-compat alias. - voip/sip/protocol.py: removed _RTPProtocol and 'from voip.rtp' import. RegisterSIP folded in from session.py. RTP referenced only under TYPE_CHECKING. call_class passed directly to create_datagram_endpoint (RTP IS the protocol). - voip/sip/session.py: deleted (RegisterSIP lives in protocol.py now). - voip/call.py: deleted entirely (no backward-compat shim). - voip/__init__.py: minimal — only version info, no global class imports. - voip/sip/__init__.py: exports SIP, RegisterSIP, SessionInitiationProtocol. - tests/test_rtp.py: updated to use RTP class directly. - tests/test_call.py: updated logger names (session->protocol). Co-authored-by: codingjoe <[email protected]>
voip/sip/protocol.py
Outdated
|
|
||
|
|
||
| SIP = SessionInitiationProtocol | ||
| class RegisterSIP(STUNProtocol, SIP): |
There was a problem hiding this comment.
Merge the RegisterSIP and SIP handler into one.
There was a problem hiding this comment.
Done in 2828962. RegisterSIP and SIP are merged into a single SessionInitiationProtocol class. Registration params (server_address, aor, username, password, stun_server_address) are optional in __init__; connection_made only calls register() when server_address is set. datagram_received multiplexes keepalive, STUN, and SIP in one method.
voip/sip/protocol.py
Outdated
|
|
||
|
|
||
| #: Alias for backward compatibility. | ||
| SessionInitiationProtocol = SIP |
There was a problem hiding this comment.
Don't reverse the naming. The class should be called SessionInitiationProtocol and SIP should be a fallback.
There was a problem hiding this comment.
Fixed in 2828962. SessionInitiationProtocol is now the primary class name; SIP = SessionInitiationProtocol is the short alias at the bottom of the file.
voip/rtp.py
Outdated
|
|
||
| RTP = RealtimeTransportProtocol | ||
| #: Backward-compatible alias. | ||
| RealtimeTransportProtocol = RTP |
There was a problem hiding this comment.
Don't reverse the naming. The class should be named RealtimeTransportProtocol and RTP is the fallback.
There was a problem hiding this comment.
Fixed in 2828962. RealtimeTransportProtocol is now the primary class name; RTP = RealtimeTransportProtocol is the short alias at the bottom of the file.
…rimary/alias naming - voip/sip/protocol.py: Merge SIP + RegisterSIP into one class called SessionInitiationProtocol. Registration params (server_address, aor, username, password, stun_server_address) are optional in __init__; connection_made conditionally registers when server_address is set. datagram_received multiplexes keepalive, STUN, and SIP in one method. SIP = SessionInitiationProtocol as short alias. - voip/rtp.py: Rename RTP -> RealtimeTransportProtocol (primary). RTP = RealtimeTransportProtocol as short alias. - voip/sip/__init__.py: Export SIP and SessionInitiationProtocol only. - voip/__main__.py: Use `from voip.sip import SIP` instead of RegisterSIP. - voip/whisper.py: WhisperCall extends RealtimeTransportProtocol directly. - tests: Update imports, class names, and rewrite patching test to reflect the single merged class. Use RealtimeTransportProtocol as primary in test_rtp.py. Add super().__init__() to ConcreteProtocol in test_aio.py. Co-authored-by: codingjoe <[email protected]>
…re None - response_received: early-return when server_address is None (no registration active), and guard auth challenge handling when username/password are None - registrar_uri: raise ValueError with clear message when aor is not set - All 134 tests pass, linter clean, no CodeQL alerts Co-authored-by: codingjoe <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## transcribe-call #12 +/- ##
===================================================
- Coverage 95.91% 94.26% -1.65%
===================================================
Files 11 11
Lines 538 523 -15
===================================================
- Hits 516 493 -23
- Misses 22 30 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR restructures the public API to clearly separate the SIP session layer (SessionInitiationProtocol / SIP) from the RTP call handler (RealtimeTransportProtocol / RTP), and removes the legacy voip/call.py abstraction.
Changes:
- Merged call handling + optional carrier registration into a single
SessionInitiationProtocol(aliased asSIP), withcall_received()→answer(..., call_class=...)/reject(...). - Introduced/positioned
RealtimeTransportProtocol(aliased asRTP) as the user-facing RTP audio handler base class; updatedWhisperCalland CLI accordingly. - Dropped Python 3.10–3.12 support (now
requires-python >= 3.13) and updated CI/pre-commit + enums toenum.StrEnum.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| voip/sip/protocol.py | Unified SIP session handler: INVITE dispatch, RTP answering, and optional REGISTER+STUN+digest auth. |
| voip/rtp.py | Defines RealtimeTransportProtocol/RTP as the RTP payload handler base. |
| voip/whisper.py | Refactors WhisperCall to be a pure RTP handler (no SIP coupling). |
| voip/types.py | Switches DigestQoP to enum.StrEnum (Python 3.13+ baseline). |
| voip/sip/types.py | Switches SIPStatus to enum.StrEnum. |
| voip/sip/init.py | Exposes SIP/SessionInitiationProtocol as the SIP entrypoint. |
| voip/main.py | Updates CLI transcribe flow to the new SIP session + RTP call_class model. |
| voip/init.py | Removes re-exports; package root now only exposes version info. |
| voip/call.py | Deletes legacy call/session abstraction layer. |
| tests/test_call.py | Rewrites tests to target new SIP session + RTP handler architecture. |
| tests/test_rtp.py | Adds alias and protocol inheritance assertions; updates naming. |
| tests/test_whisper.py | Updates tests for WhisperCall now inheriting from RTP. |
| tests/test_main.py | Adjusts CLI tests to validate call_received → answer integration. |
| tests/sip/test_aio.py | Fixes test subclass initialization to call super().__init__(). |
| README.md | Updates examples to new SIP/RTP split (but registration example currently mismatches code). |
| pyproject.toml | Raises minimum Python to 3.13 and trims classifiers accordingly. |
| .pre-commit-config.yaml | Updates pyupgrade target to --py313-plus. |
| .github/workflows/ci.yml | Drops 3.10–3.12 from CI matrices; tests on 3.13/3.14. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| #### Registering with a SIP carrier | ||
|
|
||
| Use `RegisterSIP` to register with a SIP carrier and receive inbound calls: | ||
|
|
||
| ```python | ||
| import asyncio | ||
| import voip | ||
| from voip.sip import RegisterSIP | ||
| from voip.rtp import RTP | ||
|
|
||
|
|
||
| class MyCall(RTP): | ||
| def audio_received(self, data: bytes) -> None: | ||
| print(f"Audio from {self.caller}: {len(data)} bytes") | ||
|
|
||
|
|
||
| class MyProtocol(voip.SIP): | ||
| def request_received(self, request: voip.Request, addr: tuple[str, int]) -> None: | ||
| print(request, addr) | ||
| class MySession(RegisterSIP): | ||
| def registered(self) -> None: | ||
| print("Registration successful — waiting for calls") |
There was a problem hiding this comment.
The README shows from voip.sip import RegisterSIP / class MySession(RegisterSIP), but there is no RegisterSIP exported/defined (and the new API description indicates registration is handled by SessionInitiationProtocol/SIP when server_address is provided). Update this example to use SessionInitiationProtocol/SIP (or reintroduce/export a RegisterSIP alias if that's intended).
| case "INVITE": | ||
| logger.info("INVITE received from %s", addr[0]) | ||
| call_id = request.headers.get("Call-ID", "") | ||
| self._request_addrs[call_id] = addr | ||
| self.call_received(request) |
There was a problem hiding this comment.
call_id = request.headers.get("Call-ID", "") allows missing Call-ID requests to be stored under the empty-string key, which can overwrite another pending INVITE and later cause answer()/reject() to use the wrong address. Since Call-ID is mandatory for INVITE, consider validating it and either rejecting with a 400 response (or at least early-return/log) instead of using a default "" key.
| def parse_auth_challenge(header: str) -> dict[str, str]: | ||
| """Parse Digest challenge parameters from a WWW-Authenticate/Proxy-Authenticate header.""" | ||
| _, _, params_str = header.partition(" ") | ||
| params = {} | ||
| for part in re.split(r",\s*(?=[a-zA-Z])", params_str): | ||
| key, _, value = part.partition("=") | ||
| if key.strip(): | ||
| params[key.strip()] = value.strip().strip('"') | ||
| return params |
There was a problem hiding this comment.
parse_auth_challenge splits parameters on commas without respecting quoted strings. Headers like qop="auth,auth-int" will be split mid-value, producing incorrect/extra params and potentially breaking auth negotiation. Consider parsing with a quote-aware tokenizer (or a regex that ignores commas inside quotes) so quoted comma-separated values remain intact.
Dissolves
voip/call.pyand establishes a clean, public-facing separation between the SIP session layer and the RTP audio call handler.Architecture
Changes
SessionInitiationProtocol(voip/sip/protocol.py) — single, unified SIP session handler. Merges the formerSIPbase andRegisterSIPsubclass into one class. Registration parameters (server_address,aor,username,password,stun_server_address) are optional;connection_madeconditionally sends REGISTER only whenserver_addressis set.datagram_receivedmultiplexes RFC 5626 keepalives, STUN (RFC 7983), and SIP in a single method. Exposes:call_received(request)— override to accept or reject an incoming callanswer(request, *, call_class)— schedules RTP setup and sends 200 OK with SDPreject(request, ...)— sends a reject responseregister(...)— sends a SIP REGISTER with optional digest auth credentialsregistered()— override to react after successful carrier registrationSIPis kept as a short alias.RealtimeTransportProtocol(voip/rtp.py) — user-facing base class for audio call handlers (caller: str,audio_received(data: bytes)). Extendsasyncio.DatagramProtocoldirectly, stripping the RTP header before callingaudio_received.voip/sip/has no import ofvoip.rtpat runtime —RealtimeTransportProtocolsubclasses are passed as the protocol factory tocreate_datagram_endpoint.RTPis kept as a short alias.voip/call.py— deleted entirely.voip/__init__.py— contains only version info; no global class imports. Users import fromvoip.sipandvoip.rtpdirectly.WhisperCall— extendsRealtimeTransportProtocoldirectly, with no SIP knowledge.CLI —
TranscribeSession(SIP)overridescall_receivedand callsself.answer(request=request, call_class=TranscribingCall), passing registration params at construction time.README — updated with the new
SessionInitiationProtocol/RealtimeTransportProtocolpublic API examples.🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.