Topic validation (#35)

Validation of ROS2 names:
- validate topics and namespaces
- rosify name now deals with names starting with numbers
- rosify name now deals with names starting with underscore. Warns if explicit, prepends o3de_ otherwise

Signed-off-by: Adam Dąbrowski <[email protected]>

Gems/ROS2/Code/Source/Frame/NamespaceConfiguration.cpp

@@ -92,6 +92,7 @@ namespace ROS2
                         ->EnumAttribute(NamespaceConfiguration::NamespaceStrategy::Custom, "Custom")
                     ->DataElement(AZ::Edit::UIHandlers::Default, &NamespaceConfiguration::m_namespace, "Namespace", "Namespace")
                         ->Attribute(AZ::Edit::Attributes::Visibility, &NamespaceConfiguration::IsNamespaceCustom)
+                        ->Attribute(AZ::Edit::Attributes::ChangeValidate, &ROS2Names::ValidateNamespaceField)
                         // TODO - hide for now, but desired Editor component behavior would be to show a read only value
                         ;
             }

Gems/ROS2/Code/Source/RobotControl/ROS2RobotControlComponent.cpp

@@ -50,6 +50,7 @@ namespace ROS2
                     ->ClassElement(AZ::Edit::ClassElements::EditorData, "")
                         ->Attribute(AZ::Edit::Attributes::AppearsInAddComponentMenu, AZ_CRC("Game")) // TODO - "Simulation"?
                     ->DataElement(AZ::Edit::UIHandlers::Default, &ROS2RobotControlComponent::m_topic, "Topic", "ROS2 topic to subscribe to")
+                        ->Attribute(AZ::Edit::Attributes::ChangeValidate, &ROS2Names::ValidateTopicField)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &ROS2RobotControlComponent::m_qos, "QoS", "Quality of Service settings for subscriber")
                     ;
             }

Gems/ROS2/Code/Source/Sensor/PublisherConfiguration.cpp

@@ -9,12 +9,12 @@
 #pragma once
 
 #include "PublisherConfiguration.h"
+#include "Utilities/ROS2Names.h"
 #include <AzCore/Serialization/EditContext.h>
 #include <AzCore/Serialization/SerializeContext.h>
 
 namespace ROS2
 {
-    /// A struct used to create and configure publishers
     void PublisherConfiguration::Reflect(AZ::ReflectContext* context)
     {
         if (auto serializeContext = azrtti_cast<AZ::SerializeContext*>(context))
@@ -33,6 +33,7 @@ namespace ROS2
                     ->DataElement(AZ::Edit::UIHandlers::Default, &PublisherConfiguration::m_type, "Type", "Type of topic messages")
                         ->Attribute(AZ::Edit::Attributes::ReadOnly, true)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &PublisherConfiguration::m_topic, "Topic", "Topic with no namespace")
+                        ->Attribute(AZ::Edit::Attributes::ChangeValidate, &ROS2Names::ValidateTopicField)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &PublisherConfiguration::m_qos, "QoS", "Quality of Service")
                     ;
             }

Gems/ROS2/Code/Source/Utilities/ROS2Names.cpp

@@ -8,6 +8,8 @@
 
 #include "ROS2Names.h"
 #include <AzCore/std/string/regex.h>
+#include <rcl/validate_topic_name.h>
+#include <rmw/validate_namespace.h>
 
 namespace ROS2
 {
@@ -25,8 +27,92 @@ namespace ROS2
     {
         // TODO - add unit tests
         // TODO - implement stricter guidelines and differentiate: https://design.ros2.org/articles/topic_and_service_names.html
-        // TODO - add check whether it begins with a number (and if so, prepend underscore)
+
+        AZStd::string rosified = input;
+        if (input.empty())
+        {
+            return rosified;
+        }
+
+        const char underscore = '_';
+        if (input[0] == underscore)
+        {
+            AZ_Warning("RosifyName",
+               false,
+               "'%s' name starts with an underscore, which makes topic/namespace/parameter hidden by default. Is this intended?",
+               input.c_str());
+        }
+        
+        const AZStd::string stringToReplaceViolations(1, underscore);
         const AZStd::regex ros2Disallowedlist("[^0-9|a-z|A-Z|_]");
-        return AZStd::regex_replace(input, ros2Disallowedlist, "_");
+        rosified = AZStd::regex_replace(rosified, ros2Disallowedlist, stringToReplaceViolations);
+
+        if (std::isdigit(rosified[0]) || (input[0] != underscore && rosified[0] == underscore))
+        {   // Prepend "o3de_" if it would otherwise start with a number (which would violate ros2 name requirements)
+            // Also, starting with '_' is not desired unless explicit. Topics/namespaces/parameters starting with "_" are hidden by default.
+            const AZStd::string prependToNumberStart = "o3de_";
+            rosified = prependToNumberStart + rosified;
+        }
+
+        if (input != rosified)
+        {
+            AZ_TracePrintf("RosifyName", "Name '%s' has been changed to '%s' to conform with ros2 naming restrictions", input.c_str(), rosified.c_str());
+        }
+        return rosified;
+    }
+
+    AZ::Outcome<void, AZStd::string> ROS2Names::ValidateNamespace(const AZStd::string& ros2Namespace)
+    {
+        auto ros2GlobalizedNamespace = ros2Namespace;
+        const char namespacePrefix = '/';
+        if (ros2Namespace.find(namespacePrefix, 0) != 0)
+        {  // Prepend "/" if not included, this is done automatically by rclcpp so "/"-less namespaces are ok.
+            ros2GlobalizedNamespace = namespacePrefix + ros2Namespace;
+        }
+
+        int validationResult = 0;
+        auto ret = rmw_validate_namespace(ros2GlobalizedNamespace.c_str(), &validationResult, NULL);
+        if (ret != RMW_RET_OK)
+        {
+            AZ_Error("ValidateNamespace", false, "Call to rmw validation for namespace failed");
+            return AZ::Failure(AZStd::string("Unable to validate namespace due to rmw error"));
+        }
+
+        if (validationResult != RMW_NAMESPACE_VALID)
+        {
+            return AZ::Failure(AZStd::string(rmw_namespace_validation_result_string(validationResult)));
+        }
+
+        return AZ::Success();
+    }
+
+    AZ::Outcome<void, AZStd::string> ROS2Names::ValidateNamespaceField(void* newValue, [[maybe_unused]] const AZ::Uuid& valueType)
+    {
+        AZStd::string ros2Namespace(static_cast<const char*>(newValue));
+        return ValidateNamespace(ros2Namespace);
+    }
+
+    AZ::Outcome<void, AZStd::string> ROS2Names::ValidateTopic(const AZStd::string& topic)
+    {
+        int validationResult = 0;
+        size_t invalidIndex; // Unused. We could choose to display it in information, but it is not necessary.
+        if (rcl_validate_topic_name(topic.c_str(), &validationResult, &invalidIndex) != RCL_RET_OK)
+        {
+            AZ_Error("ValidateTopic", false, "Call to rcl validation for topic failed");
+            return AZ::Failure(AZStd::string("Unable to validate topic due to rcl error"));
+        }
+
+        if (RCL_TOPIC_NAME_VALID != validationResult)
+        {
+            return AZ::Failure(AZStd::string(rcl_topic_name_validation_result_string(validationResult)));
+        }
+
+        return AZ::Success();
+    }
+
+    AZ::Outcome<void, AZStd::string> ROS2Names::ValidateTopicField(void* newValue, [[maybe_unused]] const AZ::Uuid& valueType)
+    {
+        AZStd::string topic(static_cast<const char*>(newValue));
+        return ValidateTopic(topic);
     }
 }  // namespace ROS2

Gems/ROS2/Code/Source/Utilities/ROS2Names.h

@@ -8,6 +8,8 @@
 #pragma once
 
 #include <AzCore/std/string/string.h>
+#include <AzCore/Outcome/Outcome.h>
+#include <AzCore/RTTI/TypeInfo.h>
 
 namespace ROS2
 {
@@ -16,5 +18,12 @@ namespace ROS2
     public:
         static AZStd::string GetNamespacedName(const AZStd::string& ns, const AZStd::string& name);
         static AZStd::string RosifyName(const AZStd::string& input);
+
+        // Validate namespace. One signature is straightforward, the other fits ChangeValidate for Editor fields.
+        static AZ::Outcome<void, AZStd::string> ValidateNamespace(const AZStd::string& ros2Namespace);
+        static AZ::Outcome<void, AZStd::string> ValidateNamespaceField(void* newValue, const AZ::Uuid& valueType);
+
+        static AZ::Outcome<void, AZStd::string> ValidateTopic(const AZStd::string& topic);
+        static AZ::Outcome<void, AZStd::string> ValidateTopicField(void* newValue, const AZ::Uuid& valueType);
     };
 }  // namespace ROS2