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
Dump Mono.NAT in favor of Open.NAT 2 #10281
Conversation
|
349d3dd
to
2ce8ce1
Compare
|
Fixed. |
|
This doesn't work for my router. Mono.NAT works fine: |
|
I suspect I am suffering from lontivero/Open.NAT#26 (I have both |
|
The opened bug in lontivero/Open.NAT#26 is because of a HTTP 400 - Bad Request. Is this one because of the same problem? Btw, Mono.Nat raises an event for each device it finds and that means when two are found our event handler is invoked twice, one per device. That means that in many cases it fails with one but it succeeds with the other. Is this one of those cases? |
|
Looks like it is failing on @Mailaender's integration with OpenRA catches the exception that is generated by the first failure and globally disables NAT, so we don't see the results from the second device. I can hack around the problem locally by removing the |
|
So which exceptions exactly should I catch and which not? |
|
Shouldn't I try/catch in general here? |
|
@lontivero: Would you be able to advise on the best way to query the result of GetExternalIPAsync() while ignoring the first error? |
|
Umm i think there was a misunderstanding here. I was talking about Mono.Nat. @pchote said Mono.Nat works okay and for that reason i detailed how it works (it raises an event for each device it finds). Open.NAT doesn't raise events, it returns all the devices it finds before timeout and then you should try with them in a foreach loop. |
2ce8ce1
to
5a5f7ca
Compare
|
Updated to make use of DiscoverDevicesAsync now. |
|
You'll still need a try/catch inside the loops to skip over the errors from the bogus device. |
|
Edit: Looks like this was just my weird network topology. If I connect directly to my router then it works as expected. |
|
Crash when loading the settings menu: |
5a5f7ca
to
06b50d0
Compare
|
Updated. |
06b50d0
to
0ced75f
Compare
|
Not sure if this is async enough, but neither |
|
Not async enough, sorry. Put it in a background thread if you have to, or like I mentioned above at least move it later in the init so it doesn't delay the window creation and load screen display. |
0ced75f
to
8bd4d8e
Compare
|
Rebased and moved it below. I am a bit inexperienced with this async concept. |
|
Made a new attempt #11286 based on this pull request. |
https://github.com/lontivero/Open.NAT/wiki/Why-Open.NAT
Closes #7140.
Fixes #8794.
Depends on #10279.
It also deprecates the
Settings.Server.NatDeviceAvailablehelper variable and always logs intonat.log(so we can get rid ofSettings.Server.VerboseNatDiscoveryas well) as even in verbose mode it is now pretty readable as Open.NAT only searches for NAT devices, not every other UPnP enabled device in the network.