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

libevent udp ip multicast implementation #1806

Merged
merged 4 commits into from Jul 8, 2017

Conversation

Projects
None yet
4 participants
@skoppe
Contributor

skoppe commented Jun 30, 2017

Adds necessary functions on UdpConnection to control basic IP multicast.

I am working on getting a PR into druntime to get rid of the ugly conditional imports.

Show outdated Hide outdated core/vibe/core/net.d
@@ -434,6 +434,24 @@ interface UDPConnection {
ubyte[] recv(ubyte[] buf = null, NetworkAddress* peer_address = null);
/// ditto
ubyte[] recv(Duration timeout, ubyte[] buf = null, NetworkAddress* peer_address = null);
version (VibeLibeventDriver) {

This comment has been minimized.

@s-ludwig

s-ludwig Jul 1, 2017

Member

Instead of making the interface specific to libevent, the other drivers should just have simple stubs with assert(false, "TODO!"); inside. It would also be good to update the vibe-core package accordingly, but I can do that, too.

@s-ludwig

s-ludwig Jul 1, 2017

Member

Instead of making the interface specific to libevent, the other drivers should just have simple stubs with assert(false, "TODO!"); inside. It would also be good to update the vibe-core package accordingly, but I can do that, too.

This comment has been minimized.

@skoppe

skoppe Jul 1, 2017

Contributor

Thought about it as well. Will adjust.

@skoppe

skoppe Jul 1, 2017

Contributor

Thought about it as well. Will adjust.

Show outdated Hide outdated core/vibe/core/drivers/libevent2.d
@@ -1086,6 +1103,34 @@ final class Libevent2UDPConnection : UDPConnection {
}
}
void addMembership(string group)

This comment has been minimized.

@s-ludwig

s-ludwig Jul 1, 2017

Member

At least here, the parameter should be of type ref NetworkAddress, so that the name resolution happens outside, and the driver function code can focus on the core functionality. But for convenience, UDPConnection could have a second final overload that accepts a string instead.

@s-ludwig

s-ludwig Jul 1, 2017

Member

At least here, the parameter should be of type ref NetworkAddress, so that the name resolution happens outside, and the driver function code can focus on the core functionality. But for convenience, UDPConnection could have a second final overload that accepts a string instead.

This comment has been minimized.

@skoppe

skoppe Jul 1, 2017

Contributor

Makes sense. Do you want me to squash and force update? Or create additional commit?

@skoppe

skoppe Jul 1, 2017

Contributor

Makes sense. Do you want me to squash and force update? Or create additional commit?

This comment has been minimized.

@s-ludwig

s-ludwig Jul 1, 2017

Member

An additional commit is okay (I'm happy as long as the history stays reasonably readable).

@s-ludwig

s-ludwig Jul 1, 2017

Member

An additional commit is okay (I'm happy as long as the history stays reasonably readable).

@AppVeyorBot

This comment has been minimized.

Show comment
Hide comment
@skoppe

This comment has been minimized.

Show comment
Hide comment
@skoppe

skoppe Jul 6, 2017

Contributor

Still haven't had time to finish this. It is on my radar though.

Contributor

skoppe commented Jul 6, 2017

Still haven't had time to finish this. It is on my radar though.

skoppe and others added some commits Jun 25, 2017

@s-ludwig s-ludwig added the auto-merge label Jul 8, 2017

s-ludwig added a commit to vibe-d/vibe-core that referenced this pull request Jul 8, 2017

Add UDP multicast declarations matching vibe-d/vibe.d#1806.
The actual functionality still needs to be implemented in eventcore.

@dlang-bot dlang-bot merged commit 6e25d0b into vibe-d:master Jul 8, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment