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

Refactor UpdateDisplay() to call UpdateUploadRate() #3369

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

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Sep 13, 2024

Short description of changes

Refactor UpdateDispay() method. This was extracted from #2550
and is related to: https://github.com/jamulussoftware/jamulus/pull/3364/files/21815c1a0708979fe0414c30e0294feaa7632be3#r1759185427

It makes sense to call UpdateUploadRate() from UpdateDisplay() since it gets same functionality to the same place.

Not sure, if the condition is correct, as we do not check if pClient is running, but rather it is connected now.

CHANGELOG: SKIP

Context: Fixes an issue?

No.

Does this change need documentation? What needs to be documented and how?

No. This even adds documentation

Status of this Pull Request

Ready for review

What is missing until this pull request can be merged?

Review. Tested locally by disconnecting and connecting via GUI and changing Audio quality and enabling/disabling small network buffers. Network rate shows correctly. Also checked by starting Jamulus with -c flag and seems to do what it should do: Label shows correct data if connected and --- if not connected.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide (check fails, but locally everything seems ok)
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@@ -1063,25 +1063,31 @@ void CClientSettingsDlg::OnSndCrdBufferDelayButtonGroupClicked ( QAbstractButton
UpdateDisplay();
}

/// @method
Copy link
Member Author

@ann0see ann0see Sep 13, 2024

Choose a reason for hiding this comment

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

Probably no need to add @ method here.

ann0see added a commit to ann0see/jamulus that referenced this pull request Sep 13, 2024
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