From 7442a7db74bd65af1bb3661e534b850df18a71dc Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Fri, 5 Apr 2024 16:06:30 -0700 Subject: [PATCH 1/3] Change behavior of Param::Get Previously when a Param was set from a string, the `Get` 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` method returns `true` only if the lowercase value string matches one of `"0"`, `"1"`, `"true"`, or `"false"`. Otherwise `Get` returns `false` and the value is not written. Signed-off-by: Steve Peters --- Migration.md | 10 ++++++++++ include/sdf/Param.hh | 16 ---------------- src/Param_TEST.cc | 6 +++--- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/Migration.md b/Migration.md index edc36b79d..410c4931b 100644 --- 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`** + + Previously when a Param was set from a string, the `Get` 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` 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 index 281b738a9..d0a952f4c 100644 --- a/include/sdf/Param.hh +++ b/include/sdf/Param.hh @@ -740,22 +740,6 @@ namespace sdf { _value = std::get(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/src/Param_TEST.cc b/src/Param_TEST.cc index 378a088bc..55c1a6df7 100644 --- a/src/Param_TEST.cc +++ b/src/Param_TEST.cc @@ -79,10 +79,10 @@ TEST(Param, Bool) EXPECT_TRUE(strParam.Get(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 + // causes Get to fail. EXPECT_TRUE(strParam.Set("%")); - EXPECT_TRUE(strParam.Get(value)); - EXPECT_FALSE(value); + EXPECT_FALSE(strParam.Get(value)); EXPECT_TRUE(boolParam.Set(true)); std::any anyValue; From 6d348c214bf361437c74a190ee95665ec81234bf Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Mon, 8 Apr 2024 17:26:55 -0700 Subject: [PATCH 2/3] pyParam_TEST.py: update comment Signed-off-by: Steve Peters --- python/test/pyParam_TEST.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/test/pyParam_TEST.py b/python/test/pyParam_TEST.py index dea6c2a8e..56ad8a7d6 100644 --- 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() From 597964a37ec6f8fa4629ce049d36f9411cebe476 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Tue, 9 Apr 2024 20:28:09 -0700 Subject: [PATCH 3/3] Fix comment Signed-off-by: Steve Peters --- src/Param_TEST.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Param_TEST.cc b/src/Param_TEST.cc index 55c1a6df7..0bcd38633 100644 --- a/src/Param_TEST.cc +++ b/src/Param_TEST.cc @@ -79,8 +79,8 @@ TEST(Param, Bool) EXPECT_TRUE(strParam.Get(value)); EXPECT_TRUE(value); - // Anything other than 0, 1, true, or false (any capitalization) will - // causes Get to fail. + // Anything other than "0", "1", "true", or "false" (any capitalization) will + // cause Get to fail. EXPECT_TRUE(strParam.Set("%")); EXPECT_FALSE(strParam.Get(value));