Change behavior of Param::Get<bool> (#1397)

Previously when a Param was set from a string, the `Get<bool>` method would always return `true`, and the value would be `true` if the lowercase value string matched `"1"` or `"true"`. Otherwise the value would be `false`. Now, the `Get<bool>` method returns `true` only if the lowercase value string matches one of `"0"`, `"1"`, `"true"`, or `"false"`. Otherwise `Get<bool>` returns `false` and the value is not written. Signed-off-by: Steve Peters <[email protected]>
gazebosim · Apr 10, 2024 · c0dbc0c · c0dbc0c
1 parent 9f6848a
commit c0dbc0c
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 20 deletions.
diff --git a/Migration.md b/Migration.md
@@ -20,6 +20,16 @@ but with improved human-readability..
     + Details about the 1.11 to 1.12 transition are explained below in this same
       document
 
+### Modifications
+
+1. **Behavior change of `Param::Get<bool>`**
+    + Previously when a Param was set from a string, the `Get<bool>` method
+      would always return `true`, and the value would be `true` if the lowercase
+      string value matched `"1"` or `"true"` and `false` otherwise. Now,
+      the `Get<bool>` method returns `true` only if the lowercase value string
+      matches `"0"`, `"1"`, `"true"`, or `"false"` and returns `false`
+      otherwise.
+
 ## libsdformat 13.x to 14.x
 
 ### Additions

diff --git a/include/sdf/Param.hh b/include/sdf/Param.hh
@@ -740,22 +740,6 @@ namespace sdf
       {
         _value = std::get<T>(pv);
       }
-      else if (typeStr == "bool" && this->dataPtr->typeName == "string")
-      {
-        // this section for handling bool types is to keep backward behavior
-        // TODO(anyone) remove for Fortress. For more details:
-        // https://github.com/gazebosim/sdformat/pull/638
-        valueStr = lowercase(valueStr);
-
-        std::stringstream tmp;
-        if (valueStr == "true" || valueStr == "1")
-          tmp << "1";
-        else
-          tmp << "0";
-
-        tmp >> _value;
-        return true;
-      }
 
       return success;
     }

diff --git a/python/test/pyParam_TEST.py b/python/test/pyParam_TEST.py
@@ -46,7 +46,7 @@ def test_bool(self):
         str_param.set_string("TRUE")
         self.assertTrue(str_param.get_bool()[0])
 
-        # Anything other than 1 or true is treated as a False value
+        # Anything other than "0", "1" , "false", or "true" raises an exception
         str_param.set_string("%")
         with self.assertRaises(SDFErrorsException):
             str_param.get_bool()

diff --git a/src/Param_TEST.cc b/src/Param_TEST.cc
@@ -79,10 +79,10 @@ TEST(Param, Bool)
   EXPECT_TRUE(strParam.Get<bool>(value));
   EXPECT_TRUE(value);
 
-  // Anything other than 1 or true is treated as a false value
+  // Anything other than "0", "1", "true", or "false" (any capitalization) will
+  // cause Get<bool> to fail.
   EXPECT_TRUE(strParam.Set("%"));
-  EXPECT_TRUE(strParam.Get<bool>(value));
-  EXPECT_FALSE(value);
+  EXPECT_FALSE(strParam.Get<bool>(value));
 
   EXPECT_TRUE(boolParam.Set(true));
   std::any anyValue;