Merge pull request #1354 from PepperDash/comport-controller-updates

fix: improve logging for COM port registration and configuration

src/PepperDash.Essentials.Core/Comm and IR/ComPortController.cs

@@ -1,7 +1,9 @@
 using System;
 using System.Text;
 using System.Text.RegularExpressions;
+using Crestron.SimplSharp;
 using Crestron.SimplSharpPro;
+using Crestron.SimplSharpPro.GeneralIO;
 using PepperDash.Core;
 using PepperDash.Core.Logging;
 using Serilog.Events;
@@ -85,36 +87,32 @@ namespace PepperDash.Essentials.Core
 		{
 			if (Port == null)
 			{
-				Debug.LogMessage(LogEventLevel.Information, this, "Configured com Port for this device does not exist.");
+				this.LogInformation($"Configured {Port.Parent.GetType().Name}-comport-{Port.ID} for {Key} does not exist.");
 				return;
 			}
-			// TODO [ ] - Remove commented out code once verified working
-			//if (Port.Parent is CrestronControlSystem || Port.Parent is CenIoCom102)
-			if (Port.Parent is GenericBase genericDevice && genericDevice.Registerable)
+
+			var result = Port.Register();
+			if (result != eDeviceRegistrationUnRegistrationResponse.Success)
 			{
-				//this.LogInformation($"INFO: Attempting to register {Key} using {Port.Parent.GetType().Name}-comport-{Port.ID}");
-				var result = genericDevice.Register();
-				if (result != eDeviceRegistrationUnRegistrationResponse.Success)
-				{
-					this.LogError($"ERROR: Cannot register {Key} using {Port.Parent.GetType().Name}-comport-{Port.ID} (result == {result})");
-					return; // false
-				}
+				this.LogError($"Cannot register {Key} using {Port.Parent.GetType().Name}-comport-{Port.ID} (result == {result})");
+				return;
 			}
+			this.LogInformation($"Successfully registered {Key} using {Port.Parent.GetType().Name}-comport-{Port.ID} (result == {result})");
 
 			var specResult = Port.SetComPortSpec(Spec);
 			if (specResult != 0)
 			{
-				this.LogError($"ERROR: Cannot set comspec for {Key} using {Port.Parent.GetType().Name}-comport-{Port.ID} (result == {specResult})");
+				this.LogError($"Cannot set comspec for {Key} using {Port.Parent.GetType().Name}-comport-{Port.ID} (result == {specResult})");
 				return;
 			}
-			//this.LogInformation($"INFO: Successfully set comspec for {Key} using {Port.Parent.GetType().Name}-comport-{Port.ID} (result == {specResult})");
+			this.LogInformation($"Successfully set comspec for {Key} using {Port.Parent.GetType().Name}-comport-{Port.ID} (result == {specResult})");
 
 
 			// TODO [ ] - Remove debug logging once verified working
 			// if (Port.Parent is CenIoCom102)
-			// {				
+			// {
 			// 	Port.PropertyChanged += (s, e) =>
-			// 	{					
+			// 	{
 			// 		this.LogInformation($@"RegisterAndConfigureComPort: PropertyChanged Fired >> 
 			// 			comPort-'{Port.ID}', 
 			// 			Property Changed-'{e.Property}', 

src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs

@@ -31,7 +31,12 @@ namespace PepperDash.Essentials.AppServer.Messengers
         /// <remarks>
         /// Unsoliciited feedback from a device in a messenger will ONLY be sent to devices in this subscription list. When a client disconnects, it's ID will be removed from the collection.
         /// </remarks>
-        protected HashSet<string> SubscriberIds = new HashSet<string>();
+        private readonly HashSet<string> subscriberIds = new HashSet<string>();
+
+        /// <summary>
+        /// Lock object for thread-safe access to SubscriberIds
+        /// </summary>
+        private readonly object _subscriberLock = new object();
 
         private readonly List<string> _deviceInterfaces;
 
@@ -193,14 +198,15 @@ namespace PepperDash.Essentials.AppServer.Messengers
                 return;
             }
 
-            if (SubscriberIds.Any(id => id == clientId))
+            lock (_subscriberLock)
             {
-                this.LogVerbose("Client {clientId} already subscribed", clientId);
-                return;
+                if (!subscriberIds.Add(clientId))
+                {
+                    this.LogVerbose("Client {clientId} already subscribed", clientId);
+                    return;
+                }
             }
 
-            SubscriberIds.Add(clientId);
-
             this.LogDebug("Client {clientId} subscribed", clientId);
         }
 
@@ -216,14 +222,22 @@ namespace PepperDash.Essentials.AppServer.Messengers
                 return;
             }
 
-            if (!SubscriberIds.Any(i => i == clientId))
+            bool wasSubscribed;
+            lock (_subscriberLock)
+            {
+                wasSubscribed = subscriberIds.Contains(clientId);
+                if (wasSubscribed)
+                {
+                    subscriberIds.Remove(clientId);
+                }
+            }
+
+            if (!wasSubscribed)
             {
                 this.LogVerbose("Client with ID {clientId} is not subscribed", clientId);
                 return;
             }
 
-            SubscriberIds.RemoveWhere((i) => i == clientId);
-
             this.LogInformation("Client with ID {clientId} unsubscribed", clientId);
         }
 
@@ -312,7 +326,14 @@ namespace PepperDash.Essentials.AppServer.Messengers
                 // If client is null or empty, this message is unsolicited feedback. Iterate through the subscriber list and send to all interested parties
                 if (string.IsNullOrEmpty(clientId))
                 {
-                    foreach (var client in SubscriberIds)
+                    // Create a snapshot of subscribers to avoid collection modification during iteration
+                    List<string> subscriberSnapshot;
+                    lock (_subscriberLock)
+                    {
+                        subscriberSnapshot = new List<string>(subscriberIds);
+                    }
+
+                    foreach (var client in subscriberSnapshot)
                     {
                         AppServerController?.SendMessageObject(new MobileControlMessage { Type = !string.IsNullOrEmpty(type) ? type : MessagePath, ClientId = client, Content = content });
                     }

src/PepperDash.Essentials.MobileControl/Touchpanel/ThemeMessenger.cs

@@ -40,7 +40,7 @@ namespace PepperDash.Essentials.Touchpanel
                 this.LogInformation("Setting theme to {theme}", theme.Value);
                 _tpDevice.UpdateTheme(theme.Value);
 
-                PostStatusMessage(JToken.FromObject(new { theme = theme.Value }), id);
+                PostStatusMessage(JToken.FromObject(new { theme = theme.Value }), clientId: id);
             });
         }
     }