Fix connection status issue (#2519)
This is a manual port of https://github.com/jamulussoftware/jamulus/pull/2550 by @pgScorpio
Short description of changes
First step to get to know the codebase of #2550
CHANGELOG: DO NOT MERGE
Context: Fixes an issue?
Fixes: #2519 Supersedes: #2550
Does this change need documentation? What needs to be documented and how?
No
Status of this Pull Request
Ready for first review and decision on how to move forward.
What is missing until this pull request can be merged?
Review. It compiles but not every part of the code is fully understood and commented.
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
- [ ] I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
- [ ] I've filled all the content above
Coding style check also fails on main. Seems to be an unrelated issue?
Pasting from previous PR here:
Moved Connect/Disconnect code from CClientdlg to CClient. Now using the proper connected checks in several places. Added bDisconnectAndDisable to CChannel. (For a Client now Channel.Disconnect() will block audio data and auto disable the channel on disconnected)
Ok.
https://github.com/jamulussoftware/jamulus/pull/3364/commits/a5af8ccd478e405e0ccbc760315c366f12c97367 is for having a smaller diff in this PR. The renaming of methods can happen in another PR.
Client.Start() and Client.Stop() have been re-introduced in https://github.com/jamulussoftware/jamulus/pull/3364/commits/535f781eece9a2a20598dcdf40df6a44d1c1148f, but can be - of course - merged again if wanted.
I think, this PR should now get an in depth review.
To decide:
- What to keep
- What to split into another PR
- How and what to document.
I've no idea what this change is trying to do. It appears to be making changes to both client and server. There's no explanation given as to whether one relies on the other or what happens when different versions interact.
I think it should only do changes on the client side. The PR is still huge and potentially we need to extract the most important points. See the linked PR for old discussions. My goal is to use this to lay foundations for the JSON-RPC connect/disconnect calls.
So we need to try understanding what gets changed.
Maybe we can reconstruct something from https://github.com/pgScorpio/jamulus/commit/648880cbfec48ab10b3eaac06c846c9d6f365c20
I looked quickly through the changes the other day and didn't notice anything of concern, but I haven't yet had time to study in detail.
I believe there's a need to check if the changed conditions in the if statements are really correct or if the bug lays elsewhere
Thanks for the review! I'll go through what I can answer later.
Tagged for Jamulus 4. This PR should NOT be merged. However, the comments are still valuable.