Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC: Fix bug when transitioning states #296

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sterlingdeng
Copy link
Contributor

@sterlingdeng sterlingdeng commented Dec 20, 2024

The original implementation did not transition rate controller states properly. The initial state of the transition was incorrect because it did not use or record the previous state of the rate controller. This lastState field was available but never used, which indicates that it probably was originally planned for this use case but might have slipped through the cracks during the initial implementation.

See GCC RFC for details.

Description

Reference issue

Fixes #271

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.07%. Comparing base (e187410) to head (577d59e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #296      +/-   ##
==========================================
+ Coverage   71.31%   72.07%   +0.76%     
==========================================
  Files          80       80              
  Lines        4483     4484       +1     
==========================================
+ Hits         3197     3232      +35     
+ Misses       1153     1117      -36     
- Partials      133      135       +2     
Flag Coverage Δ
go 72.07% <100.00%> (+0.89%) ⬆️
wasm 70.16% <100.00%> (+0.76%) ⬆️

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.

@sterlingdeng sterlingdeng force-pushed the sdeng/gcc_fix_rate_controller branch 2 times, most recently from aecb15e to 6da699f Compare December 21, 2024 08:16
This fixes an issue described in pion#271.
The original implementation did not transition rate controller states
properly.
@sterlingdeng sterlingdeng force-pushed the sdeng/gcc_fix_rate_controller branch from 6da699f to 577d59e Compare December 21, 2024 08:22
@sterlingdeng sterlingdeng changed the title Fixes bug in GCC implementation GCC: Fix bug when transitioning states Dec 21, 2024
@sterlingdeng sterlingdeng marked this pull request as ready for review December 21, 2024 08:36
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.

GCC: RateController does not use the internal state
1 participant