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

Allow setting source address #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hadleyrich
Copy link

@hadleyrich hadleyrich commented Aug 15, 2023

I needed to set the source address to broadcast on a specific interface. This allowed me to do so, not sure if it's of use to anyone else but I thought I'd contribute in case you wanted it :)

Summary by CodeRabbit

  • New Feature: Added the ability to specify a source_address in the StupidArtnet class. This allows users to define a specific network interface for sending Art-Net packets, providing greater control over packet routing. The socket option SO_REUSEADDR is set when a source address is provided, enabling the reuse of socket addresses.

@cpvalente
Copy link
Owner

cpvalente commented Aug 16, 2023

Hey @hadleyrich, really good work, and thank you for making the PR

It looks like I might need to upgrade the python version in CI to see the tests, meanwhile would you mind offering a small description of the feature either in code or in the readme?

Can you also rebase master so that the test suite can run?

@cpvalente cpvalente marked this pull request as draft October 3, 2023 06:46
@cpvalente cpvalente marked this pull request as ready for review October 3, 2023 06:46
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2023

Walkthrough

The change introduces a new source_address parameter to the StupidArtnet class's initializer. This allows clients to specify a network interface for sending Art-Net packets by setting the SO_REUSEADDR socket option and binding the socket to the provided address.

Changes

File Summary
stupidArtnet/StupidArtnet.py Introduced a source_address parameter to the __init__ method of the StupidArtnet class. If provided, the socket binds to this address, enabling the use of a specific network interface for sending Art-Net packets.

🐇💻

In the land of code, changes are afoot,

A rabbit hops, leaving tracks in the soot.

With a tweak here, and a tweak there,

The packets find their way, through the digital air.

So raise your carrots high, let's give a cheer,

For the code is clear, and the path is near! 🥕🎉


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 3e1b203 and ce4091c.
Files selected for processing (1)
  • stupidArtnet/StupidArtnet.py (2 hunks)
Additional comments (Suppressed): 2
stupidArtnet/StupidArtnet.py (2)
  • 22-23: The function signature of __init__ has been changed to include a new parameter source_address. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify if the default value of None for source_address is appropriate in all use cases.

  • 60-63: The socket option SO_REUSEADDR is set and the socket is bound to the source address if it is provided. This allows the client to use a specific network interface for sending Art-Net packets. However, be aware that setting SO_REUSEADDR can potentially lead to some security risks as it allows multiple sockets on the same host to bind to the same IP address and port. Make sure you understand the implications of this change and consider whether it's necessary in your context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants