wip: feat: implementing trickle ice #56
Conversation
rprata
force-pushed
the
feat/trickle-ice
branch
2 times, most recently
from
803e32d
to
fab6fc2
Compare
src/aioice/ice.py
Outdated
| host_candidates, host_protocols = await self.get_host_candidates( | ||
| component, addresses | ||
| ) | ||
| candidates.append(host_candidates) |
There was a problem hiding this comment.
This looks wrong, you're appending a list to a list. Did you mean candidates += host_candidates ?
src/aioice/ice.py
Outdated
| async def query_stun_server( | ||
| self, host_protocols: List[StunProtocol], timeout: int = 5 | ||
| ) -> Optional[List[Candidate]]: | ||
| candidates = None |
There was a problem hiding this comment.
Why not simply initialise candidates to an empty list instead of having to deal with None?
src/aioice/ice.py
Outdated
| # query STUN server for server-reflexive candidates (IPv4 only) | ||
| return candidates, host_protocols | ||
|
|
||
| async def query_stun_server( |
There was a problem hiding this comment.
This looks like a private method, prefix it with an underscore
src/aioice/ice.py
Outdated
| async def get_component_candidates( | ||
| self, component: int, addresses: List[str], timeout: int = 5 | ||
| ) -> List[Candidate]: | ||
| async def get_host_candidates( |
There was a problem hiding this comment.
Private method, prefix it with an underscore
src/aioice/ice.py
Outdated
| host_protocols=host_protocols, timeout=timeout | ||
| ) | ||
| if srflx_candidates: | ||
| candidates.append(srflx_candidates) |
There was a problem hiding this comment.
Same as above, candidates += srflx_candidates and you can drop the if.
|
The CI errors look specific to your branch, I've re-run the tests on the I have to admit I don't see where this code is going. I was expecting something like |
rprata
force-pushed
the
feat/trickle-ice
branch
2 times, most recently
from
39dee70
to
be6fd70
Compare
Sorry, I made something wrong in my local environment.
I thought at that moment refactor the function |
|
I still think you probably want an async iterator at the end of the day, so that we can async for candidate in conn.gather():
do_something(candidate)Obviously feel free to suggest a different API! I think taking a look at what other implementations are doing might be of value too before diving into the nitty-gritty work. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
===========================================
- Coverage 100.00% 99.75% -0.25%
===========================================
Files 7 7
Lines 1200 1213 +13
===========================================
+ Hits 1200 1210 +10
- Misses 0 3 +3
☔ View full report in Codecov by Sentry. |
Sorry @jlaine, but I can't continue this work now. Feel free to this ti close the PR. |
wip