--- a/ServerMonitor/Objects/ServerMonitor.cs	Tue Apr 30 20:40:58 2019 -0400
+++ b/ServerMonitor/Objects/ServerMonitor.cs	Sat May 25 15:14:26 2019 -0400
@@ -4,11 +4,9 @@
 using ServerMonitorApp.Properties;
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Net.NetworkInformation;
-using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using System.Windows.Forms;
@@ -16,51 +14,74 @@
 
 namespace ServerMonitorApp
 {
+    /// <summary>Central class for scheduling and executing checks against remote servers.</summary>
     public class ServerMonitor
     {
         private readonly string configFileDir;
         private readonly Logger logger;
+        // Cancellation tokens for executing checks, keyed by check ID.
         private readonly Dictionary<int, CancellationTokenSource> tokens = new Dictionary<int, CancellationTokenSource>();
+        // SSH private keys, keyed by the path to the private key file.
+        // A value of NULL indicates that the private key is inaccessible or encrypted.
         private readonly Dictionary<string, PrivateKeyFile> privateKeys = new Dictionary<string, PrivateKeyFile>();
+        // IDs of all checks that have been paused due to network unavailability,
+        // or due to the system being suspended.
+        // Not to be confused with checks that have been disabled by the user.
         private readonly List<int> pausedChecks = new List<int>();
         private bool running, networkAvailable, suspend;
+        // List of check execution tasks that have been started.
+        // A check task begins by sleeping until the next scheduled execution time,
+        // then executes.
         private Dictionary<Task<CheckResult>, int> tasks;
         private ServerSummaryForm mainForm;
 
-        //private List<Task<CheckResult>> tasks;
-
+        /// <summary>Fires when the status of a check changes.</summary>
         public event EventHandler<CheckStatusChangedEventArgs> CheckStatusChanged;
 
+        /// <summary>The collection of registered servers.</summary>
         public List<Server> Servers { get; private set; } = new List<Server>();
 
+        /// <summary>A collection of all checks belonging to all registerd servers.</summary>
         public IEnumerable<Check> Checks => Servers.SelectMany(s => s.Checks);
 
+        /// <summary>Path to the file that stores server and check configuration.</summary>
         public string ConfigFile { get; private set; }
 
+        /// <summary>Path to the file that stores server and check configuration.</summary>
         public IEnumerable<string> LockedKeys { get { return privateKeys.Where(kvp => kvp.Value == null).Select(kvp => kvp.Key); } }
 
+        /// <summary>ServerMonitor constructor.</summary>
+        /// <param name="mainForm">A reference to the main form.</param>
         public ServerMonitor(ServerSummaryForm mainForm)
         {
             this.mainForm = mainForm;
+            // Store configuration in %appdata%\ServerMonitor
             configFileDir = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "ServerMonitor");
             ConfigFile = Path.Combine(configFileDir, "servers.xml");
             logger = new Logger(Path.Combine(configFileDir, "monitor.log"));
         }
 
+        /// <summary>Registers a new server with the server monitor.</summary>
+        /// <param name="server">The server to be added.</param>
         public void AddServer(Server server)
         {
             Servers.Add(server);
             SaveServers();
         }
 
+        /// <summary>Deletes a server from the server monitor.</summary>
+        /// <param name="server">The server to be deleted.</param>
         public void DeleteServer(Server server)
         {
             Servers.Remove(server);
             SaveServers();
         }
 
+        /// <summary>Loads all servers and checks from the config file.</summary>
         public void LoadServers()
         {
+            bool triedBackup = false;
+
             Read:
             TextReader reader = null;
             try
@@ -69,16 +90,23 @@
                 XmlSerializer serializer = CreateXmlSerializer();
                 Servers.Clear();
                 Servers.AddRange((List<Server>)serializer.Deserialize(reader));
-                // Update the Checks so they know what Server they belong to.
-                // Would rather do this in the Server object on deserialization, but
-                // that doesn't work when using the XML serializer for some reason.
+                // Do some more set-up now that the servers and checks have been loaded.
                 foreach (Server server in Servers)
                 {
+                    // Read private keys into memory if they are accessible and not encrypted.
+                    // If PrivateKeyFile != null, it means same the key has already been loaded for
+                    // a different server and nothing more needs to be done.
                     if (server.LoginType == LoginType.PrivateKey && server.PrivateKeyFile == null)
                         OpenPrivateKey(server.KeyFile);
                     foreach (Check check in server.Checks)
                     {
+                        // Update the checks so they know what server they belong to.
+                        // Would rather do this in the Server object on deserialization, but
+                        // that doesn't work when using the XML serializer for some reason.
                         check.Server = server;
+                        // If the program last exited while the check was running, change its status
+                        // to the result of its last execution (since, at this point, the check is
+                        // not running).
                         if (check.Status == CheckStatus.Running)
                             check.Status = check.LastRunStatus;
                     }
@@ -92,9 +120,23 @@
             catch (InvalidOperationException)
             {
                 reader?.Close();
-                File.Copy(ConfigFile, ConfigFile + ".error", true);
-                File.Copy(ConfigFile + ".bak", ConfigFile, true);
-                goto Read;
+                // If there was an error parsing the XML, try again with the backup config file.
+                if (!triedBackup)
+                {
+                    File.Copy(ConfigFile, ConfigFile + ".error", true);
+                    string backupConfig = ConfigFile + ".bak";
+                    if (File.Exists(backupConfig))
+                    {
+                        File.Copy(backupConfig, ConfigFile, true);
+                    }
+                    triedBackup = true;
+                    goto Read;
+                }
+                else
+                {
+                    // If there was an error reading the backup file too, give up.
+                    throw;
+                }
             }
             finally
             {
@@ -103,10 +145,14 @@
             Application.ApplicationExit += Application_ApplicationExit;
             NetworkChange.NetworkAddressChanged += NetworkChange_NetworkAddressChanged;
             SystemEvents.PowerModeChanged += SystemEvents_PowerModeChanged;
+
+            // Remove old entries from the log file according to user preferences.
             logger.TrimLog();
+
             Run();
         }
 
+        /// <summary>Saves all servers and checks to the config file.</summary>
         public void SaveServers()
         {
             GenerateIds();
@@ -114,6 +160,7 @@
             XmlSerializer serializer = null;
             try
             {
+                // Make a backup first in case something goes wrong in the middle of writing.
                 File.Copy(ConfigFile, ConfigFile + ".bak", true);
             }
             catch { }
@@ -125,6 +172,7 @@
             }
             catch (DirectoryNotFoundException)
             {
+                // If the directory does not exist, create it and try again.
                 Directory.CreateDirectory(configFileDir);
                 writer = new StreamWriter(ConfigFile);
                 serializer = CreateXmlSerializer();
@@ -136,11 +184,17 @@
             }
         }
 
+        /// <summary>Main server monitor loop. Schedules and executes checks.</summary>
         private async void Run()
         {
+            // Do not run again if already running or if the system is suspending or resuming.
             if (running || suspend)
                 return;
+
             running = true;
+
+            // If the network is available, immediately execute checks that were supposed to run
+            // earlier but could not due to network unavailability or the system being suspended.
             networkAvailable = Helpers.IsNetworkAvailable();
             if (networkAvailable)
             {
@@ -150,41 +204,110 @@
                 }
                 pausedChecks.Clear();
             }
+
+            // Schedule all checks to run according to their schedules.
+            // Each check will sleep until it is scheduled to run, then execute.
             tasks = Checks.ToDictionary(c => ScheduleExecuteCheckAsync(c), c => c.Id);
             while (tasks.Count > 0)
             {
+                // When any check is done sleeping and executing, remove the completed
+                // task and queue a new task to schedule it again.
                 Task<CheckResult> task = await Task.WhenAny(tasks.Keys);
                 tasks.Remove(task);
                 try
                 {
                     CheckResult result = await task;
-                    // Result will be null if a scheduled check was disabled
+                    // Do not schedule the task again if it is now disabled.
+                    // Result will be null if a scheduled check was disabled.
                     if (result != null && result.CheckStatus != CheckStatus.Disabled)
                         tasks.Add(ScheduleExecuteCheckAsync(result.Check), result.Check.Id);
                 }
                 catch (OperationCanceledException)
                 {
-
+                    // When a server's state changes to Disabled, any checks that are executing
+                    // are immediately cancelled. Silently catch these expected exceptions.
                 }
             }
+            // If there are no enabled checks scheduled, exit the main loop.
+            // It will be restarted when a check or server is enabled.
             running = false;
         }
 
+        /// <summary>Schedules a check to be run on its schedule.</summary>
+        /// <param name="check">The check to execute.</param>
+        /// <returns>The async check result.</returns>
+        private async Task<CheckResult> ScheduleExecuteCheckAsync(Check check)
+        {
+            // Do not schedule or execute the check if it or its server is disabled.
+            if (!check.Enabled || !check.Server.Enabled)
+                return await Task.FromResult(new CheckResult(check, CheckStatus.Disabled, null));
+
+            // Create a cancellation token that will be used to cancel the check if it or
+            // its server is disabled while it is executing.
+            CancellationTokenSource cts = new CancellationTokenSource();
+            tokens[check.Id] = cts;
+
+            // Sleep until next time the check is supposed to be executed.
+            // Use the LastScheduledRunTime so manual executions by the user do not
+            // interfere with the schedule.
+            check.NextRunTime = check.Schedule.GetNextTime(check.LastScheduledRunTime);
+            int delay = Math.Max(0, (int)(check.NextRunTime - DateTime.Now).TotalMilliseconds);
+            await Task.Delay(delay, cts.Token);
+            check.LastScheduledRunTime = check.NextRunTime;
+
+            // Execute the check if not cancelled.
+            if (!cts.IsCancellationRequested)
+            {
+                // If the network is available, execute the check.
+                // Otherwise, add it to the list of paused checks to be executed
+                // when the network becomes available again.
+                if (networkAvailable)
+                {
+                    return await ExecuteCheckAsync(check, cts.Token);
+                }
+                else
+                {
+                    if (!pausedChecks.Contains(check.Id))
+                        pausedChecks.Add(check.Id);
+                }
+            }
+            return await Task.FromResult(new CheckResult(check, CheckStatus.Disabled, null));
+        }
+
+        /// <summary>Executes a check asynchronously.</summary>
+        /// <param name="check">The check to execute.</param>
+        /// <param name="token">A chancellation token that may be used to cancel the check execution.</param>
+        /// <returns>The async check result.</returns>
         public async Task<CheckResult> ExecuteCheckAsync(Check check, CancellationToken token = default(CancellationToken))
         {
+            // Update the status.
             check.Status = CheckStatus.Running;
             OnCheckStatusChanged(check);
+
+            // Execute the check.
             CheckResult result = await check.ExecuteAsync(token);
+
+            // Increment the consecutive failure counter on failue, or reset
+            // the counter on success.
             if (result.Failed)
                 check.ConsecutiveFailures++;
+            else
+                check.ConsecutiveFailures = 0;
+
             OnCheckStatusChanged(check, result);
             HandleResultAsync(result);
             return result;
         }
 
+        /// <summary>Handles the result of a check execution.</summary>
+        /// <param name="result">The result.</param>
         private void HandleResultAsync(CheckResult result)
         {
+            // Log the result.
             logger.Log(result);
+
+            // Notify the user of failure according to user preferences.
+            // If the check succeeded, result.FailAction will be None.
             if (result.Check.ConsecutiveFailures >= result.Check.MaxConsecutiveFailures)
             {
                 if (result.FailAction == FailAction.FlashTaskbar)
@@ -194,31 +317,41 @@
             }
         }
 
+        /// <summary>Reads all check results from the log for a server.</summary>
+        /// <param name="server">The server whose check results should be read.</param>
+        /// <returns>A list of all check results found in the log file for the given server.</returns>
         public IList<CheckResult> GetLog(Server server)
         {
             return logger.Read(server);
         }
 
+        /// <summary>Saves the check settings and notifies event subscribers when the status of a check changes.</summary>
+        /// <param name="check">The check whose status has changed.</param>
+        /// <param name="result">The check result that caused the status to change, if any.</param>
         private void OnCheckStatusChanged(Check check, CheckResult result = null)
         {
             SaveServers();
             CheckStatusChanged?.Invoke(check, new CheckStatusChangedEventArgs(check, result));
         }
 
+        /// <summary>Handles user modifications to a check's settings.</summary>
+        /// <param name="sender">The check that was modified.</param>
         private void Server_CheckModified(object sender, EventArgs e)
         {
             Check check = (Check)sender;
-            Task<CheckResult> task = tasks.FirstOrDefault(kvp => kvp.Value == check.Id).Key;
+
+            // No need to mess with the task queue if not currently running.
             if (running)
             {
+                Task<CheckResult> task = tasks.FirstOrDefault(kvp => kvp.Value == check.Id).Key;
                 if (task == null)
                 {
-                    // No tasks associated with the check, so schedule a new one
+                    // No tasks associated with the check, so schedule a new one.
                     tasks.Add(ScheduleExecuteCheckAsync(check), check.Id);
                 }
                 else
                 {
-                    // Check was modified or deleted, so remove any waiting tasks
+                    // Check was modified or deleted, so remove any waiting tasks.
                     CancelCheck(check);
                     if (check.Server != null)
                     {
@@ -230,19 +363,26 @@
                     }
                 }
             }
-            // Run again in case removing a task above caused it to stop
+            // Run again in case removing a task above caused it to stop.
             Run();
         }
 
+        /// <summary>Handles the enabled state of a server changing.</summary>
+        /// <param name="sender">The server that was enabled or disabled.</param>
         private void Server_EnabledChanged(object sender, EventArgs e)
         {
             Server server = (Server)sender;
+
+            // Make sure the monitor is running. If no servers were enabled before this
+            // one was enabled, it is not running.
             if (server.Enabled)
             {
                 Run();
             }
             else
             {
+                // Cancel all queued and executing checks belonging to a
+                // server that was disabled.
                 foreach (Check check in server.Checks)
                 {
                     CancelCheck(check);
@@ -250,50 +390,42 @@
             }
         }
 
+        /// <summary>Cancels a check that may be executing.</summary>
+        /// <param name="check">The check to cancel.</param>
         private void CancelCheck(Check check)
         {
             if (tasks == null)
                 return;
+
+            // Find the waiting or executing task for the check and remove it.
             Task<CheckResult> task = tasks.FirstOrDefault(kvp => kvp.Value == check.Id).Key;
             if (task != null)
                 tasks.Remove(task);
+
+            // Remove it from the list of paused checks so it doesn't get restarted later.
             pausedChecks.RemoveAll(id => id == check.Id);
+
+            // Cancel the current execution.
             if (tokens.TryGetValue(check.Id, out CancellationTokenSource cts))
                 cts.Cancel();
         }
 
-        private async Task<CheckResult> ScheduleExecuteCheckAsync(Check check)
-        {
-            if (!check.Enabled || !check.Server.Enabled)
-                return await Task.FromResult(new CheckResult(check, CheckStatus.Disabled, null));
-
-            CancellationTokenSource cts = new CancellationTokenSource();
-            tokens[check.Id] = cts;
-            check.NextRunTime = check.Schedule.GetNextTime(check.LastScheduledRunTime);
-            int delay = Math.Max(0, (int)(check.NextRunTime - DateTime.Now).TotalMilliseconds);
-            await Task.Delay(delay, cts.Token);
-            check.LastScheduledRunTime = check.NextRunTime;
-            if (networkAvailable && !cts.IsCancellationRequested)
-            {
-                return await ExecuteCheckAsync(check, cts.Token);
-            }
-            else
-            {
-                if (!pausedChecks.Contains(check.Id))
-                    pausedChecks.Add(check.Id);
-                return await Task.FromResult(new CheckResult(check, CheckStatus.Disabled, null));
-            }
-        }
-
+        /// <summary>Handles network state changing.</summary>
         private void NetworkChange_NetworkAddressChanged(object sender, EventArgs e)
         {
             networkAvailable = Helpers.IsNetworkAvailable();
+            // If the network is available, it might not have been before.
+            // This method is not called from the correct thread, so special
+            // handling is needed to start it on the UI thread again.
             if (networkAvailable)
                 mainForm.Invoke((MethodInvoker)(() => Run()));
         }
 
+        /// <summary>Handles system power mode changes.</summary>
         private async void SystemEvents_PowerModeChanged(object sender, PowerModeChangedEventArgs e)
         {
+            // If the system is being suspended, cancel all waiting and executing checks.
+            // Once all the checks are removed, the main loop will exit.
             if (e.Mode == PowerModes.Suspend)
             {
                 foreach (Check check in Checks)
@@ -304,32 +436,45 @@
             }
             else if (e.Mode == PowerModes.Resume)
             {
+                // When resuming from suspend, examine each check to find out if it was
+                // scheduled to be executed during the time period when the systems was
+                // suspended. Add them to the paused checks list, to be executed almost
+                // immediately.
+                // Make sure the list is empty to start.
                 pausedChecks.Clear();
                 foreach (Check check in Checks)
                 {
-                    //CancelCheck(check);
                     if (check.Enabled && check.Server.Enabled && check.NextRunTime < DateTime.Now)
                     {
                         pausedChecks.Add(check.Id);
                     }
                 }
+                // Wait 10 seconds to give things time to quiet down after resuming.
                 await Task.Delay(10000);
                 suspend = false;
                 Run();
             }
         }
 
+        /// <summary>Unregister system events when exiting.</summary>
         private void Application_ApplicationExit(object sender, EventArgs e)
         {
             NetworkChange.NetworkAddressChanged -= NetworkChange_NetworkAddressChanged;
             SystemEvents.PowerModeChanged -= SystemEvents_PowerModeChanged;
         }
 
+        /// <summary>Attempts to read a private file.</summary>
+        /// <param name="path">The path to the private key file.</param>
+        /// <param name="password">The password used to encrypt the key.</param>
+        /// <returns>A status indicating the result of the attempt.</returns>
         public KeyStatus OpenPrivateKey(string path, string password = null)
         {
             KeyStatus keyStatus;
+
             if (path == null)
                 keyStatus = KeyStatus.NotAccessible;
+
+            // Check if the key has already been open and read.
             if (privateKeys.TryGetValue(path, out PrivateKeyFile key) && key != null)
                 keyStatus = KeyStatus.Open;
             else
@@ -339,15 +484,20 @@
                     key = new PrivateKeyFile(path, password);
                     keyStatus = KeyStatus.Open;
                 }
+                // If the key is encrypted and the password is empty or incorrect,
+                // return the NeedPassword status.
                 catch (Exception e) when (e is SshPassPhraseNullOrEmptyException || e is InvalidOperationException)
                 {
                     keyStatus = KeyStatus.NeedPassword;
                 }
+                // For any other failure reason, return the NotAccessible status.
                 catch (Exception)
                 {
                     keyStatus = KeyStatus.NotAccessible;
                 }
             }
+            // A single private key may be used by multiple servers. Update all servers
+            // that use this private key with the results of the above operations.
             foreach (Server server in Servers)
             {
                 if (server.KeyFile == path)
@@ -356,14 +506,21 @@
                     server.KeyStatus = keyStatus;
                 }
             }
+            // Keep a reference to this private key so we don't have to re-open
+            // it later if the same key is used on a different server.
             privateKeys[path] = key;
+
             return keyStatus;
         }
 
+        /// <summary>Generates internal IDs for servers and checks.</summary>
         private void GenerateIds()
         {
             if (Servers.Any())
             {
+                // Start at the maximum ID to make sure IDs are not reused
+                // if a server was deleted so old log entries do not get associated
+                // with a new server.
                 int id = Servers.Max(s => s.Id);
                 foreach (Server server in Servers)
                 {
@@ -374,6 +531,8 @@
 
             if (Checks.Any())
             {
+                // Start with the max check ID, same reasons as above.
+                // Is there a reason this is stored in a setting?
                 int id = Math.Max(Settings.Default.MaxCheckId, Checks.Max(c => c.Id));
                 foreach (Check check in Checks)
                 {
@@ -385,16 +544,21 @@
             }
         }
 
+        /// <summary>Creates an XML serializer that can handle servers and all check types.</summary>
+        /// <returns>An XML serializer that can handle servers and all check types.</returns>
         private XmlSerializer CreateXmlSerializer()
         {
             return new XmlSerializer(typeof(List<Server>), Check.CheckTypes);
         }
     }
 
+    /// <summary>Event arguments for when a check status changes.</summary>
     public class CheckStatusChangedEventArgs : EventArgs
     {
+        /// <summary>The check whose status changed.</summary>
         public Check Check { get; private set; }
 
+        /// <summary>The check result that caused the status to change, if any.</summary>
         public CheckResult CheckResult { get; private set; }
 
         public CheckStatusChangedEventArgs(Check check, CheckResult result)
@@ -404,5 +568,15 @@
         }
     }
 
-    public enum FailAction { FlashTaskbar = 0, NotificationBalloon = 1, None = 10 }
+    /// <summary>Possible actions that may be taken when a check fails.</summary>
+    public enum FailAction
+    {
+        /// <summary>Flashes the Server Monitor tasbar program icon.</summary>
+        FlashTaskbar = 0,
+        /// <summary>Shows a balloon in the notification area.</summary>
+        NotificationBalloon = 1,
+        /// <summary>Take no action.</summary>
+        None = 10
+    }
+
 }