Outgoing call #19
Outgoing call #19
Conversation
It appears the From header is actually more meaningingful for calls coming from Asterisk, as the Contact header does not distinguish between which device initiated a call like the From header does.
jaminh
force-pushed
the
outgoing-call
branch
2 times, most recently
from
aff1c9e
to
70c3b9c
Compare
jaminh
force-pushed
the
outgoing-call
branch
3 times, most recently
from
14c4a00
to
7a774b6
Compare
This provides a test script for outgoing calls based on the work originally started by Michael Hansen. Instructions for running the outgoing call test script can be found in the README.md file. I tried to make the acceptable coding for the OPUS codecs in the SIP messages more flexible, but the number may need to be changed back from 96 to 123 to work with Grandstream phones. I don't have a Grandstream yet to test with.
|
Please don't use force-push, it makes it harder to track fixes |
Modify codec negotiation to work with Grandstream Move example code out of call_phone.py
|
I got a Grandstream this weekend and was able to get calls to it working. I also tried to move the example code out to a separate file. |
voip_utils/sip.py
Outdated
|
|
||
|
|
||
| @dataclass | ||
| class SipEndpoint: |
There was a problem hiding this comment.
I think after adding the URI as the device identifier it will probably make more sense to construct this with the URI and then parse out the individual parts rather than building the URI from parts like it is doing now.
voip_utils/sip.py
Outdated
| print((invite_bytes + sdp_bytes).decode()) | ||
|
|
There was a problem hiding this comment.
Debug print still left in the code. If you want to keep this, consider logger. Also you can consider logging sdp_text and invite_text directly to avoid a decoding step.
call_example.py
Outdated
| lambda call_info, rtcp_state: PreRecordMessageProtocol( | ||
| "problem.pcm", | ||
| call_info.opus_payload_type, | ||
| loop=loop, |
There was a problem hiding this comment.
I know it's just an example, but let's not pass in the loop. It is not needed in modern asyncio and loop access should rely on get_running_loop
Use SipEndpoint dataclass for parsing SIP headers
voip_utils/sip.py
Outdated
| source: SipEndpoint, | ||
| dest: SipEndpoint, | ||
| rtp_port: int, | ||
| loop: Optional[asyncio.AbstractEventLoop] = None, |
There was a problem hiding this comment.
Let's not pass in the loop
voip_utils/sip.py
Outdated
| if port: | ||
| uri += f":{port}" | ||
| if description: | ||
| uri = f'"{description} <{uri}>' |
There was a problem hiding this comment.
This only has an opening ", not a closing one. Is that correct?
voip_utils/sip.py
Outdated
| self._source_endpoint = source | ||
| self._dest_endpoint = dest | ||
| self._rtp_port = rtp_port | ||
| self._request_uri = f"sip:{dest.username}@{dest.host}:{dest.port}" |
There was a problem hiding this comment.
should we use the get_sip_endpoint helper function here?
voip_utils/sip.py
Outdated
|
|
||
| for i, line in enumerate(response_lines): | ||
| line = line.strip() | ||
| if i == 0: |
There was a problem hiding this comment.
You can make this code more readable by turning this around.
if not line:
break
if i > 0:
continue
voip_utils/sip.py
Outdated
| elif not line: | ||
| break | ||
|
|
||
| if is_ok: |
There was a problem hiding this comment.
Let's turn this into a guard clause too:
if not is_ok:
return
voip_utils/sip.py
Outdated
|
|
||
| if is_ok: | ||
| _LOGGER.debug("Got OK message") | ||
| if self.transport is not None: |
There was a problem hiding this comment.
This can also be a guard clause.
voip_utils/sip.py
Outdated
| opus_payload_type=opus_payload_type, # Should probably update this to eventually support more codecs | ||
| ) | ||
| ) | ||
| except Exception: |
There was a problem hiding this comment.
Do we really need this? Can we specify which exceptions we expect ?
There was a problem hiding this comment.
This is probably out of scope for this exercise but the SipDatagramProtocol datagram_received method was also using a try catch, which is why I did the same here. I would suspect we probably want to remove the try/except there as well, and also change the places it was raising ValueError to make it consistent with this new class.
Prefer using guard clauses Use SipEndpoint attributes where appropriate Fix end quote for description
voip_utils/sip.py
Outdated
| # The From header should give us the URI used for sending SIP messages to the device | ||
| if headers.get("from") is not None: | ||
| caller_uri, caller_name = self._parse_uri_header(headers.get("from")) | ||
| caller_endpoint = SipEndpoint(headers.get("from") or "") |
There was a problem hiding this comment.
Did the "or" to make mypy happy, is there a better way to do this?
There was a problem hiding this comment.
You can pass "" as a second parameter.
| caller_endpoint = SipEndpoint(headers.get("from") or "") | |
| caller_endpoint = SipEndpoint(headers.get("from", "")) |
There was a problem hiding this comment.
Looks good! I will be home on Sunday and able to test it. (Unless Mike beats me to it).
We're currently adding a new entity type assist-satellie to Home Assistant. This will allow us to target the device to trigger Assist Pipelines (and so we can do calls 😄 )
|
Tested it and it works! 🎉 |
I got as far as successfully calling from the voip library to one of my IP phones through asterisk and playing a pre-recorded message. It is still in a pretty rough state but I thought it might be worthwhile to start getting feedback at this point. I plan to get a Grandstream soon and try testing with that.