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

command line parameters #3

Open
fsuarezo opened this issue Jan 6, 2023 · 8 comments
Open

command line parameters #3

fsuarezo opened this issue Jan 6, 2023 · 8 comments

Comments

@fsuarezo
Copy link

fsuarezo commented Jan 6, 2023

We are working on a Multicast environment testing products and we are trying to test systems and automate those tests.

MC-test works great but it can go further by adding command line arguments to just "fire it and watch".

A simple -s for "send" or -r for "receive", -i for "interface" (like eth0, ens2, whatever) would be nice.
Or a simpler and better way: "-r eth0", "-s ens5", "-r 10.2.5.29",...
For example:
Sender: mc-test -s eth0
Receiver: mc-test -r ens6

@enclave-marc-barry
Copy link
Contributor

Agreed. Are you planning to raise a PR?

@fsuarezo
Copy link
Author

fsuarezo commented Jan 9, 2023

No, I'm not a developer, sorry. I'm only hoping that someone can do the trick.

@gochan21
Copy link

how can I change the multicast ip address?

@enclave-marc-barry
Copy link
Contributor

@gochan21 README.md has the answer: "By default, the tool uses multicast stream IP address 239.0.1.2 with port 20480. Pull requests to enable customisation are welcome."

@groupmsl
Copy link
Contributor

groupmsl commented Jan 4, 2024

I've created a pull request #7 to allow the user to change the multicast address and port. It doesn't go as far as accepting command line arguments (yet)

@enclave-marc-barry
Copy link
Contributor

That's great, thank you for raising the PR. I think the validation code is a more complicated than it needs to be. Could you make it a little easier to understand by using TryPrase() instead? Something like..

while (true)
{
    Console.Write("Enter multicast address to use [239.0.1.2]: ");

    if (IPAddress.TryParse(Console.ReadLine(), out IPAddress multicastAddress))
    {
        if (IsMulticast(multicastAddress))
        {
            MulticastAddress = multicastAddress;
            break;
        }

        Console.WriteLine("A multicast IP addresses must be between 224.0.0.0 to 239.255.255.255.");
    }
    else
    {
        if (string.IsNullOrEmpty(multicastAddress)) 
        {
            break; // use default multicast address
        }

        Console.WriteLine("Not a valid IP address.");
    }
}

I like that you're checking to see if the address is in the multi-cast range, but perhaps a helper method?

private static bool IsMulticast(IPAddress ip)
{
    byte firstOctet = ip.GetAddressBytes()[0];
    return firstOctet >= 224 && firstOctet <= 239;
}

I don't understand the need for <Nullable>enable</Nullable>?

@groupmsl
Copy link
Contributor

groupmsl commented Jan 5, 2024

I have made changes similar to these and updated the PR. I like what you did as it also allows for the user to use the default multicast address and port if not specified.

RE: nullable, it was in the default .csprj created with a dotnet create console, but (as you hinted) it isn't required and removes a bunch of warnings when disabled. I have done this in the latest commit.

@enclave-marc-barry
Copy link
Contributor

Thanks @groupmsl. That PR won't close this issue for CLI-based config options, but introducing the ability to change multicast address and port I'm sure will be helpful to anyone else using the tool 👍

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

No branches or pull requests

4 participants