Merge pull request #173 from PepperDash/feature/fix-devMan-threading

Update DeviceManager to provide protections for multiple threads or reentrancy

PepperDashEssentials/ControlSystem.cs

@@ -26,6 +26,8 @@ namespace PepperDash.Essentials
     {
         HttpLogoServer LogoServer;
 
+        private CTimer _startTimer;
+        private const long StartupTime = 500;
 
         public ControlSystem()
             : base()
@@ -39,6 +41,11 @@ namespace PepperDash.Essentials
         /// Entry point for the program
         /// </summary>
         public override void InitializeSystem()
+        {
+            _startTimer = new CTimer(StartSystem,StartupTime);
+        }
+
+        private void StartSystem(object obj)
         {
             DeterminePlatform();
 

essentials-framework/Essentials Core/PepperDashEssentialsBase/Devices/DeviceManager.cs

@@ -5,10 +5,6 @@ using System.Text;
 using System.Text.RegularExpressions;
 using Crestron.SimplSharp;
 using Crestron.SimplSharpPro;
-using Crestron.SimplSharpPro.DeviceSupport;
-using Crestron.SimplSharpPro.EthernetCommunication;
-using Crestron.SimplSharpPro.UI;
-using Crestron.SimplSharp.Reflection;
 
 using PepperDash.Core;
 
@@ -17,18 +13,23 @@ namespace PepperDash.Essentials.Core
 {
 	public static class DeviceManager
 	{
+	    private static readonly CCriticalSection DeviceCriticalSection = new CCriticalSection();
+	    private static readonly CEvent AllowAddDevicesCEvent = new CEvent(false, true);
 		//public static List<Device> Devices { get { return _Devices; } }
 		//static List<Device> _Devices = new List<Device>();
 
-		static Dictionary<string, IKeyed> Devices = new Dictionary<string, IKeyed>(StringComparer.OrdinalIgnoreCase);
+		static readonly Dictionary<string, IKeyed> Devices = new Dictionary<string, IKeyed>(StringComparer.OrdinalIgnoreCase);
 
 		/// <summary>
 		/// Returns a copy of all the devices in a list
 		/// </summary>
 		public static List<IKeyed> AllDevices { get { return new List<IKeyed>(Devices.Values); } }
 
+	    public static bool AddDeviceEnabled;
+
 		public static void Initialize(CrestronControlSystem cs)
 		{
+		    AddDeviceEnabled = true;
 			CrestronConsole.AddNewConsoleCommand(ListDeviceCommStatuses, "devcommstatus", "Lists the communication status of all devices",
 				ConsoleAccessLevelEnum.AccessOperator);
 			CrestronConsole.AddNewConsoleCommand(ListDeviceFeedbacks, "devfb", "Lists current feedbacks", 
@@ -37,18 +38,9 @@ namespace PepperDash.Essentials.Core
 				ConsoleAccessLevelEnum.AccessOperator);
 			CrestronConsole.AddNewConsoleCommand(DeviceJsonApi.DoDeviceActionWithJson, "devjson", "", 
 				ConsoleAccessLevelEnum.AccessOperator);
-			CrestronConsole.AddNewConsoleCommand(s =>
+			CrestronConsole.AddNewConsoleCommand(s => CrestronConsole.ConsoleCommandResponse(DeviceJsonApi.GetProperties(s)), "devprops", "", ConsoleAccessLevelEnum.AccessOperator);
-			{
+			CrestronConsole.AddNewConsoleCommand(s => CrestronConsole.ConsoleCommandResponse(DeviceJsonApi.GetMethods(s)), "devmethods", "", ConsoleAccessLevelEnum.AccessOperator);
-				CrestronConsole.ConsoleCommandResponse(DeviceJsonApi.GetProperties(s));
+			CrestronConsole.AddNewConsoleCommand(s => CrestronConsole.ConsoleCommandResponse(DeviceJsonApi.GetApiMethods(s)), "apimethods", "", ConsoleAccessLevelEnum.AccessOperator);
-			}, "devprops", "", ConsoleAccessLevelEnum.AccessOperator);
-			CrestronConsole.AddNewConsoleCommand(s =>
-			{
-				CrestronConsole.ConsoleCommandResponse(DeviceJsonApi.GetMethods(s));
-			}, "devmethods", "", ConsoleAccessLevelEnum.AccessOperator);
-			CrestronConsole.AddNewConsoleCommand(s =>
-			{
-				CrestronConsole.ConsoleCommandResponse(DeviceJsonApi.GetApiMethods(s));
-			}, "apimethods", "", ConsoleAccessLevelEnum.AccessOperator);
             CrestronConsole.AddNewConsoleCommand(SimulateComReceiveOnDevice, "devsimreceive",
                 "Simulates incoming data on a com device", ConsoleAccessLevelEnum.AccessOperator);
 		}
@@ -58,6 +50,10 @@ namespace PepperDash.Essentials.Core
 		/// </summary>
 		public static void ActivateAll()
 		{
+		    try
+		    {
+		        DeviceCriticalSection.Enter();
+                AddDeviceEnabled = false;
 		        // PreActivate all devices
 		        foreach (var d in Devices.Values)
 		        {
@@ -100,16 +96,28 @@ namespace PepperDash.Essentials.Core
 		            }
 		        }
 		    }
+		    finally
+		    {
+                DeviceCriticalSection.Leave();
+		    }
+		}
 
 		/// <summary>
 		/// Calls activate on all Device class items
 		/// </summary>
 		public static void DeactivateAll()
 		{
-			foreach (var d in Devices.Values)
+		    try
 		    {
-				if (d is Device)
+		        DeviceCriticalSection.Enter();
-					(d as Device).Deactivate();
+		        foreach (var d in Devices.Values.OfType<Device>())
+		        {
+		            d.Deactivate();
+		        }
+		    }
+		    finally
+		    {
+		        DeviceCriticalSection.Leave();
 		    }
 		}
 
@@ -134,9 +142,9 @@ namespace PepperDash.Essentials.Core
 		//    }
 		//}
 
-		static void ListDevices(string s)
+	    private static void ListDevices(string s)
 	    {
-			Debug.Console(0, "{0} Devices registered with Device Mangager:",Devices.Count);
+	        Debug.Console(0, "{0} Devices registered with Device Manager:", Devices.Count);
 	        var sorted = Devices.Values.ToList();
 	        sorted.Sort((a, b) => a.Key.CompareTo(b.Key));
 
@@ -147,7 +155,7 @@ namespace PepperDash.Essentials.Core
 	        }
 	    }
 
-		static void ListDeviceFeedbacks(string devKey)
+	    private static void ListDeviceFeedbacks(string devKey)
 	    {
 	        var dev = GetDeviceForKey(devKey);
 	        if (dev == null)
@@ -175,13 +183,13 @@ namespace PepperDash.Essentials.Core
         //    Debug.Console(0, "This needs to be reworked.  Stay tuned.", devKey);
         //}
 
-		static void ListDeviceCommStatuses(string input)
+	    private static void ListDeviceCommStatuses(string input)
 	    {
 	        var sb = new StringBuilder();
-			foreach (var dev in Devices.Values)
+	        foreach (var dev in Devices.Values.OfType<ICommunicationMonitor>())
 	        {
-				if (dev is ICommunicationMonitor)
+	            sb.Append(string.Format("{0}: {1}\r", dev,
-					sb.Append(string.Format("{0}: {1}\r", dev.Key, (dev as ICommunicationMonitor).CommunicationMonitor.Status));
+	                dev.CommunicationMonitor.Status));
 	        }
 	        CrestronConsole.ConsoleCommandResponse(sb.ToString());
 	    }
@@ -194,10 +202,24 @@ namespace PepperDash.Essentials.Core
 
 		public static void AddDevice(IKeyed newDev)
 		{
+		    try
+		    {
+		        if (!DeviceCriticalSection.TryEnter())
+		        {
+		            Debug.Console(0, Debug.ErrorLogLevel.Error, "Currently unable to add devices to Device Manager. Please try again");
+		            return;
+		        }
 		        // Check for device with same key
 		        //var existingDevice = _Devices.FirstOrDefault(d => d.Key.Equals(newDev.Key, StringComparison.OrdinalIgnoreCase));
 		        ////// If it exists, remove or warn??
 		        //if (existingDevice != null)
+
+		        if (!AddDeviceEnabled)
+		        {
+		            Debug.Console(0, Debug.ErrorLogLevel.Error, "All devices have been activated. Adding new devices is not allowed.");
+		            return;
+		        }
+
 		        if (Devices.ContainsKey(newDev.Key))
 		        {
 		            Debug.Console(0, newDev, "WARNING: A device with this key already exists.  Not added to manager");
@@ -207,9 +229,17 @@ namespace PepperDash.Essentials.Core
 		        //if (!(_Devices.Contains(newDev)))
 		        //    _Devices.Add(newDev);
 		    }
+		    finally
+		    {
+		        DeviceCriticalSection.Leave();
+		    }
+		}
 
 		public static void RemoveDevice(IKeyed newDev)
 		{
+		    try
+		    {
+                DeviceCriticalSection.Enter();
 		        if (newDev == null)
 		            return;
 		        if (Devices.ContainsKey(newDev.Key))
@@ -219,6 +249,11 @@ namespace PepperDash.Essentials.Core
 		        else
 		            Debug.Console(0, "Device manager: Device '{0}' does not exist in manager.  Cannot remove", newDev.Key);
 		    }
+		    finally
+		    {
+		        DeviceCriticalSection.Leave();
+		    }
+		}
 
 		public static IEnumerable<string> GetDeviceKeys()
 		{
@@ -256,7 +291,7 @@ namespace PepperDash.Essentials.Core
             }
             //Debug.Console(2, "**** {0} - {1} ****", match.Groups[1].Value, match.Groups[2].Value);
 
-            ComPortController com = GetDeviceForKey(match.Groups[1].Value) as ComPortController;
+            var com = GetDeviceForKey(match.Groups[1].Value) as ComPortController;
             if (com == null)
             {
                 CrestronConsole.ConsoleCommandResponse("'{0}' is not a comm port device", match.Groups[1].Value);