Skip to content

Allow sending and receiving messages over the same UDP socket #66

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions modules/core/src/main/java/com/illposed/osc/transport/OSCPort.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,17 @@ protected OSCPort(
this(local, remote, serializerAndParserBuilder, NetworkProtocol.UDP);
}

protected OSCPort(
final Transport transport)
{
if (transport == null) {
throw new IllegalStateException(
Copy link
Owner

Choose a reason for hiding this comment

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

Would a NullPointerException not be more appropriate?

"Can not use NULL as transport"
);
}
this.transport = transport;
}

public Transport getTransport() {
return transport;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,28 @@ public static List<OSCPacketListener> defaultPacketListeners() {
return listeners;
}

/**
* Create an OSC-Port that uses a concrete {@code transport} given
* as parameter and with {@link #isResilient() resilient} set to true.
* One must make sure that the appropriate connection
* has already been set up before using this instance, meaning
* that local and remote address are set correctly.
* @param transport the transport used for receiving OSC packets
* @param packetListeners to handle received and serialized OSC packets
* @throws IOException if we fail to bind a channel to the local address
*/
public OSCPortIn(
final Transport transport,
final List<OSCPacketListener> packetListeners)
{
super(transport);

this.listening = false;
this.daemonListener = true;
this.resilient = true;
this.packetListeners = packetListeners;
}

/**
* Create an OSC-Port that listens on the given local socket for packets from {@code remote},
* using a parser created with the given factory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class OSCPortInBuilder {
private SocketAddress local;
private SocketAddress remote;
private NetworkProtocol networkProtocol = NetworkProtocol.UDP;
private Transport transport;

private OSCPacketListener addDefaultPacketListener() {
if (packetListeners == null) {
Expand All @@ -37,6 +38,33 @@ private OSCPacketListener addDefaultPacketListener() {
}

public OSCPortIn build() throws IOException {

if (packetListeners == null) {
addDefaultPacketListener();
}

// If transport is set, other settings cannot be used
if (transport != null) {
if (remote != null) {
throw new IllegalArgumentException(
"Cannot use remote socket address / port in conjunction with transport object.");
}

if (local != null) {
throw new IllegalArgumentException(
"Cannot use local socket address / port in conjunction with transport object.");
}

if (parserBuilder != null) {
throw new IllegalArgumentException(
"Cannot use parserBuilder in conjunction with transport object.");
}

return new OSCPortIn(
transport, packetListeners
);
}

if (local == null) {
throw new IllegalArgumentException(
"Missing local socket address / port.");
Expand All @@ -50,10 +78,6 @@ public OSCPortIn build() throws IOException {
parserBuilder = new OSCSerializerAndParserBuilder();
}

if (packetListeners == null) {
addDefaultPacketListener();
}

return new OSCPortIn(
parserBuilder, packetListeners, local, remote, networkProtocol
);
Expand Down Expand Up @@ -97,6 +121,11 @@ public OSCPortInBuilder setNetworkProtocol(final NetworkProtocol protocol) {
return this;
}

public OSCPortInBuilder setTransport(final Transport networkTransport) {
transport = networkTransport;
Copy link
Owner

Choose a reason for hiding this comment

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

No must, and I see it is the same in the other methods, which is probably why you did it this way, but I' rather do this as this.transport = transport;, vs choosing arbitrary different names for the two variables.
As said.. leaving it this way is totally fine too.

return this;
}

public OSCPortInBuilder setPacketListeners(
final List<OSCPacketListener> listeners)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,19 @@ public OSCPortOut() throws IOException {
this(new InetSocketAddress(InetAddress.getLocalHost(), DEFAULT_SC_OSC_PORT));
}

/**
* Creates an OSC-Port that sends to a remote host from the specified given
* {@code transport} that was previously created using appropriate local
* and remote addresses, network protocol, and serializers.
* @param transport the transport used for sending OSC packets
* @throws IOException if we fail to bind a channel to the local address
*/
public OSCPortOut(
final Transport transport)
{
super(transport);
}

/**
* Converts and sends an OSC packet (message or bundle) to the remote address.
* @param packet the bundle or message to be converted and sent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,32 @@ public class OSCPortOutBuilder {
private SocketAddress remote;
private SocketAddress local;
private NetworkProtocol networkProtocol = NetworkProtocol.UDP;
private Transport transport;

public OSCPortOut build() throws IOException {

// If transport is set, other settings cannot be used
if (transport != null) {
if (remote != null) {
throw new IllegalArgumentException(
"Cannot use remote socket address / port in conjunction with transport.");
}

if (local != null) {
throw new IllegalArgumentException(
"Cannot use local socket address / port in conjunction with transport.");
}

if (serializerBuilder != null) {
throw new IllegalArgumentException(
"Cannot use serializerBuilder in conjunction with transport.");
}

return new OSCPortOut(
transport
);
}

if (remote == null) {
throw new IllegalArgumentException(
"Missing remote socket address / port.");
Expand Down Expand Up @@ -74,4 +98,9 @@ public OSCPortOutBuilder setNetworkProtocol(final NetworkProtocol protocol) {
networkProtocol = protocol;
return this;
}

public OSCPortOutBuilder setTransport(final Transport networkTransport) {
transport = networkTransport;
Copy link
Owner

Choose a reason for hiding this comment

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

(... same here)

return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@

/**
* A {@link Transport} implementation for sending and receiving OSC packets over
* a network via UDP.
* a network via UDP. This implementation uses separate ByteBuffers for sending
* and receiving, making it possible to use the same UDP socket for sending packets
* and listening simultaneously.
*/
public class UDPTransport implements Transport {

Expand All @@ -34,8 +36,8 @@ public class UDPTransport implements Transport {
* incoming datagram data size.
*/
public static final int BUFFER_SIZE = 65507;
private final ByteBuffer buffer = ByteBuffer.allocate(BUFFER_SIZE);

private final ByteBuffer sendBuffer = ByteBuffer.allocate(BUFFER_SIZE);
private final ByteBuffer receiveBuffer = ByteBuffer.allocate(BUFFER_SIZE);
private final SocketAddress local;
private final SocketAddress remote;
private final DatagramChannel channel;
Expand Down Expand Up @@ -131,12 +133,12 @@ public void close() throws IOException {

@Override
public void send(final OSCPacket packet) throws IOException, OSCSerializeException {
oscChannel.send(buffer, packet, remote);
oscChannel.send(sendBuffer, packet, remote);
}

@Override
public OSCPacket receive() throws IOException, OSCParseException {
return oscChannel.read(buffer);
return oscChannel.read(receiveBuffer);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,44 @@ public void testSocketAutoClose() throws Exception {
.build();
}

@Test
public void testUseSameTransportForOutAndIn() throws Exception {

if (sender != null) {
sender.close();
}

sender = new OSCPortOutBuilder()
.setRemoteSocketAddress(new InetSocketAddress(4711))
.setLocalSocketAddress(new InetSocketAddress( 4711))
.setNetworkProtocol(NetworkProtocol.UDP)
.build();

// create new receiver based on sender's transport implementation
receiver = new OSCPortInBuilder()
.setTransport(sender.getTransport())
.build();

Assert.assertEquals("expected same transport for both sender and receiver",
sender.getTransport(), receiver.getTransport());

// exchange simple message
OSCMessage message = new OSCMessage("/message/receiving");
SimpleOSCMessageListener listener = new SimpleOSCMessageListener();
receiver.getDispatcher().addListener(new OSCPatternAddressMessageSelector("/message/receiving"),
listener);

receiver.startListening();
sender.send(message);

// as the socket for sending is the same as for receiving, we will just produce an echo here
assertMessageReceived(listener, WAIT_FOR_RECEIVE_MS);

// manually close here before tearDown()
receiver.close();
sender.close();
}

private void assertMessageReceived(
SimpleOSCMessageListener listener, int timeout) {
assertEventuallyTrue(
Expand Down