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

Fix SERVER_SRS_HOST_AUTO (Attempt 2) #715

Merged
merged 2 commits into from
Dec 30, 2024
Merged

Conversation

Camble
Copy link
Contributor

@Camble Camble commented Jul 11, 2024

Original PR: #676 This fixes the SERVER_SRS_HOST_AUTO by making use of net.get_server_host() on the client-side.

@ciribob MESSAGE_PATTERN_OLD & MESSAGE_PATTERN_OLDER should now match to obtain the host where no :PORT has been specified.

I've updated the auto-connect message to only send the port if SERVER_SRS_HOST_AUTO is enabled, otherwise it uses the original message with SERVER_SRS_HOST and SERVER_SRS_PORT.

I've removed the isAutoConnectMessage function in favour of regex matching. We try to get the port from the message, if this fails, fallback to matching on IP and Port.

Camble added 2 commits July 11, 2024 21:45
…ect message

The receiver can then use net.get_server_host() to include the IP address of the server
@Camble Camble marked this pull request as ready for review July 11, 2024 20:49
@ciribob
Copy link
Owner

ciribob commented Jul 28, 2024

Thanks for this! As im in a rush for the Apache fix I wont include this as I dont have time to test but i'll add to the next release

this looks great :)

@Camble
Copy link
Contributor Author

Camble commented Jul 28, 2024

Nice one, thanks. No rush. Sorry it took so long to revisit. I don't have access to your test suite or I'd double check everything. Might be worth checking before merging though. Let me know if it needs any tweaks!

@ciribob
Copy link
Owner

ciribob commented Aug 15, 2024

Finally had a bit of time to look at this - i'd still have an override option for configuring the IP as in some cases the IP isnt the same as the DCS host machine. I'd also have an optional one as well for still showing the IP and Port for users that want it (if people have disabled autoconnect)

Then it can in effect be :

  • current behaviour (default)
  • Your new behaviour with a much smaller message and an nice auto IP feature

@Camble
Copy link
Contributor Author

Camble commented Aug 15, 2024

So I poached SERVER_SRS_HOST_AUTO as it wasn't working. I left the existing behaviour in place if this is false. I may be missing something, but this should allow the message to be constructed using SERVER_SRS_PORT and SERVER_SRS_HOST.

Default behaviour is here.

@ciribob
Copy link
Owner

ciribob commented Nov 18, 2024

Sorry for being so slow on this - i'll have a test and get in the next release. Thanks!

@ciribob
Copy link
Owner

ciribob commented Dec 30, 2024

Works perfectly! I'll add this to the next release - thank you!

@ciribob ciribob merged commit a610bb1 into ciribob:master Dec 30, 2024
2 checks passed
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