Skip to content

Refactor send queue#105

Open
boivie wants to merge 2 commits into
webrtc:mainfrom
boivie:refactor-send-queue
Open

Refactor send queue#105
boivie wants to merge 2 commits into
webrtc:mainfrom
boivie:refactor-send-queue

Conversation

@boivie

@boivie boivie commented Mar 20, 2026

Copy link
Copy Markdown
Collaborator

The large part of this PR is to change ThresholdWatcher to be a "simpler component" by not having it trigger callbacks itself. It was more or less ported from C++ and used a C++ way of doing things.

Now it is more explicit, and requires callers to perform actions when the threshold is crossed (which the mutators indicate). This removes the lifetime annotations and having to clone the events, and all that.

And then there are some very minor refactorings to use the Rust language better.

lndmrk
lndmrk previously approved these changes Mar 20, 2026
Comment thread src/tx/send_queue.rs Outdated
Comment thread src/tx/send_queue.rs
Comment thread src/tx/send_queue.rs Outdated
if old_value > self.low_threshold && self.value <= self.low_threshold {
(self.low_cb)();
}
// Returns `true` if the new threshold is lower than the current value, which means that the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ditto

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

boivie added 2 commits April 13, 2026 19:39
This wasn't very Rust friendly by holding on to a callback that was used
to add events whenever the low threshold was crossed. This was more or 
less a port of the C++ version of this component.

With this change, the caller will be notified (by return value) if the
threshold was crossed downwards, so that it can add events themselves.

This removes the callback, the lifetime modifier, and having to clone
events.
Just minor improvements to use Rust language features.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants