-
-
Notifications
You must be signed in to change notification settings - Fork 175
Added mDNS advertising for simple LAN access to UI #738
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
Conversation
|
Gooday, interesting request. Personally there are times when i'm running the application on the same network on two different computers (main pc where im developing + one lower specced machine which where i can run longer session to gather data). For this specific use case i would assume i would ran into dns name collision in the same network. Which at that point having a unique name in the same network seems rather an obstacle then a benefit. Secondly Makaretu.Dns.Multicast v0.27.0 a 7 years old. Also some notices about the PR in general:
Missing using - Null safety in _serviceDiscovery?.Unadvertise(_serviceProfile); // _serviceProfile could be null
|
|
Totally! This came from helping out a friend who didn't have a bunch of context about networking and trying to explain how to reach the UI from another computer in their house. I'll get that IP caching in, that makes total sense. The System.Collections.Generic was removed via Intellisense, I'll throw it in the using properly. And I'll get that null safety check, that was just me working too fast. Thanks for the review on it and I'll get the code up shortly! |
|
I had time to think about it, how about making this feature guarded by a feature flag ? like a command line argument ? |
|
That's actually what I was working on setting up as we speak, the system by default probes for wowbot.local and won't advertise if it already exists. By default if you are running one instance you get wowbot.local. I changed the batch back to localhost to prevent it not working if there is a conflict. And the part I am working on currently is making a command line argument you can use with the dotnet run call to pick a custom mDNS name to be advertised. So if you wanted you could have a fleet all advertising unique DNS names based on whatever you had put in your run.bat or on the cmd. |
…as better built in methods.
…ods. Added MDNS_HOSTNAME env var to allow for custom naming
|
Ok! Got the caching, null safety, custom mDNS names, and documentation handled, also moved to a fork of the mdns library that is significantly newer/up to date. |
|
|
||
| // Cached IP addresses - refreshed when network interfaces change | ||
| private IPAddress[] _cachedAddresses = []; | ||
| private readonly object _addressLock = new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since .NET 9 introduced System.Threading.Lock it's the recommended approach now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented as requested 49c876c
|
After reading more about the mDNS topic in Windows there's a requirement I'm aware that many people are still running the "project" under a Virtual machine where even Windows ?7? could be installed 👀 |
|
|
||
| private static int GetServerPort() | ||
| { | ||
| var urls = Environment.GetEnvironmentVariable("ASPNETCORE_URLS"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't account for launchSettings.json or appsettings.json Kestrel config.
Recommendation
using Microsoft.AspNetCore.Hosting.Server;
using Microsoft.AspNetCore.Hosting.Server.Features;public sealed class MdnsAdvertisingService : IHostedService, IDisposable
{
private readonly ILogger<MdnsAdvertisingService> _logger;
private readonly IServer _server;
private readonly IHostApplicationLifetime _lifetime;
private readonly string _hostname;
private int _port = 5000; // fallback
private CancellationTokenRegistration _startedRegistration;
public MdnsAdvertisingService(
ILogger<MdnsAdvertisingService> logger,
IServer server,
IHostApplicationLifetime lifetime)
{
_logger = logger;
_server = server;
_lifetime = lifetime;
_hostname = Environment.GetEnvironmentVariable("MDNS_HOSTNAME") ?? "wowbot";
}
public Task StartAsync(CancellationToken cancellationToken)
{
// Server addresses aren't available until after Kestrel binds
// Wait for ApplicationStarted event
_startedRegistration = _lifetime.ApplicationStarted.Register(() =>
{
var addressFeature = _server.Features.Get<IServerAddressesFeature>();
if (addressFeature != null)
{
foreach (var address in addressFeature.Addresses)
{
if (Uri.TryCreate(address, UriKind.Absolute, out var uri))
{
_port = uri.Port;
_logger.LogDebug("mDNS: Detected server port {Port} from {Address}", _port, address);
break;
}
}
}
// Now start advertising with the correct port
StartAdvertising();
});
return Task.CompletedTask;
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented as requested in 57b0511
BlazorServer/Program.cs
Outdated
| }); | ||
|
|
||
| // Register mDNS advertising service for http://wowbot.local access | ||
| services.AddHostedService<MdnsAdvertisingService>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice when the feature is not used, then the service its won't be registered in the Dependency Injection container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added USE_MDNS env var check, if it is not null it will load the service
d69eac7
|
Thanks, its shaping better and better, i have no more nitpicks, i like the capability, however i do not want this to be the default behaviour of accessing the BlazorServer portal, however those who want to opt it can do so! From my end this is acceptable, i will squash and merge! Thank you for the contribution! 🙇 |
Added basic mDNS advertising to the BlazorServer setup.
UI will now advertise on the LAN at http://wowbot.local:5000.
This makes the prompt properly pop up from Windows to allow access to the program and makes accessing the UI significantly easier from remote devices.