copilot code review

Author: williamrjackson

ArduinoSerialCommunication.cs

@@ -2,6 +2,7 @@
 using System.IO.Ports;
 #endif
 using UnityEngine;
+using UnityEngine.Events;   
 using System.Collections.Generic;
 using System.Diagnostics;
 using System;
@@ -19,6 +20,10 @@ public class ArduinoSerialCommunication : MonoBehaviour
         [SerializeField]
         [Range(-1, 256)]
         private int portNumber = -1;
+        [SerializeField]
+        private bool logReceivedMessages = true;
+        [SerializeField]
+        private OnStringEvent[] onStringEvents;
 
         private string _rawData = string.Empty;
         private uint _dataIndex = 0;
@@ -29,6 +34,7 @@ public class ArduinoSerialCommunication : MonoBehaviour
 
         private Dictionary<string, string> portAssignments = new Dictionary<string, string>();
         private string comPort;
+        private volatile bool _keepListening = false;
 
         SerialPort port;
         Thread serialPortListenerThread;
@@ -69,7 +75,7 @@ void Start()
             PopulatePortList();
             if (portNumber >= 0 && PortExists(portNumber))
             {
-                comPort = portNumber.ToString();
+                comPort = "COM" + portNumber;
                 Connect();
             }
             else if ((comPort = GetFirstPortMatchingName(deviceName)) != null)
@@ -80,43 +86,44 @@ void Start()
             {
                 UnityEngine.Debug.LogWarning("Arduino Not Found");
             }
+            OnSerialEvent += HandleSerialString;
         }
 
         private void OnDestroy()
         {
-            if (serialPortListenerThread != null)
-            {
-                serialPortListenerThread.Abort();
-                ClosePort();
-            }
+            StopListening();
         }
         private void OnDisable()
         {
-            if (serialPortListenerThread != null)
-            {
-                serialPortListenerThread.Abort();
-                ClosePort();
-            }
+            StopListening();
         }
 
         void Connect()
         {
-            if (serialPortListenerThread != null)
-            {
-                serialPortListenerThread.Abort();
-                ClosePort();
-            }
+            StopListening();
 
             Utils.SafeTry(() => InitializeArduino(comPort, (int)BaudRate));
 
+            _keepListening = true;
             serialPortListenerThread = new Thread(RecieveDataInHelperThread);
             serialPortListenerThread.Start();
         }
 
+        private void StopListening()
+        {
+            _keepListening = false;
+            if (serialPortListenerThread != null && serialPortListenerThread.IsAlive)
+            {
+                serialPortListenerThread.Join(200);
+            }
+            ClosePort();
+            serialPortListenerThread = null;
+        }
+
         public void ClosePort()
         {
             UnityEngine.Debug.Log("Close port");
-            Utils.SafeTry(() => port.Close());
+            Utils.SafeTry(() => port?.Close());
         }
 
         void InitializeArduino(string listeningPort, int baudRate)
@@ -135,20 +142,27 @@ void InitializeArduino(string listeningPort, int baudRate)
 
         void RecieveDataInHelperThread()
         {
-            while (port.IsOpen)
+            try
             {
-                String str = port.ReadLine();
-                if (!string.IsNullOrWhiteSpace(str))
+                while (_keepListening && port != null && port.IsOpen)
                 {
-                    _rawData = str;
-                    _dataIndex++;
-                    if (OnSerialEvent != null)
+                    string str = port.ReadLine();
+                    if (!string.IsNullOrWhiteSpace(str))
                     {
-                        // Notify on main thread...
-                        Enqueue(ActionWrapper(() => OnSerialEvent(str)));
+                        _rawData = str;
+                        _dataIndex++;
+                        if (OnSerialEvent != null)
+                        {
+                            // Notify on main thread...
+                            Enqueue(ActionWrapper(() => OnSerialEvent(str)));
+                        }
                     }
                 }
             }
+            catch (Exception)
+            {
+                // Swallow read exceptions during shutdown/disconnect.
+            }
         }
 
         // Send text to the serial port.
@@ -223,7 +237,7 @@ private string GetFirstPortMatchingName(string contains)
         }
         private bool PortExists(int portNum)
         {
-            return (portAssignments.ContainsValue(portNum.ToString()));
+            return (portAssignments.ContainsValue("COM" + portNum));
         }
         void Enqueue(IEnumerator action)
         {
@@ -240,6 +254,54 @@ IEnumerator ActionWrapper(Action action)
             action();
             yield return null;
         }
+
+        private void HandleSerialString(string message)
+        {
+            foreach (var onStringEvent in onStringEvents)
+            {
+                if (onStringEvent.Message == message)
+                {
+                    bool invoked = onStringEvent.Invoke();
+                    if (logReceivedMessages)
+                    {
+                        if (invoked)
+                        {
+                            UnityEngine.Debug.Log($"Invoked command for message: {message}");
+                        }
+                        else
+                        {
+                            UnityEngine.Debug.Log($"Debounced command for message: {message}");
+                        }
+                    }
+                }
+            }
+        }
+        [System.Serializable]
+        class OnStringEvent
+        {
+            [SerializeField]
+            private string message;
+            [SerializeField]
+            private float debounceTime = 0.1f;
+            [SerializeField]
+            private UnityEvent command;
+
+            private float lastInvokeTime = float.NegativeInfinity;
+            public string Message { get { return message; } }
+
+            public bool Invoke()
+            {
+                float timeSinceLastInvoke = Time.time - lastInvokeTime;
+                if (timeSinceLastInvoke < debounceTime)
+                {
+                    return false;
+                }
+
+                lastInvokeTime = Time.time;
+                command.Invoke();
+                return true;
+            }
+        }
 #endif
     }
 }

Audio/RandomizedSoundEffect.cs

@@ -18,13 +18,13 @@ void Start()
 
         public void PlayRandom(float pitchMin = 1f, float pitchMax = 1f, float volumeMin = 1f, float volumeMax = 1f)
         {
-            if (audioClips.Count == 0 || audioClips == null)
+            if (audioClips == null || audioClips.Count == 0)
                 return;
             m_AudioPool.PlayPitchOneShot(audioClips.GetRandom(), Random.Range(pitchMin, pitchMax), Random.Range(volumeMin, volumeMax));
         }
         public void PlayRandom()
         {
-            if (audioClips.Count == 0 || audioClips == null)
+            if (audioClips == null || audioClips.Count == 0)
                 return;
             m_AudioPool.PlayOneShot(audioClips.GetRandom());
         }