Use options pattern instead of config by philipch07 · Pull Request #450 · pion/sctp

philipch07 · 2025-12-28T18:16:55Z

Description

Uses the options pattern instead of the config pattern, which is a necessary change before adding SNAP (#449).

Reference issue

Resolves #446

JoTurk

yeah this is a good direction, we need to add more validations tho.

association_options.go

codecov · 2025-12-28T18:25:04Z

Codecov Report

❌ Patch coverage is 91.72932% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.01%. Comparing base (cb53d27) to head (6fcb82c).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
association.go	88.23%	11 Missing and 9 partials ⚠️
association_rack_options.go	92.85%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #450      +/-   ##
==========================================
+ Coverage   83.81%   84.01%   +0.19%     
==========================================
  Files          52       54       +2     
  Lines        4060     4266     +206     
==========================================
+ Hits         3403     3584     +181     
- Misses        480      493      +13     
- Partials      177      189      +12

Flag	Coverage Δ
go	`84.01% <91.72%> (+0.19%)`	⬆️

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.

association_options.go

JoTurk

looks good, i think we'll just need to add test coverage and make newAssociationWithOptions public

JoTurk · 2025-12-30T17:08:10Z

^ mean making newAssociationWithOptions through new APIs to construct Client and Server.

JoTurk

looks good, just need to finish it, fix the lint and it's ready for merge.

philipch07 · 2026-01-23T22:31:05Z

^ mean making newAssociationWithOptions through new APIs to construct Client and Server.

@JoTurk I'm not sure if the client and server need to have specific settings for each beyond the existing options. Did you have something more specific in mind? I left a couple TODO's that you can ctrl+f in associations.go and association_options.go where I'm unsure about how to proceed.

Also looking into createAssociation(), it currently just takes Config but changing to ServerConfig or ClientConfig would imply to users that there are different configs that need to be set up, so we would probably have to be careful to specify whatever the differences are if we choose to split them.

JoTurk · 2026-01-24T14:31:49Z

@philipch07 In DTLS we have Option which is a union between client and server options. For SCTP we don't need this split now because our current options symmetric, but IMO it's nice to split them early before we eventually add dial/listen APIs in v2 and different options for client and server with overlapping options.

100% up to you, I support any choice you pick.

fippo · 2026-02-20T11:27:40Z

how do we make progress? I have a local rebase of #449 ontop of this and it seems to work ok

JoTurk

Thank you a lot nofear :)

association.go

errors.go

association_rack_options.go

JoTurk · 2026-02-20T11:39:53Z

association_test.go

+	loggerFactory := logging.NewDefaultLoggerFactory()
+	opts = append([]AssociationOption{WithLoggerFactory(loggerFactory)}, opts...)
+
+	clientCfg := Config{NetConn: ca}
+	serverCfg := Config{NetConn: cb}
+
+	for _, opt := range opts {
+		if err := opt.applyClient(&clientCfg); err != nil {
+			_ = ca.Close()
+			_ = cb.Close()
+
+			return nil, nil, err
+		}
+		if err := opt.applyServer(&serverCfg); err != nil {
+			_ = ca.Close()
+			_ = cb.Close()
+
+			return nil, nil, err
+		}
+	}


Not a big deal but maybe we should try to test the external functions instead, so refactoring is less painful.

JoTurk

This is just missing the public constructors :) ServerWithOptions& ClientWithOptions

JoTurk

Thank you for all the work you did in this PR.
(I finished the rest of the PR since you told me that you'll be busy for a while).
If you can please take a quick look before we merge this

Edit: confirmed with nofear over discord that this is ready.

- Add public options contructors - Do not run options tests for wasm - Update examples to options --------- Co-authored-by: Jo Turk <[email protected]>

JoTurk reviewed Dec 28, 2025

View reviewed changes

association_options.go Outdated Show resolved Hide resolved

association_options.go Outdated Show resolved Hide resolved

association_options.go Outdated Show resolved Hide resolved

philipch07 force-pushed the pch07/config-to-options branch from f888d39 to 1192971 Compare December 28, 2025 18:23

philipch07 force-pushed the pch07/config-to-options branch 2 times, most recently from 124aeb4 to 32dc80b Compare December 28, 2025 19:22

JoTurk reviewed Dec 28, 2025

View reviewed changes

association_options.go Outdated Show resolved Hide resolved

philipch07 force-pushed the pch07/config-to-options branch from 32dc80b to 8021a05 Compare December 28, 2025 20:24

JoTurk mentioned this pull request Dec 28, 2025

Implement SCTP Negotiation Acceleration Protocol (SNAP) #449

Merged

philipch07 force-pushed the pch07/config-to-options branch from 8021a05 to aa606bd Compare December 28, 2025 20:36

JoTurk reviewed Dec 30, 2025

View reviewed changes

philipch07 force-pushed the pch07/config-to-options branch from aa606bd to c1a345b Compare January 21, 2026 20:36

JoTurk reviewed Jan 22, 2026

View reviewed changes

philipch07 force-pushed the pch07/config-to-options branch 2 times, most recently from ef25bcb to 5cdf355 Compare January 23, 2026 22:24

philipch07 force-pushed the pch07/config-to-options branch 4 times, most recently from 527f759 to f71b4fd Compare January 25, 2026 01:39

JoTurk reviewed Feb 20, 2026

View reviewed changes

JoTurk mentioned this pull request Feb 20, 2026

allow users to change the congestion control algorithm #458

Draft

philipch07 force-pushed the pch07/config-to-options branch 5 times, most recently from 1276fd5 to a714f13 Compare February 22, 2026 20:20

philipch07 marked this pull request as ready for review February 22, 2026 20:53

JoTurk reviewed Feb 23, 2026

View reviewed changes

JoTurk force-pushed the pch07/config-to-options branch from 9975511 to 59aa13b Compare February 26, 2026 21:03

JoTurk approved these changes Feb 26, 2026

View reviewed changes

JoTurk force-pushed the pch07/config-to-options branch from 59aa13b to 675bd76 Compare February 26, 2026 21:16

Migrate from config to options pattern

6fcb82c

- Add public options contructors - Do not run options tests for wasm - Update examples to options --------- Co-authored-by: Jo Turk <[email protected]>

JoTurk force-pushed the pch07/config-to-options branch from 675bd76 to 6fcb82c Compare February 26, 2026 21:17

JoTurk merged commit 6fcb82c into master Feb 26, 2026
17 checks passed

JoTurk deleted the pch07/config-to-options branch February 26, 2026 21:20

Conversation

philipch07 commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reference issue

Uh oh!

JoTurk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

JoTurk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoTurk commented Dec 30, 2025

Uh oh!

JoTurk left a comment

Choose a reason for hiding this comment

Uh oh!

philipch07 commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoTurk commented Jan 24, 2026

Uh oh!

fippo commented Feb 20, 2026

Uh oh!

JoTurk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoTurk Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

JoTurk left a comment

Choose a reason for hiding this comment

Uh oh!

JoTurk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

philipch07 commented Dec 28, 2025 •

edited

Loading

codecov bot commented Dec 28, 2025 •

edited

Loading

JoTurk left a comment •

edited

Loading

philipch07 commented Jan 23, 2026 •

edited

Loading

JoTurk left a comment •

edited

Loading