Refactor protocol structure - #59
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #59 +/- ##
==========================================
- Coverage 93.93% 87.66% -6.27%
==========================================
Files 24 26 +2
Lines 1994 1954 -40
==========================================
- Hits 1873 1713 -160
- Misses 121 241 +120 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot add tests, 100% or it didn't happen. No mocks, follow the superjoe and contributing guidelines. |
Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com> Agent-Logs-Url: https://github.com/codingjoe/VoIP/sessions/dd6b4390-f976-4e9d-ae5f-e207b4c92ae9
Done in d6657e5. Added 100% line coverage for all SIP files changed in this PR:
No mocks — only transport is stubbed per CONTRIBUTING.md guidelines. 342 tests pass total. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot unit tests must match the API they are testing. e.G. |
…scenario naming Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com> Agent-Logs-Url: https://github.com/codingjoe/VoIP/sessions/50ee7641-f523-4ea5-b7b9-ac597fe93272
Done in 4762b05. Restructured all test files to one
All 343 tests still pass at 100% SIP coverage. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the SIP stack by moving request/response handling into explicit transaction classes, introduces a shared NetworkAddress type for host/port values, and renames the RTP call-leg base class from RTPCall to Session. It updates the CLI, docs, and tests accordingly.
Changes:
- Add
NetworkAddressand apply it across STUN/RTP/SIP addressing and logging. - Extract SIP REGISTER/INVITE behaviors into
voip/sip/transactions.pyand simplifySessionInitiationProtocolto dispatch to transactions. - Rename RTP call leg handler base class (
RTPCall→Session) and update docs/tests.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| voip/types.py | Adds NetworkAddress helper type used across networking layers. |
| voip/stun.py | Switches STUNProtocol APIs to accept NetworkAddress. |
| voip/rtp.py | Renames RTPCall→Session and migrates address typing to NetworkAddress. |
| voip/audio.py | Updates AudioCall base class to the new Session. |
| voip/sip/messages.py | Adds dialog/branch/tag helpers and introduces Dialog. |
| voip/sip/types.py | Extends SipUri with maddr/ttl/transport helpers and hides password in repr. |
| voip/sip/transactions.py | New transaction layer with REGISTER + INVITE transaction logic. |
| voip/sip/protocol.py | Refactors protocol to transaction dispatch and keepalive logic. |
| voip/sip/exceptions.py | New module for RegistrationError. |
| voip/sip/init.py | Re-exports transaction classes. |
| voip/sdp/types.py | Makes several SDP dataclasses frozen for immutability. |
| voip/sdp/messages.py | Adds slots to SessionDescription. |
| voip/main.py | Refactors CLI wiring to use transactions + NetworkAddress. |
| tests/, tests/sip/ | Updates/rewrites tests to align with the refactor and new APIs. |
| docs/*, mkdocs.yml, README.md | Updates docs/nav/examples for the new structure and class names. |
| CONTRIBUTING.md, .pre-commit-config.yaml | Adjusts contributing guidance and pre-commit configuration. |
Comments suppressed due to low confidence (1)
docs/sessions.md:3
- Spelling: "Dessions" should be "Sessions".
for more information, see https://pre-commit.ci
|
@codingjoe I've opened a new pull request, #61, to work on those changes. Once the pull request is ready, I'll request review from you. |
`data_received()` assumed each TCP chunk was exactly one complete SIP message. TCP is a stream protocol — frames can be split across multiple deliveries or coalesced into one. ## Changes - **`recv_buffer: bytearray`** — accumulates raw bytes from the TCP stream across `data_received()` calls - **`PING` / `PONG` constants** — module-level `typing.Final[bytes]` constants (`b"\r\n\r\n"` and `b"\r\n"`) replace all scattered byte literals in the keepalive path - **`extract_frames()`** — generator that extracts complete frames from the buffer using `Content-Length` header framing ([RFC 3261 §18.3](https://datatracker.ietf.org/doc/html/rfc3261#section-18.3)); yields a `memoryview` into the buffer for each SIP message (zero-copy until `Message.parse` needs `bytes`) and the `PING`/`PONG` constants for keepalive frames; the view is explicitly released before the buffer is compacted; partial messages remain buffered until all bytes arrive - **`dispatch_frame()`** — routes a single complete frame (`memoryview | bytes`) to the appropriate handler, converting to `bytes` only at parse time - **`data_received()`** — appends to `recv_buffer` and iterates the `extract_frames()` generator ```python def data_received(self, data: bytes) -> None: self.recv_buffer.extend(data) for frame in self.extract_frames(): self.dispatch_frame(frame) ``` ## Tests 39 new tests covering: split headers, split body, coalesced messages, all keepalive edge cases (partial PING, PONG-then-message, etc.), invalid `Content-Length`, and end-to-end `data_received` reassembly scenarios. <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com> Co-authored-by: Johannes Maron <johannes@maron.family>
No description provided.