From aeb5e21e7d3c15d679f4f8cc5adc32f4636b3fc5 Mon Sep 17 00:00:00 2001 From: Parzival Date: Sat, 10 Aug 2024 01:55:00 +0200 Subject: [PATCH] Refactor Code (#12) * Added run scripts * Removed obsolete "new" keyword * Removed some obsolete code * Added comment for possible bug when compiling using NativeAOT * Fixed invalid null return on non-nullable type * Removed redundant disposal * Fixed namespace * Refactored class * Refactored class * Refactored class * Fix possible NullPointerException * Removed redundant suppression expressions * Refactored class * Fixed typos * Renamed LogLevel fields * Refactored class * Refactored class * Disabled unused field warning --- .run/Build & Install - Linux.run.xml | 17 ++++++++ .run/Build & Install - Windows.run.xml | 17 ++++++++ FaderSyncPlugin/GoXLR/UtilitySingleton.cs | 6 +-- FaderSyncPlugin/Module.cs | 6 +-- FaderSyncPlugin/OBS/GoXlrChannelSyncFilter.cs | 28 ++++++------ FaderSyncPlugin/OBS/Logger.cs | 16 +++---- FaderSyncPlugin/OBS/Plugin.cs | 13 +++--- UtilityClient/Native/SocketClient.cs | 5 +-- UtilityClient/Native/WebsocketClient.cs | 43 ++++++++----------- UtilityClient/Utility.cs | 24 +++++------ 10 files changed, 102 insertions(+), 73 deletions(-) create mode 100644 .run/Build & Install - Linux.run.xml create mode 100644 .run/Build & Install - Windows.run.xml diff --git a/.run/Build & Install - Linux.run.xml b/.run/Build & Install - Linux.run.xml new file mode 100644 index 0000000..ba0f97d --- /dev/null +++ b/.run/Build & Install - Linux.run.xml @@ -0,0 +1,17 @@ + + + + \ No newline at end of file diff --git a/.run/Build & Install - Windows.run.xml b/.run/Build & Install - Windows.run.xml new file mode 100644 index 0000000..8b0e2ce --- /dev/null +++ b/.run/Build & Install - Windows.run.xml @@ -0,0 +1,17 @@ + + + + \ No newline at end of file diff --git a/FaderSyncPlugin/GoXLR/UtilitySingleton.cs b/FaderSyncPlugin/GoXLR/UtilitySingleton.cs index 4def558..c4d3c17 100644 --- a/FaderSyncPlugin/GoXLR/UtilitySingleton.cs +++ b/FaderSyncPlugin/GoXLR/UtilitySingleton.cs @@ -8,7 +8,7 @@ public static class UtilitySingleton private static readonly Logger Log = new Logger(typeof(Plugin), Module.Name); private static Utility? _utility; - private static Thread? _connectionThread = null; + private static Thread? _connectionThread; private static async void Connect() { @@ -17,7 +17,7 @@ private static async void Connect() if (_utility.IsConnectionAlive()) { - Log.Info($"Connected to GoXLR Utility v{_utility.Status?["config"]?["daemon_version"]}"); + Log.Info($"Connected to GoXLR Utility v{_utility.Status["config"]?["daemon_version"]}"); } else { @@ -30,7 +30,7 @@ public static Utility GetInstance() _utility ??= new Utility(); if (_utility.IsConnectionAlive()) return _utility; - _utility.OnException += (sender, exception) => + _utility.OnException += (_, exception) => { Log.Error($"Something internally went wrong in the GoXLR Utility API Client: {exception.Message}"); }; diff --git a/FaderSyncPlugin/Module.cs b/FaderSyncPlugin/Module.cs index 8e868a0..fdeebdc 100644 --- a/FaderSyncPlugin/Module.cs +++ b/FaderSyncPlugin/Module.cs @@ -4,9 +4,9 @@ namespace FaderSync; public static class Module { - public static readonly string Name = Assembly.GetExecutingAssembly()?.GetName()?.Name!; + public static readonly string Name = Assembly.GetExecutingAssembly().GetName().Name!; - public static readonly string Version = Assembly.GetExecutingAssembly()? - .GetCustomAttribute()?.InformationalVersion! + public static readonly string Version = Assembly.GetExecutingAssembly() + .GetCustomAttribute()?.InformationalVersion .Split('+')[0]!; // remove commit hash } \ No newline at end of file diff --git a/FaderSyncPlugin/OBS/GoXlrChannelSyncFilter.cs b/FaderSyncPlugin/OBS/GoXlrChannelSyncFilter.cs index 57d880e..9b7f174 100644 --- a/FaderSyncPlugin/OBS/GoXlrChannelSyncFilter.cs +++ b/FaderSyncPlugin/OBS/GoXlrChannelSyncFilter.cs @@ -90,7 +90,7 @@ private static unsafe void Tick(void* data, float seconds) var channelName = Marshal.PtrToStringUTF8((IntPtr)context->ChannelName); var target = Obs.obs_filter_get_parent(context->Source); - var systemVolume = utility.Status?["mixers"]?[deviceSerial ?? ""]?["levels"]?["volumes"]?[channelName ?? ""]? + var systemVolume = utility.Status["mixers"]?[deviceSerial ?? ""]?["levels"]?["volumes"]?[channelName ?? ""]? .GetValue() ?? 0; // Ok, the GoXLR seems to decrease the volume by 1dB for every (on average) 4.85 volume steps, it @@ -106,12 +106,12 @@ private static unsafe void Tick(void* data, float seconds) utilityBase += count * 0.115f; } - // Now we convert this into a OBS value... + // Now we convert this into an OBS value... var obsVolume = (float)Math.Pow(10, -utilityBase / 20f); // check if channel is muted var isMuted = false; - var faderStatus = (JsonObject)utility.Status?["mixers"]?[deviceSerial ?? ""]?["fader_status"]; + var faderStatus = (JsonObject?)utility.Status["mixers"]?[deviceSerial ?? ""]?["fader_status"]; if (faderStatus != null) foreach (var faderEntry in faderStatus) { @@ -134,7 +134,7 @@ private static unsafe void Tick(void* data, float seconds) [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })] private static unsafe void GetDefaults(obs_data* settings) { - // Todo: implement + // do nothing } [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })] @@ -186,7 +186,7 @@ private static unsafe void GetDefaults(obs_data* settings) sChannelLineOutId = "LineOut"u8.ToArray() ) { - // Create the Serial Dropdown.. + // Create the Serial Dropdown... var deviceList = ObsProperties.obs_properties_add_list(properties, (sbyte*)sDeviceSerialId, (sbyte*)sDeviceSerialDescription, obs_combo_type.OBS_COMBO_TYPE_LIST, obs_combo_format.OBS_COMBO_FORMAT_STRING); @@ -206,9 +206,9 @@ private static unsafe void GetDefaults(obs_data* settings) ObsProperties.obs_property_list_add_string(channelList, (sbyte*)sChannelMicMonitor, (sbyte*)sChannelMicMonitorId); ObsProperties.obs_property_list_add_string(channelList, (sbyte*)sChannelLineOut, (sbyte*)sChannelLineOutId); - // Before we Proceed, we need to fetch a list of the available GoXLRs on the System.. + // Before we Proceed, we need to fetch a list of the available GoXLRs on the System... var utility = UtilitySingleton.GetInstance(); - var mixers = (JsonObject)utility.Status?["mixers"]; + var mixers = (JsonObject?)utility.Status["mixers"]; var locatedDevices = new ArrayList(); var forcedSerial = false; @@ -222,9 +222,9 @@ private static unsafe void GetDefaults(obs_data* settings) // Get an initial count of devices which we'll use for stuff later! var locatedDeviceCount = locatedDevices.Count; - // If the user has perviously configured a GoXLR but it's not currently attached to the Utility, we need to + // If the user has previously configured a GoXLR, but it's not currently attached to the Utility, we need to // force the serial into the list to prevent arbitrary device switching later on. We'll also flag this as a - // forced entry so we can appropriately label it. + // forced entry, so we can appropriately label it. if (serial != "" && !locatedDevices.Contains(serial)) { locatedDevices.Add(serial); forcedSerial = true; @@ -232,12 +232,12 @@ private static unsafe void GetDefaults(obs_data* settings) if (locatedDevices.Count == 0) { // We're in some kind of error state. Either the utility connection is broken or there are no GoXLRs attached, and the - // user hasn't previously defined a GoXLR. In this case we'll forcably add the 'Error' serial to the list so we can + // user hasn't previously defined a GoXLR. In this case we'll forcibly add the 'Error' serial to the list, so we can // display the problem to the user in the drop-down. locatedDevices.Add(deviceSerialError); } - // Start filling out the list.. + // Start filling out the list... foreach (var located in locatedDevices) { fixed (byte* sSerial = Encoding.UTF8.GetBytes((string)located)) { if (located.Equals(deviceSerialError) && mixers == null) { @@ -254,8 +254,8 @@ private static unsafe void GetDefaults(obs_data* settings) // Has this device been forced into the located list due to it being disconnected? if (forcedSerial && located.Equals(serial)) { - // We can do a *LOT* better than this and potentially check WHY it's disconnected.. - title = String.Format("{0} - Disconnected", located); + // We can do a *LOT* better than this and potentially check WHY it's disconnected... + title = $"{located} - Disconnected"; } fixed(byte* sTitle = Encoding.UTF8.GetBytes(title)) { ObsProperties.obs_property_list_add_string(deviceList, (sbyte*)sTitle, (sbyte*)sSerial); @@ -283,6 +283,7 @@ public static unsafe void Update(void* data, obs_data* settings) } } +#pragma warning disable CS0649 // Field is never assigned to, and will always have its default value private unsafe struct FilterContext { public obs_source* Source; @@ -292,4 +293,5 @@ private unsafe struct FilterContext public sbyte* DeviceSerial; public sbyte* ChannelName; } +#pragma warning restore CS0649 // Field is never assigned to, and will always have its default value } \ No newline at end of file diff --git a/FaderSyncPlugin/OBS/Logger.cs b/FaderSyncPlugin/OBS/Logger.cs index 9095b5a..cde85de 100644 --- a/FaderSyncPlugin/OBS/Logger.cs +++ b/FaderSyncPlugin/OBS/Logger.cs @@ -10,10 +10,10 @@ public class Logger public enum LogLevel { - ERROR = ObsBase.LOG_ERROR, - WARNING = ObsBase.LOG_WARNING, - INFO = ObsBase.LOG_INFO, - DEBUG = ObsBase.LOG_DEBUG, + Error = ObsBase.LOG_ERROR, + Warning = ObsBase.LOG_WARNING, + Info = ObsBase.LOG_INFO, + Debug = ObsBase.LOG_DEBUG, } private static unsafe void Log(LogLevel level, string text) @@ -40,8 +40,8 @@ public Logger(MemberInfo loggerClass) _className = loggerClass.Name; } - public void Error(string message) => Log(LogLevel.ERROR, $"<{_className}> {message}"); - public void Warning(string message) => Log(LogLevel.WARNING, $"<{_className}> {message}"); - public void Info(string message) => Log(LogLevel.INFO, $"<{_className}> {message}"); - public void Debug(string message) => Log(LogLevel.DEBUG, $"<{_className}> {message}"); + public void Error(string message) => Log(LogLevel.Error, $"<{_className}> {message}"); + public void Warning(string message) => Log(LogLevel.Warning, $"<{_className}> {message}"); + public void Info(string message) => Log(LogLevel.Info, $"<{_className}> {message}"); + public void Debug(string message) => Log(LogLevel.Debug, $"<{_className}> {message}"); } \ No newline at end of file diff --git a/FaderSyncPlugin/OBS/Plugin.cs b/FaderSyncPlugin/OBS/Plugin.cs index cfb45e2..7b495a1 100644 --- a/FaderSyncPlugin/OBS/Plugin.cs +++ b/FaderSyncPlugin/OBS/Plugin.cs @@ -1,5 +1,4 @@ -using System.Reflection; -using System.Runtime.InteropServices; +using System.Runtime.InteropServices; using FaderSync.GoXLR; using ObsInterop; @@ -13,7 +12,7 @@ public static class Plugin CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] public static unsafe void obs_module_set_pointer(obs_module* obsModulePointer) { - + // do nothing, needs to exist for OBS to load } [UnmanagedCallersOnly(EntryPoint = "obs_module_ver", @@ -29,7 +28,7 @@ public static uint obs_module_ver() [UnmanagedCallersOnly(EntryPoint = "obs_module_load", CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] - public static unsafe bool obs_module_load() + public static bool obs_module_load() { Log.Info($"Loading {Module.Name} v{Module.Version}"); @@ -62,14 +61,14 @@ public static void obs_module_unload() CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] public static unsafe void obs_module_set_locale(char* locale) { - + // TODO: add locale support } [UnmanagedCallersOnly(EntryPoint = "obs_module_free_locale", CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] - public static unsafe void obs_module_free_locale() + public static void obs_module_free_locale() { - + // do nothing, needs to exist for OBS to load } } } \ No newline at end of file diff --git a/UtilityClient/Native/SocketClient.cs b/UtilityClient/Native/SocketClient.cs index a2a4164..40cffb3 100644 --- a/UtilityClient/Native/SocketClient.cs +++ b/UtilityClient/Native/SocketClient.cs @@ -1,7 +1,7 @@ using System.IO.Pipes; using System.Text; -namespace GoXLRUtilityClient.client; +namespace GoXLRUtilityClient.Native; public class SocketClient : IDisposable { @@ -41,7 +41,7 @@ public string ReadMessage() // read message length byte[] lengthBytes; try { lengthBytes = _reader.ReadBytes(4); } - catch (IOException) { return null; } + catch (IOException) { return ""; } if (BitConverter.IsLittleEndian) Array.Reverse(lengthBytes); var messageLength = BitConverter.ToUInt32(lengthBytes); @@ -65,6 +65,5 @@ public void Dispose() _reader.Dispose(); _writer.Dispose(); - _client.Dispose(); } } \ No newline at end of file diff --git a/UtilityClient/Native/WebsocketClient.cs b/UtilityClient/Native/WebsocketClient.cs index 01fdbf9..e4ad5a3 100644 --- a/UtilityClient/Native/WebsocketClient.cs +++ b/UtilityClient/Native/WebsocketClient.cs @@ -1,17 +1,17 @@ using System.Net.WebSockets; using System.Text; -namespace GoXLRUtilityClient.client; +namespace GoXLRUtilityClient.Native; public class WebsocketClient : IDisposable { private ClientWebSocket _client = new(); private CancellationTokenSource _cancellationTokenSource = new(); - private Task _receiveMessageTask; - private Task _connectionTask; + private Task? _receiveMessageTask; + private Task? _connectionTask; - private bool _isConnected = false; + private bool _isConnected; public event EventHandler? OnConnected; public event EventHandler? OnDisconnected; @@ -31,7 +31,7 @@ private async Task ReceiveMessageTask() continue; } - WebSocketReceiveResult result = null; + WebSocketReceiveResult? result = null; try { result = await _client.ReceiveAsync(new ArraySegment(buffer), @@ -54,11 +54,10 @@ private async Task ReceiveMessageTask() { var message = Encoding.UTF8.GetString(buffer, 0, result.Count); this.OnMessage?.Invoke(this, message); - break; } else { - memoryStream.Write(buffer, 0, result.Count); + await memoryStream.WriteAsync(buffer, 0, result.Count); if (result.EndOfMessage) { var message = Encoding.UTF8.GetString(memoryStream.GetBuffer(), 0, (int)memoryStream.Position); @@ -73,12 +72,11 @@ private async Task ReceiveMessageTask() hasReceivedCloseMessage = true; // If the server initiates the close handshake, handle it - Task.Run(DisconnectAsync); + await Task.Run(DisconnectAsync); break; - case WebSocketMessageType.Binary: default: - // This wont occur with the Utility, but we need to trigger DisconnectAsync to perform tidying up! + // This won't occur with the Utility, but we need to trigger DisconnectAsync to perform tidying up! await _client.CloseAsync(WebSocketCloseStatus.ProtocolError, "Only Text is supported.", CancellationToken.None); this.OnDisconnected?.Invoke(this, "Connection closed because server tried to send binary or invalid message."); break; @@ -100,10 +98,7 @@ private async Task ConnectionTask() case WebSocketState.Aborted: case WebSocketState.Closed: // Trigger an internal disconnect to clean resources. - Task.Run(DisconnectAsync); - break; - - default: + await Task.Run(DisconnectAsync); break; } @@ -121,7 +116,7 @@ public async Task ConnectAsync(string uri) return await this.ConnectAsync(new Uri(uri)); } - public async Task ConnectAsync(Uri uri) + protected async Task ConnectAsync(Uri uri) { this._receiveMessageTask = ReceiveMessageTask(); this._connectionTask = ConnectionTask(); @@ -129,7 +124,7 @@ public async Task ConnectAsync(Uri uri) return this._client.State == WebSocketState.Open; } - public async Task DisconnectAsync() + protected async Task DisconnectAsync() { // Only attempt to close the socket if it's not already closed if (this._client.State != WebSocketState.Aborted && this._client.State != WebSocketState.Closed) { @@ -137,8 +132,8 @@ public async Task DisconnectAsync() } this.OnDisconnected?.Invoke(this, "Connection closed."); - this._cancellationTokenSource.Cancel(); - var shutdownSuccessful = Task.WaitAll(new[] { this._receiveMessageTask, this._connectionTask }, + await _cancellationTokenSource.CancelAsync(); + var shutdownSuccessful = Task.WaitAll(new[] { this._receiveMessageTask!, this._connectionTask! }, TimeSpan.FromSeconds(5)); if (!shutdownSuccessful) { @@ -146,8 +141,8 @@ public async Task DisconnectAsync() return; } - this._receiveMessageTask.Dispose(); - this._connectionTask.Dispose(); + this._receiveMessageTask!.Dispose(); + this._connectionTask!.Dispose(); // Dispose of, and create a new client / Token for future connections this._client.Dispose(); @@ -159,7 +154,7 @@ public async Task DisconnectAsync() this._isConnected = false; } - public async Task SendMessage(string message) + protected async Task SendMessage(string message) { await this._client.SendAsync( new ArraySegment(Encoding.UTF8.GetBytes(message)), @@ -172,7 +167,7 @@ public void Dispose() { this._cancellationTokenSource.Cancel(); - var shutdownSuccessful = Task.WaitAll(new[] { this._receiveMessageTask, this._connectionTask }, + var shutdownSuccessful = Task.WaitAll(new[] { this._receiveMessageTask!, this._connectionTask! }, TimeSpan.FromSeconds(5)); if (!shutdownSuccessful) { @@ -182,7 +177,7 @@ public void Dispose() this._cancellationTokenSource.Dispose(); this._client.Dispose(); - this._receiveMessageTask.Dispose(); - this._connectionTask.Dispose(); + this._receiveMessageTask!.Dispose(); + this._connectionTask!.Dispose(); } } \ No newline at end of file diff --git a/UtilityClient/Utility.cs b/UtilityClient/Utility.cs index 0d3d7a8..50404be 100644 --- a/UtilityClient/Utility.cs +++ b/UtilityClient/Utility.cs @@ -1,6 +1,6 @@ using System.Text.Json; using System.Text.Json.Nodes; -using GoXLRUtilityClient.client; +using GoXLRUtilityClient.Native; using Json.Patch; namespace GoXLRUtilityClient; @@ -13,23 +13,23 @@ public class Utility : WebsocketClient public JsonNode Status; - public Utility() : base() + public Utility() { this.Status = JsonNode.Parse("{}")!; // basic message handler - base.OnMessage += (object? sender, string message) => + OnMessage += (_, message) => { try { var jsonNode = JsonNode.Parse(message); // test if message is valid - if (jsonNode!["id"] == null || jsonNode!["data"] == null) return; + if (jsonNode!["id"] == null || jsonNode["data"] == null) return; - var isPatchMessage = jsonNode!["data"]!["Patch"] != null; - var id = jsonNode!["id"]!.GetValue(); - var data = jsonNode!["data"]!; + var isPatchMessage = jsonNode["data"]!["Patch"] != null; + var id = jsonNode["id"]!.GetValue(); + var data = jsonNode["data"]!; this.OnMessageReceived?.Invoke(this, new MessageData(id, data, isPatchMessage)); @@ -37,7 +37,7 @@ public Utility() : base() if (isPatchMessage) { var patchString = data["Patch"]!.ToJsonString(); - var patch = JsonSerializer.Deserialize(patchString); + var patch = JsonSerializer.Deserialize(patchString); // FIXME: can break functionality when AOT compiling var resultResult = patch?.Apply(this.Status); this.Status = resultResult!.Result!; } @@ -48,7 +48,7 @@ public Utility() : base() } // nothing }; - base.OnDisconnected += async (object? sender, string message) => { + OnDisconnected += (_, message) => { Console.WriteLine("Disconnected from Host: {0}", message); // Reset the status, so Upstream code knows we're not connected. @@ -90,7 +90,7 @@ void TempMessageHandler(object? _, MessageData message) } } - public new async Task ConnectAsync() + public async Task ConnectAsync() { String pipeName = OperatingSystem.IsWindows() ? "@goxlr.socket" : "/tmp/goxlr.socket"; SocketClient socketClient = new SocketClient(pipeName); @@ -127,8 +127,8 @@ void TempMessageHandler(object? _, MessageData message) // request status var statusRequest = JsonNode.Parse("{}"); statusRequest!["id"] = requestId; - statusRequest!["data"] = "GetStatus"; - await base.SendMessage(statusRequest.ToJsonString()); + statusRequest["data"] = "GetStatus"; + await SendMessage(statusRequest.ToJsonString()); var result = await this.AwaitResponse(requestId); if (result != null) this.Status = result["Status"]!;