Skip to content

Commit

Permalink
Refactors (#979)
Browse files Browse the repository at this point in the history
* Primary ctrs and some warnings fixed

* Adding timeout
  • Loading branch information
helto4real committed Oct 29, 2023
1 parent c5d971d commit 25460d7
Show file tree
Hide file tree
Showing 18 changed files with 114 additions and 186 deletions.
10 changes: 9 additions & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,12 @@ dotnet_diagnostic.CA1034.severity = none
#Static holder types should be Static or NotInheritable
dotnet_diagnostic.CA1052.severity = none
#Do not raise reserved exception types
dotnet_diagnostic.CA2201.severity = none
dotnet_diagnostic.CA2201.severity = none

dotnet_diagnostic.CA1854.severity = none
#The enum member 'Itself' has the same constant value
dotnet_diagnostic.CA1069.severity = none
#Do not create task without passing TaskScheduler
dotnet_diagnostic.CA2008.severity = none
#Use concrete types
dotnet_diagnostic.CA1859.severity = none
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

namespace NetDaemon.AppModel.Internal.AppAssemblyProviders;

/// <summary>
/// Provides interface for applications residing in assemblies
/// </summary>
public interface IAppAssemblyProvider
{
/// <summary>
/// Gets the assembly that has the NetDaemon applications
/// </summary>
public Assembly GetAppAssembly();
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace NetDaemon.AppModel.Internal;

internal sealed class ApplicationContext
internal sealed class ApplicationContext : IAsyncDisposable
{
private readonly CancellationTokenSource _cancelTokenSource = new();
private readonly IServiceScope? _serviceScope;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,21 +67,21 @@ public async Task TestTaskCompleteWithTimeoutShouldLogWarning()
private static async Task<HassMessage> SomeSuccessfulResult()
{
// Simulate som time
await Task.Delay(100);
await Task.Delay(400);
return new HassMessage {Success = true};
}

private static async Task<HassMessage> SomeUnSuccessfulResult()
{
// Simulate som time
await Task.Delay(100);
await Task.Delay(400);
return new HassMessage {Success = false};
}

private static async Task<HassMessage> SomeUnSuccessfulResultThrowsException()
{
// Simulate som time
await Task.Delay(100);
await Task.Delay(400);
throw new InvalidOperationException("Ohh noooo!");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace NetDaemon.Client.Internal.Exceptions;

[SuppressMessage("", "RCS1194")]
public class HomeAssistantApiCallException : ApplicationException
public class HomeAssistantApiCallException : Exception
{
public HttpStatusCode Code { get; private set; }
public HomeAssistantApiCallException(string? message, HttpStatusCode code) : base(message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,6 @@ public async ValueTask DisposeAsync()
{
await Task.WhenAny( Task.WhenAll(_backgroundTasks.Keys), Task.Delay(TimeSpan.FromSeconds(5))).ConfigureAwait(false);
}
_tokenSource.Dispose();
}
}
36 changes: 11 additions & 25 deletions src/Client/NetDaemon.HassClient/Internal/HomeAssistantClient.cs
Original file line number Diff line number Diff line change
@@ -1,25 +1,11 @@
namespace NetDaemon.Client.Internal;

internal class HomeAssistantClient : IHomeAssistantClient
{
private readonly IHomeAssistantConnectionFactory _connectionFactory;
private readonly ILogger<IHomeAssistantClient> _logger;
private readonly IWebSocketClientTransportPipelineFactory _transportPipelineFactory;
private readonly IWebSocketClientFactory _webSocketClientFactory;

public HomeAssistantClient(
ILogger<IHomeAssistantClient> logger,
internal class HomeAssistantClient(ILogger<IHomeAssistantClient> logger,
IWebSocketClientFactory webSocketClientFactory,
IWebSocketClientTransportPipelineFactory transportPipelineFactory,
IHomeAssistantConnectionFactory connectionFactory
)
{
_logger = logger;
_webSocketClientFactory = webSocketClientFactory;
_transportPipelineFactory = transportPipelineFactory;
_connectionFactory = connectionFactory;
}

IHomeAssistantConnectionFactory connectionFactory)
: IHomeAssistantClient
{
public Task<IHomeAssistantConnection> ConnectAsync(string host, int port, bool ssl, string token,
CancellationToken cancelToken)
{
Expand All @@ -31,14 +17,14 @@ IHomeAssistantConnectionFactory connectionFactory
CancellationToken cancelToken)
{
var websocketUri = GetHomeAssistantWebSocketUri(host, port, ssl, websocketPath);
_logger.LogDebug("Connecting to Home Assistant websocket on {Path}", websocketUri);
var ws = _webSocketClientFactory.New();
logger.LogDebug("Connecting to Home Assistant websocket on {Path}", websocketUri);
var ws = webSocketClientFactory.New();

try
{
await ws.ConnectAsync(websocketUri, cancelToken).ConfigureAwait(false);

var transportPipeline = _transportPipelineFactory.New(ws);
var transportPipeline = transportPipelineFactory.New(ws);

var hassVersionInfo = await HandleAuthorizationSequenceAndReturnHassVersionInfo(token, transportPipeline, cancelToken).ConfigureAwait(false);

Expand All @@ -47,20 +33,20 @@ IHomeAssistantConnectionFactory connectionFactory
await AddCoalesceSupport(transportPipeline, cancelToken).ConfigureAwait(false);
}

var connection = _connectionFactory.New(transportPipeline);
var connection = connectionFactory.New(transportPipeline);

if (await CheckIfRunning(connection, cancelToken).ConfigureAwait(false)) return connection;
await connection.DisposeAsync().ConfigureAwait(false);
throw new HomeAssistantConnectionException(DisconnectReason.NotReady);
}
catch (OperationCanceledException)
{
_logger.LogDebug("Connect to Home Assistant was cancelled");
logger.LogDebug("Connect to Home Assistant was cancelled");

Check warning on line 44 in src/Client/NetDaemon.HassClient/Internal/HomeAssistantClient.cs

View check run for this annotation

Codecov / codecov/patch

src/Client/NetDaemon.HassClient/Internal/HomeAssistantClient.cs#L44

Added line #L44 was not covered by tests
throw;
}
catch (Exception e)
{
_logger.LogDebug(e, "Error connecting to Home Assistant");
logger.LogDebug(e, "Error connecting to Home Assistant");
throw;
}
}
Expand Down Expand Up @@ -111,7 +97,7 @@ private static async Task<bool> CheckIfRunning(IHomeAssistantConnection connecti
var connectTimeoutTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancelToken);
connectTimeoutTokenSource.CancelAfter(5000);
// Begin the authorization sequence
// Expect 'auth_required'
// Expect 'auth_required'
var msg = await transportPipeline.GetNextMessagesAsync<HassMessage>(connectTimeoutTokenSource.Token)
.ConfigureAwait(false);
if (msg[0].Type != "auth_required")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,14 @@ public async Task WaitForConnectionToCloseAsync(CancellationToken cancelToken)
where T : CommandMessage
{
var resultMessageTask = await SendCommandAsyncInternal(command, cancelToken);
var result = await resultMessageTask.WaitAsync(TimeSpan.FromMilliseconds(WaitForResultTimeout), cancelToken);

var awaitedTask = await Task.WhenAny(resultMessageTask, Task.Delay(WaitForResultTimeout, cancelToken));

if (awaitedTask != resultMessageTask)
// We have a timeout
throw new InvalidOperationException(
$"Send command ({command.Type}) did not get response in timely fashion. Sent command is {command.ToJsonElement()}");

if (resultMessageTask.Result.Success ?? false)
return resultMessageTask.Result;
if (result.Success ?? false)
return result;

// Non successful command should throw exception
throw new InvalidOperationException(
$"Failed command ({command.Type}) error: {resultMessageTask.Result.Error}. Sent command is {command.ToJsonElement()}");
$"Failed command ({command.Type}) error: {result.Error}. Sent command is {command.ToJsonElement()}");
}

public async ValueTask DisposeAsync()
Expand All @@ -148,7 +142,7 @@ public async ValueTask DisposeAsync()
}

if (!_internalCancelSource.IsCancellationRequested)
_internalCancelSource.Cancel();
await _internalCancelSource.CancelAsync();

// Gracefully wait for task or timeout
await Task.WhenAny(
Expand All @@ -158,6 +152,9 @@ public async ValueTask DisposeAsync()

await _transportPipeline.DisposeAsync().ConfigureAwait(false);
_internalCancelSource.Dispose();
_hassMessageSubject.Dispose();
await _resultMessageHandler.DisposeAsync();
_messageIdSemaphore.Dispose();
}

public Task<T?> GetApiCallAsync<T>(string apiPath, CancellationToken cancelToken)
Expand Down Expand Up @@ -231,7 +228,7 @@ private async Task HandleNewMessages()
_logger.LogTrace("Stop processing new messages");
// make sure we always cancel any blocking operations
if (!_internalCancelSource.IsCancellationRequested)
_internalCancelSource.Cancel();
await _internalCancelSource.CancelAsync();
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,10 @@
namespace NetDaemon.Client.Internal;

internal class HomeAssistantConnectionFactory : IHomeAssistantConnectionFactory
internal class HomeAssistantConnectionFactory(ILogger<IHomeAssistantConnection> logger,
IHomeAssistantApiManager apiManager) : IHomeAssistantConnectionFactory
{
private readonly IHomeAssistantApiManager _apiManager;
private readonly ILogger<IHomeAssistantConnection> _logger;

public HomeAssistantConnectionFactory(
ILogger<IHomeAssistantConnection> logger,
IHomeAssistantApiManager apiManager
)
{
_logger = logger;
_apiManager = apiManager;
}

public IHomeAssistantConnection New(IWebSocketClientTransportPipeline transportPipeline)
{
return new HomeAssistantConnection(_logger, transportPipeline, _apiManager);
return new HomeAssistantConnection(logger, transportPipeline, apiManager);
}
}
27 changes: 8 additions & 19 deletions src/Client/NetDaemon.HassClient/Internal/HomeAssistantRunner.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
namespace NetDaemon.Client.Internal;

internal class HomeAssistantRunner : IHomeAssistantRunner
internal class HomeAssistantRunner(IHomeAssistantClient client,
ILogger<IHomeAssistantRunner> logger) : IHomeAssistantRunner
{
private readonly IHomeAssistantClient _client;

// The internal token source will make sure we
// always cancel operations on dispose
private readonly CancellationTokenSource _internalTokenSource = new();
Expand All @@ -14,16 +13,6 @@ internal class HomeAssistantRunner : IHomeAssistantRunner

private Task? _runTask;

public HomeAssistantRunner(
IHomeAssistantClient client,
ILogger<IHomeAssistantRunner> logger
)
{
_client = client;
_logger = logger;
}

private readonly ILogger<IHomeAssistantRunner> _logger;
public IObservable<IHomeAssistantConnection> OnConnect => _onConnectSubject;
public IObservable<DisconnectReason> OnDisconnect => _onDisconnectSubject;
public IHomeAssistantConnection? CurrentConnection { get; internal set; }
Expand All @@ -44,7 +33,7 @@ public async ValueTask DisposeAsync()
{
if (_isDisposed)
return;
_internalTokenSource.Cancel();
await _internalTokenSource.CancelAsync();

if (_runTask?.IsCompleted == false)
try
Expand Down Expand Up @@ -73,7 +62,7 @@ public async ValueTask DisposeAsync()
{
try
{
CurrentConnection = await _client.ConnectAsync(host, port, ssl, token, websocketPath, combinedToken.Token)
CurrentConnection = await client.ConnectAsync(host, port, ssl, token, websocketPath, combinedToken.Token)
.ConfigureAwait(false);
// Start the event processing before publish the connection
var eventsTask = CurrentConnection.WaitForConnectionToCloseAsync(combinedToken.Token);
Expand All @@ -82,14 +71,14 @@ public async ValueTask DisposeAsync()
}
catch (HomeAssistantConnectionException de) when (de.Reason == DisconnectReason.Unauthorized)
{
_logger.LogError("User token unauthorized! Will not retry connecting...");
logger.LogError("User token unauthorized! Will not retry connecting...");
await DisposeConnectionAsync();
_onDisconnectSubject.OnNext(DisconnectReason.Unauthorized);
return;
}
catch (HomeAssistantConnectionException de) when (de.Reason == DisconnectReason.NotReady)
{
_logger.LogInformation("Home Assistant is not ready yet!");
logger.LogInformation("Home Assistant is not ready yet!");
await DisposeConnectionAsync();
_onDisconnectSubject.OnNext(DisconnectReason.NotReady);
}
Expand All @@ -109,13 +98,13 @@ public async ValueTask DisposeAsync()
}
catch (Exception e)
{
_logger.LogError(e, "Error running HassClient");
logger.LogError(e, "Error running HassClient");
await DisposeConnectionAsync();
_onDisconnectSubject.OnNext(DisconnectReason.Error);
}

await DisposeConnectionAsync();
_logger.LogInformation("Client disconnected, retrying in {Seconds} seconds...", timeout.TotalSeconds);
logger.LogInformation("Client disconnected, retrying in {Seconds} seconds...", timeout.TotalSeconds);
await Task.Delay(timeout, combinedToken.Token).ConfigureAwait(false);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,6 @@ public Task ConnectAsync(Uri uri, CancellationToken cancel)
await Task.FromException(new NotImplementedException()).ConfigureAwait(false);
}

public Task<WebSocketReceiveResult> ReceiveAsync(ArraySegment<byte> buffer,
CancellationToken cancellationToken)
{
return Task.FromException<WebSocketReceiveResult>(new NotImplementedException());
}

public ValueTask<ValueWebSocketReceiveResult> ReceiveAsync(Memory<byte> buffer,
CancellationToken cancellationToken)
{
Expand All @@ -76,4 +70,4 @@ public ValueTask DisposeAsync()
_ws.Dispose();
return ValueTask.CompletedTask;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public async ValueTask DisposeAsync()
}

await _ws.DisposeAsync().ConfigureAwait(false);
_internalCancelSource.Dispose();
}

public async ValueTask<T[]> GetNextMessagesAsync<T>(CancellationToken cancelToken) where T : class
Expand All @@ -57,7 +58,7 @@ public async ValueTask DisposeAsync()
try
{
// First we start the serialization task that will process
// the pipeline for new data written from websocket input
// the pipeline for new data written from websocket input
// We want the processing to start before we read data
// from the websocket so the pipeline is not getting full
var serializeTask = ReadMessagesFromPipelineAndSerializeAsync<T>(combinedTokenSource.Token);
Expand Down Expand Up @@ -163,14 +164,14 @@ private async Task ReadMessageFromWebSocketAndWriteToPipelineAsync(CancellationT
await SendCorrectCloseFrameToRemoteWebSocket().ConfigureAwait(false);

// Cancel so the write thread is canceled before pipe is complete
_internalCancelSource.Cancel();
await _internalCancelSource.CancelAsync();
}
}
}
finally
{
// We have successfully read the whole message,
// make available to reader
// We have successfully read the whole message,
// make available to reader
// even if failure or we cannot reset the pipe
await _pipe.Writer.CompleteAsync().ConfigureAwait(false);
}
Expand Down Expand Up @@ -215,7 +216,7 @@ await _ws.CloseOutputAsync(WebSocketCloseStatus.NormalClosure, "Closing", timeou
}
case WebSocketState.Open:
{
// Do full close
// Do full close
await _ws.CloseAsync(WebSocketCloseStatus.NormalClosure, "Closing", timeout.Token)
.ConfigureAwait(false);
if (_ws.State != WebSocketState.Closed)
Expand All @@ -233,7 +234,7 @@ await _ws.CloseAsync(WebSocketCloseStatus.NormalClosure, "Closing", timeout.Toke
// After the websocket is properly closed
// we can safely cancel all actions
if (!_internalCancelSource.IsCancellationRequested)
_internalCancelSource.Cancel();
await _internalCancelSource.CancelAsync();
}
}
}
}
Loading

0 comments on commit 25460d7

Please sign in to comment.