Skip to content

Check if only the NOMINATION attribute is set for binding request#884

Open
martinxsliu wants to merge 2 commits intopion:masterfrom
Navigate-AI:ml/check-nomination-attr
Open

Check if only the NOMINATION attribute is set for binding request#884
martinxsliu wants to merge 2 commits intopion:masterfrom
Navigate-AI:ml/check-nomination-attr

Conversation

@martinxsliu
Copy link

Re: #881

libwebrtc's implementation of ICE renomination only sets the NOMINATION attribute and not the USE_CANDIDATE attribute when renomination is enabled and libwebrtc is controlling: https://webrtc.googlesource.com/src/+/refs/heads/main/p2p/base/connection.cc#1157.

In order to make Pion compatible with libwebrtc, this PR updates the handle binding request check to look for either attribute.

@JoTurk JoTurk closed this Feb 9, 2026
@JoTurk JoTurk reopened this Feb 9, 2026
@JoTurk
Copy link
Member

JoTurk commented Feb 9, 2026

(i closed and opened this PR, because Github CI was stuck).

@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.51%. Comparing base (7aa0886) to head (4c4de2a).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #884      +/-   ##
==========================================
- Coverage   88.53%   88.51%   -0.02%     
==========================================
  Files          44       44              
  Lines        5540     5540              
==========================================
- Hits         4905     4904       -1     
- Misses        440      441       +1     
  Partials      195      195              
Flag Coverage Δ
go 88.51% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@JoTurk JoTurk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test for this?
Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants