From c5b7597363e9b23e8767386e442df91c0b330c3b Mon Sep 17 00:00:00 2001 From: vidurvij-Unity <60901103+vidurvij-Unity@users.noreply.github.com> Date: Tue, 6 Apr 2021 16:10:55 -0700 Subject: [PATCH 1/5] Add Ip address check --- .../Editor/ROSSettingsEditor.cs | 25 ++++++++++-- .../Runtime/TcpConnector/ROSConnection.cs | 38 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs b/com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs index 8d9ae9a5..ed5a37a7 100644 --- a/com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs +++ b/com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs @@ -17,6 +17,8 @@ public static void OpenWindow() GameObject prefabObj; ROSConnection prefab; + string rosIP = "127.0.0.1"; + string unityOverrideIP = ""; protected virtual void OnGUI() { @@ -41,13 +43,30 @@ protected virtual void OnGUI() } EditorGUILayout.LabelField("Settings for a new ROSConnection.instance", EditorStyles.boldLabel); - prefab.rosIPAddress = EditorGUILayout.TextField("ROS IP Address", prefab.rosIPAddress); + rosIP = EditorGUILayout.TextField("ROS IP Address", rosIP); prefab.rosPort = EditorGUILayout.IntField("ROS Port", prefab.rosPort); EditorGUILayout.Space(); - prefab.overrideUnityIP = EditorGUILayout.TextField( + unityOverrideIP = EditorGUILayout.TextField( new GUIContent("Override Unity IP Address", "If blank, determine IP automatically."), - prefab.overrideUnityIP); + unityOverrideIP); prefab.unityPort = EditorGUILayout.IntField("Unity Port", prefab.unityPort); + if (GUILayout.Button("Set IP")) + { + if(unityOverrideIP != "") + { + if(ROSConnection.IPFormatIsCorrect(unityOverrideIP)) + prefab.overrideUnityIP = unityOverrideIP; + else + Debug.LogError("Override Unity IP Address is incorrect"); + } + + if(ROSConnection.IPFormatIsCorrect(rosIP)) + prefab.rosIPAddress = rosIP; + else + Debug.LogError("ROS IP Address is incorrect"); + + this.Close(); + } EditorGUILayout.Space(); EditorGUILayout.LabelField("If awaiting a service response:", EditorStyles.boldLabel); prefab.awaitDataMaxRetries = EditorGUILayout.IntField( diff --git a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs index 2dbd2918..94e32967 100644 --- a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs +++ b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs @@ -8,6 +8,7 @@ using System.Text; using System.Threading.Tasks; using Unity.Robotics.ROSTCPConnector.MessageGeneration; +using System.Globalization; using UnityEngine; using UnityEngine.Serialization; @@ -224,11 +225,15 @@ void OnEnable() private void Start() { + if(!IPFormatIsCorrect(rosIPAddress)) + Debug.LogError("ROS IP address is not correct"); InitializeHUD(); Subscribe(ERROR_TOPIC_NAME, RosUnityErrorCallback); if (overrideUnityIP != "") { + if(!IPFormatIsCorrect(overrideUnityIP)) + Debug.LogError("Override Unity IP address is not correct"); StartMessageServer(overrideUnityIP, unityPort); // no reason to wait, if we already know the IP } @@ -588,5 +593,38 @@ private void WriteDataStaggered(NetworkStream networkStream, string rosTopicName networkStream.Write(segmentData, 0, segmentData.Length); } } + + public static bool IPFormatIsCorrect(string iPaddress) + { + string[] subAdds = iPaddress.Split('.'); + if(subAdds.Length != 4) + { + Debug.LogError($"{subAdds.Length} is incorrect number of octets in IP address"); + return false; + } + foreach(string subAdd in subAdds) + { + try + { + int number = Int32.Parse(subAdd,NumberStyles.None); + if(!(number >= 0 && number <= 255)) + { + Debug.LogError($"{number} is incorrect value of octets in IP address (0-255)"); + return false; + } + } + catch (FormatException) + { + Debug.LogError($"Unable to convert '{subAdd}'."); + return false; + } + catch (OverflowException) + { + Debug.LogError($"'{subAdd}' is out of range of the Int32 type."); + return false; + } + } + return true; + } } } \ No newline at end of file From f70d90106665e5f27da268b5997f7cfa35aa1e55 Mon Sep 17 00:00:00 2001 From: vidurvij-Unity <60901103+vidurvij-Unity@users.noreply.github.com> Date: Tue, 6 Apr 2021 17:28:17 -0700 Subject: [PATCH 2/5] Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html --- .../Runtime/TcpConnector/ROSConnection.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs index 94e32967..2b31a96c 100644 --- a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs +++ b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs @@ -596,6 +596,20 @@ private void WriteDataStaggered(NetworkStream networkStream, string rosTopicName public static bool IPFormatIsCorrect(string iPaddress) { + //if IP address is set using + if(Char.IsLetter(iPaddress[0])) + { + foreach(Char subChar in iPaddress) + { + if(!(Char.IsLetterOrDigit(subChar) || subChar == '-'|| subChar == '.')) + return false; + } + + if(!Char.IsLetterOrDigit(iPaddress[iPaddress.Length - 1])) + return false; + return true; + } + string[] subAdds = iPaddress.Split('.'); if(subAdds.Length != 4) { From e34f77465d94f4518ee74c346566509df4f4cd33 Mon Sep 17 00:00:00 2001 From: vidurvij-Unity <60901103+vidurvij-Unity@users.noreply.github.com> Date: Mon, 12 Apr 2021 22:15:10 -0700 Subject: [PATCH 3/5] Corrections for PR review 1. Used IPaddress.TryParse method to parse the address. 2. Display warning messages on the ROSConnection menu if address is invalid. --- .../Editor/ROSSettingsEditor.cs | 31 +++++---------- .../Runtime/TcpConnector/ROSConnection.cs | 38 +++++-------------- 2 files changed, 19 insertions(+), 50 deletions(-) diff --git a/com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs b/com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs index ed5a37a7..e70e8048 100644 --- a/com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs +++ b/com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs @@ -17,9 +17,6 @@ public static void OpenWindow() GameObject prefabObj; ROSConnection prefab; - string rosIP = "127.0.0.1"; - string unityOverrideIP = ""; - protected virtual void OnGUI() { if (prefab == null) @@ -43,29 +40,21 @@ protected virtual void OnGUI() } EditorGUILayout.LabelField("Settings for a new ROSConnection.instance", EditorStyles.boldLabel); - rosIP = EditorGUILayout.TextField("ROS IP Address", rosIP); + prefab.rosIPAddress = EditorGUILayout.TextField("ROS IP Address", prefab.rosIPAddress); prefab.rosPort = EditorGUILayout.IntField("ROS Port", prefab.rosPort); EditorGUILayout.Space(); - unityOverrideIP = EditorGUILayout.TextField( + prefab.overrideUnityIP = EditorGUILayout.TextField( new GUIContent("Override Unity IP Address", "If blank, determine IP automatically."), - unityOverrideIP); + prefab.overrideUnityIP); prefab.unityPort = EditorGUILayout.IntField("Unity Port", prefab.unityPort); - if (GUILayout.Button("Set IP")) + if ((prefab.overrideUnityIP != "" && !ROSConnection.IPFormatIsCorrect(prefab.overrideUnityIP))) { - if(unityOverrideIP != "") - { - if(ROSConnection.IPFormatIsCorrect(unityOverrideIP)) - prefab.overrideUnityIP = unityOverrideIP; - else - Debug.LogError("Override Unity IP Address is incorrect"); - } - - if(ROSConnection.IPFormatIsCorrect(rosIP)) - prefab.rosIPAddress = rosIP; - else - Debug.LogError("ROS IP Address is incorrect"); - - this.Close(); + EditorGUILayout.HelpBox("Unity Override IP invalid", MessageType.Warning); + } + + if(!ROSConnection.IPFormatIsCorrect(prefab.rosIPAddress)) + { + EditorGUILayout.HelpBox("ROS IP is invalid", MessageType.Warning); } EditorGUILayout.Space(); EditorGUILayout.LabelField("If awaiting a service response:", EditorStyles.boldLabel); diff --git a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs index 2b31a96c..3dc3ac0a 100644 --- a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs +++ b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs @@ -594,51 +594,31 @@ private void WriteDataStaggered(NetworkStream networkStream, string rosTopicName } } - public static bool IPFormatIsCorrect(string iPaddress) + public static bool IPFormatIsCorrect(string ipAddress) { - //if IP address is set using - if(Char.IsLetter(iPaddress[0])) + if(ipAddress == null || ipAddress == "") + return false; + if(Char.IsLetter(ipAddress[0])) { - foreach(Char subChar in iPaddress) + foreach(Char subChar in ipAddress) { if(!(Char.IsLetterOrDigit(subChar) || subChar == '-'|| subChar == '.')) return false; } - if(!Char.IsLetterOrDigit(iPaddress[iPaddress.Length - 1])) + if(!Char.IsLetterOrDigit(ipAddress[ipAddress.Length - 1])) return false; return true; } - string[] subAdds = iPaddress.Split('.'); + string[] subAdds = ipAddress.Split('.'); if(subAdds.Length != 4) { Debug.LogError($"{subAdds.Length} is incorrect number of octets in IP address"); return false; } - foreach(string subAdd in subAdds) - { - try - { - int number = Int32.Parse(subAdd,NumberStyles.None); - if(!(number >= 0 && number <= 255)) - { - Debug.LogError($"{number} is incorrect value of octets in IP address (0-255)"); - return false; - } - } - catch (FormatException) - { - Debug.LogError($"Unable to convert '{subAdd}'."); - return false; - } - catch (OverflowException) - { - Debug.LogError($"'{subAdd}' is out of range of the Int32 type."); - return false; - } - } - return true; + IPAddress parsedipAddress; + return IPAddress.TryParse(ipAddress, out parsedipAddress); } } } \ No newline at end of file From e1c1dcb77a7a869e6ef959fed38e5a094e113d1f Mon Sep 17 00:00:00 2001 From: vidurvij-Unity <60901103+vidurvij-Unity@users.noreply.github.com> Date: Tue, 13 Apr 2021 15:59:13 -0700 Subject: [PATCH 4/5] Adding a comment about checks on hostname --- .../Runtime/TcpConnector/ROSConnection.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs index 3dc3ac0a..9f290f9e 100644 --- a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs +++ b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs @@ -598,6 +598,8 @@ public static bool IPFormatIsCorrect(string ipAddress) { if(ipAddress == null || ipAddress == "") return false; + + // If IP address is set using static lookup tables https://man7.org/linux/man-pages/man5/hosts.5.html if(Char.IsLetter(ipAddress[0])) { foreach(Char subChar in ipAddress) From c5c90c1a525bc0737b7af9dbbc590310c0aa18c9 Mon Sep 17 00:00:00 2001 From: vidurvij-Unity <60901103+vidurvij-Unity@users.noreply.github.com> Date: Tue, 13 Apr 2021 16:32:59 -0700 Subject: [PATCH 5/5] Update ROSConnection.cs --- .../Runtime/TcpConnector/ROSConnection.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs index 9f290f9e..eebc18e9 100644 --- a/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs +++ b/com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs @@ -616,7 +616,6 @@ public static bool IPFormatIsCorrect(string ipAddress) string[] subAdds = ipAddress.Split('.'); if(subAdds.Length != 4) { - Debug.LogError($"{subAdds.Length} is incorrect number of octets in IP address"); return false; } IPAddress parsedipAddress;