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

Issue 772 proxy send reverse direction #787

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kiranlahiri
Copy link

Introduce changes to the Proxy class to enable bidirectional handling of control packets, which was not previously supported.

Added a control endpoint (receiver_control_endp_) to manage control packets.
Update to handle and route control packets appropriately using a new control queue (control_queue_).
Enhanced configuration to bind control endpoints during Proxy initialization

Add test case ProxyWithControlEndpoint that validates the following

  • Initialization of Proxy with source, repair, and control endpoints.
  • Routing and handling of control packets via the Proxy class.
  • Integration of control packets into the real-time streaming workflow.

Copy link

github-actions bot commented Dec 9, 2024

🤖 Upon creation, pull request description does not have a link to an issue. If there is a related issue, please add it to the description using any of the supported formats.

@gavv
Copy link
Member

gavv commented Dec 19, 2024

Hi, thanks for the patch.

  1. Please rebase and retarget your PR on develop branch, see contribution guidelines.

  2. There is no need to create a new test file, we already have test_loopback_sender_2_receiver.cpp.

  3. Can you tell me how you created this patch, did you ask an LLM to write it entirely or partially? I'm just curious.

    The code in ProxyWithControlEndpoint uses nonexistent API like roc_proxy and shouldn't compile. The test needs to be rewritten in line with the other tests and needs to use test::Context, test::Receiver, etc, instead of using roc_context, roc_receiver, etc.

I haven't looked into the code very closely yet, as it obviously needs major edits.

@gavv gavv added needs revision Pull request should be revised by its author contribution A pull-request by someone else except maintainers needs rebase Pull request has conflicts and should be rebased labels Dec 19, 2024
@github-actions github-actions bot removed the needs rebase Pull request has conflicts and should be rebased label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution A pull-request by someone else except maintainers needs revision Pull request should be revised by its author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants