-
-
Notifications
You must be signed in to change notification settings - Fork 79
Minimal NetDaemon #1315
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
base: main
Are you sure you want to change the base?
Minimal NetDaemon #1315
Conversation
3a031c7
to
9a7487f
Compare
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.
Pull Request Overview
This PR introduces a "Minimal NetDaemon" implementation that allows simple console applications to connect to Home Assistant without the full NetDaemon runtime. The main addition is a new HaContextFactory
that provides a simplified entry point for creating IHaContext
instances.
Key changes:
- Added a new
HaContextFactory
class with a staticCreateAsync
method for simple HA connections - Refactored cache initialization to accept
IHomeAssistantConnection
directly instead of relying onIHomeAssistantRunner
- Added new overloaded connection methods in
HomeAssistantClientConnector
to support WebSocket URL strings - Created a debug console client project to demonstrate the minimal usage pattern
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
src/HassModel/NetDaemon.HassModel/HaContextFactory.cs | New factory class providing simplified IHaContext creation for console apps |
src/HassModel/NetDaemon.HassModel/ICacheManager.cs | Updated interface to accept IHomeAssistantConnection parameter |
src/HassModel/NetDaemon.HassModel/Internal/*.cs | Refactored cache classes to use direct connection instead of runner dependency |
src/Client/NetDaemon.HassClient/Common/HomeAssistantClientConnector.cs | Added overloaded methods for WebSocket URL connection |
src/debug/ConsoleClient/* | New debug project demonstrating minimal NetDaemon usage |
NetDaemon.sln | Added new ConsoleClient project to solution |
src/debug/ConsoleClient/Program.cs
Outdated
@@ -0,0 +1,6 @@ | |||
//#:package NetDaemon.HassModel@25.18.1 | |||
|
|||
var ha = await NetDaemon.HassModel.HaContextFactory.CreateAsync("ws://localhost:8123/api/websocket", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiI4MDhlZjQ3NWRlOTU0YWJmYTYwNTRkZDc2YzRkZmJjNiIsImlhdCI6MTc0NjkxMTkxOSwiZXhwIjoyMDYyMjcxOTE5fQ.KSwYw1IER965EUcN2_7XPPgVikeIli-mTH8XreveWvA"); |
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.
A hardcoded JWT token is exposed in the source code. This token provides access to Home Assistant and should be removed from the codebase. Consider using environment variables, configuration files, or user secrets to store sensitive tokens.
var ha = await NetDaemon.HassModel.HaContextFactory.CreateAsync("ws://localhost:8123/api/websocket", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiI4MDhlZjQ3NWRlOTU0YWJmYTYwNTRkZDc2YzRkZmJjNiIsImlhdCI6MTc0NjkxMTkxOSwiZXhwIjoyMDYyMjcxOTE5fQ.KSwYw1IER965EUcN2_7XPPgVikeIli-mTH8XreveWvA"); | |
var token = Environment.GetEnvironmentVariable("HOME_ASSISTANT_JWT_TOKEN"); | |
if (string.IsNullOrEmpty(token)) | |
{ | |
Console.WriteLine("Error: HOME_ASSISTANT_JWT_TOKEN environment variable is not set."); | |
return; | |
} | |
var ha = await NetDaemon.HassModel.HaContextFactory.CreateAsync("ws://localhost:8123/api/websocket", token); |
Copilot uses AI. Check for mistakes.
/// </summary> | ||
/// <remarks>Do not use this in NetDaemon apps. Apps should use UseNetDaemonRuntime and resolve IHaContext via dependency injection</remarks> | ||
/// <param name="homeAssistantWebsocketUrl">The Websocket Url to HomeAssistant, eg: ws://localhost:8123/api/websocket</param> | ||
/// <param name="token">The long lived accestoken for Home Assitant</param> |
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.
There are spelling errors in the parameter documentation: 'accestoken' should be 'access token' and 'Assitant' should be 'Assistant'.
/// <param name="token">The long lived accestoken for Home Assitant</param> | |
/// <param name="token">The long-lived access token for Home Assistant</param> |
Copilot uses AI. Check for mistakes.
} | ||
|
||
public async Task<JsonElement?> CallServiceWithResponseAsync(string domain, string service, ServiceTarget? target = null, object? data = null) | ||
{ | ||
ObjectDisposedException.ThrowIf(_isDisposed, this); | ||
_ = _hassRunner.CurrentConnection ?? throw new InvalidOperationException("No connection to Home Assistant"); | ||
_ = _homeAssistantConnection ?? throw new InvalidOperationException("No connection to Home Assistant"); |
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.
The null check is redundant because _homeAssistantConnection is set in the constructor and cannot be null. This check should be removed as it's unreachable code.
_ = _homeAssistantConnection ?? throw new InvalidOperationException("No connection to Home Assistant"); |
Copilot uses AI. Check for mistakes.
@@ -101,7 +97,7 @@ public IObservable<StateChange> StateAllChanges() | |||
public void SendEvent(string eventType, object? data = null) | |||
{ | |||
ObjectDisposedException.ThrowIf(_isDisposed, this); | |||
_backgroundTaskTracker.TrackBackgroundTask(_apiManager.SendEventAsync(eventType, _tokenSource.Token, data), "Error in sending event"); | |||
_backgroundTaskTracker.TrackBackgroundTask(_homeAssistantConnection?.SendEventAsync(eventType, _tokenSource.Token, data), "Error in sending event"); |
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.
The null-conditional operator (?.) is unnecessary here since _homeAssistantConnection cannot be null after constructor initialization. This could also cause a null Task to be passed to TrackBackgroundTask which may cause issues.
_backgroundTaskTracker.TrackBackgroundTask(_homeAssistantConnection?.SendEventAsync(eventType, _tokenSource.Token, data), "Error in sending event"); | |
_backgroundTaskTracker.TrackBackgroundTask(_homeAssistantConnection.SendEventAsync(eventType, _tokenSource.Token, data), "Error in sending event"); |
Copilot uses AI. Check for mistakes.
/// </summary> | ||
public static async Task<IHomeAssistantConnection> ConnectClientAsync(Uri websocketUrl, string token, CancellationToken cancelToken) | ||
{ | ||
return await ConnectClientAsync(websocketUrl.Host, websocketUrl.Port, websocketUrl.Scheme is "https" or "wss", token, websocketUrl.PathAndQuery, cancelToken); |
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.
The SSL detection logic is incorrect. WebSocket URLs use 'ws' and 'wss' schemes, not 'https'. The condition should only check for 'wss' to determine SSL usage.
return await ConnectClientAsync(websocketUrl.Host, websocketUrl.Port, websocketUrl.Scheme is "https" or "wss", token, websocketUrl.PathAndQuery, cancelToken); | |
return await ConnectClientAsync(websocketUrl.Host, websocketUrl.Port, websocketUrl.Scheme == "wss", token, websocketUrl.PathAndQuery, cancelToken); |
Copilot uses AI. Check for mistakes.
7d0b317
to
0420fb9
Compare
|
||
_backgroundTaskTracker.TrackBackgroundTask(_hassRunner.CurrentConnection.CallServiceAsync(domain, service, data, target.Map(), _tokenSource.Token), "Error in sending event"); | ||
_backgroundTaskTracker.TrackBackgroundTask(_homeAssistantConnection.CallServiceAsync(domain, service, data, target.Map(), _tokenSource.Token), "Error in sending event"); |
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.
This line will break the restart logic. The current connection is always just that. Or am I missing something?
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.
When we reconnect, we dispose current apps and new apps get a new IHaContext with the new connection. A scenario where this would change current behavior is if somehow you still use this IHaContext after the reconnect. At that point the haContext itself is disposed, and some operations will fail anyways.
Breaking change
Proposed change
Type of change
Additional information
Checklist
If user exposed functionality or configuration are added/changed: