Closed git-developer closed 1 year ago
Hello. Thank you for your contribution.
I just have one concern - shouldn't the environment variable have a name that is more specific to the application? SERVER_PORT seems too generic for me (might be set for something else, and sdgyrodsu
would use it unknowingly to the user). Please correct me if I'm wrong. Otherwise, please change the name to something more specific (e.g. SDGYRO_SERVER_PORT).
Thanks for your feedback! I agree that a more specific name reduces the chance for name conflicts for the user at runtime, so I've changed it as requested.
Is there anything else I can do, @kmicki?
Sorry for the wait. Just had a chance to test the changes.
There is a problem. The port that application uses is different than set in the env variable. I think htons
is missing.
Oh, sorry for overseeing that, and thanks for testing. You're right, 26671 = 0x6889
and 35176 = 0x8968
, so byte swapping by calling htons
should fix it. Committed.
Making the UDP server port configurable simplifies testing and simplifies running other cemuhook providers on the same host. It is probably not very important for most users, but it doesn't break anything either.
To keep the efforts as small as the benefit, an environment variable is used instead of a command line argument.