Add video proxy #60
Add video proxy #60
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new UdpToTcpProxy class that implements a network proxy service, forwarding UDP packets to connected TCP clients. This appears to be adding video streaming proxy functionality to the Yamcs system.
- Implements a UDP-to-TCP proxy that listens for UDP packets and forwards them to multiple TCP clients
- Extends
AbstractThreadedTcDataLinkfor integration with the Yamcs framework - Provides system parameter monitoring for UDP and TCP port configurations
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| } | ||
| tcpClients.remove(client); | ||
| } | ||
| } |
There was a problem hiding this comment.
Removing an element from a synchronized collection while iterating over it can cause a ConcurrentModificationException. Consider using an iterator with remove() or collecting clients to remove in a separate list first.
| } | |
| clientsToRemove.add(client); | |
| } | |
| } | |
| tcpClients.removeAll(clientsToRemove); |
| try { | ||
| while (!clientSocket.isClosed()) { | ||
| if (clientSocket.getInputStream().read() == -1) { | ||
| break; |
There was a problem hiding this comment.
The read() call blocks indefinitely and could prevent proper cleanup. Consider using a timeout or non-blocking approach to detect client disconnections more efficiently.
| break; | |
| clientSocket.setSoTimeout(1000); // Set read timeout to 1 second | |
| while (!clientSocket.isClosed()) { | |
| try { | |
| if (clientSocket.getInputStream().read() == -1) { | |
| break; | |
| } | |
| } catch (java.net.SocketTimeoutException e) { | |
| // Timeout occurred, check again if socket is closed | |
| continue; |
| import org.yamcs.yarch.TupleDefinition; | ||
|
|
||
| // public class UdpToTcpProxy extends AbstractThreadedTcDataLink implements SystemParametersProducer | ||
| // { |
There was a problem hiding this comment.
Remove commented-out code. If this interface implementation is planned for the future, consider adding a TODO comment with context instead.
| // { | |
| // TODO: If implementing SystemParametersProducer is planned, add interface and implementation here. |
| } | ||
| } | ||
|
|
||
| // @Override |
There was a problem hiding this comment.
Large blocks of commented-out code (lines 252-310) should be removed to improve readability. If these methods need to be implemented later, consider adding proper TODO comments instead.
|
|
||
| @Override | ||
| protected void startUp() throws Exception { | ||
| // TODO Auto-generated method stub |
There was a problem hiding this comment.
The startUp() method is empty with only a TODO comment. Either implement the required functionality or remove the comment if no implementation is needed.
| // TODO Auto-generated method stub |
|
|
||
| @Override | ||
| protected void shutDown() throws Exception { | ||
| // TODO Auto-generated method stub |
There was a problem hiding this comment.
The shutDown() method is empty with only a TODO comment. Either implement the required cleanup functionality or remove the comment if no implementation is needed.
| // TODO Auto-generated method stub | |
| super.shutDown(); |
| /* Update PV telemetry */ | ||
| TupleDefinition tdef = gftdef.copy(); | ||
| // pushTuple(tdef, cols); | ||
|
|
There was a problem hiding this comment.
Commented-out code for telemetry update should be either implemented or removed. The variables 'tdef' and undefined 'cols' suggest incomplete implementation.
No description provided.