Closed jlaine closed 3 years ago
Awesome, thanks for taking the time to put that together!
I've tested with the client with the 3 other servers and they are all able to negotiate the ICE connection and complete the DTLS handshake.
For the automated test the client needs to return 0
if the peer connection was successful or 1
if it wasn't. That's something else I missed from the docs and will correct. For the aiortc client is that just a matter of adding a @pc.on("connectionstatechange")
callback?.
@jlaine I came up with this function to check if the DTLS handshake has succeeded:
async def checkIfConnected(*, pc):
print("Check if connected.")
isConnected = False
attempts = 0
while attempts < 10:
transceivers = pc.getTransceivers()
if transceivers:
print(f'Transceiver state: {transceivers[0].receiver.transport.state}.')
if transceivers[0].receiver.transport.state == "connected":
isConnected = True
break
else:
await asyncio.sleep(1)
attempts = attempts + 1
print(f'Check if connected done, isConnected={isConnected}.');
And then in main
:
#loop.run_forever()
loop.run_until_complete(checkIfConnected(pc=pc))
Does that look reasonable? It probably should be using a command line option as well.
aiortc's RTCPeerConnection recently gained a "connectionState" attribute which should simplify things. I'll give it a shot.
What command line option were you referring to?
What command line option were you referring to?
The aiortc client is more "feature rich" than the other echo test clients in that it can actually be used to send and receive/record media. The automated echo test needs a way to stop and exit after the DTLS handshake succeeds. The option I'm referring to would be something like -eoc
for exit on connect so that the client plays well with the automated tests.
Understood, I think I'll add a --check-connection-state
flag or something similar.
In the end I decided not to add a command line flag : if neither --play-from
nor --record-to
is specified, there is no point in looping forever.
I set a timeout at 10s for the entire process (HTTP request / response / setting descriptions / getting to the connected
state), to avoid getting stuck at any step. Feel free to tweak this value if necessary.
It might work if $GITHUB_ACTOR
is replaced with sipsorcery
.
But I think it's safe to merge this PR. If it breaks the CI job it's no big deal, things are expected to break regularly at this point. Plus I have latitude to fix it at the moment.
OK here goes..
Fixes: #18