altium: Fix #6242 by increasing clamp limit for unit conversation. Furthermore fix error which happens when parsing negative units.

common/plugins/altium/altium_parser.cpp

@@ -156,48 +156,21 @@ int32_t ALTIUM_PARSER::PropertiesReadKicadUnit( const std::map<wxString, wxStrin
 {
     const wxString& value = PropertiesReadString( aProperties, aKey, aDefault );
 
-    size_t decimal_point = value.find( '.' );
-    size_t value_end     = value.find_first_not_of( "+-0123456789." );
-
-    wxString before_decimal_str   = value.Left( decimal_point );
-    int      before_decimal       = wxAtoi( before_decimal_str );
-    int      after_decimal        = 0;
-    size_t   after_decimal_digits = 0;
-    if( decimal_point != wxString::npos )
+    wxString prefix;
+    if( !value.EndsWith( "mil", &prefix ) )
     {
-        if( value_end != wxString::npos )
-        {
-            after_decimal_digits = value_end - ( decimal_point + 1 );
-        }
-        else
-        {
-            after_decimal_digits = value.size() - ( decimal_point + 1 ); // TODO: correct?
-        }
-        wxString after_decimal_str = value.Mid( decimal_point + 1, after_decimal_digits );
-        after_decimal              = wxAtoi( after_decimal_str );
+        wxLogError( wxString::Format( "Unit '%s' does not end with mil", value ) );
+        return 0;
     }
 
-    if( value.length() > 3 && value.compare( value.length() - 3, 3, "mil" ) == 0 )
+    double mils;
+    if( !prefix.ToCDouble( &mils ) )
     {
-        // ensure after_decimal is formatted to base 1000
-        int after_decimal_1000;
-        if( after_decimal_digits <= 4 )
-        {
-            after_decimal_1000 =
-                    static_cast<int>( after_decimal * std::pow( 10, 4 - after_decimal_digits ) );
-        }
-        else
-        {
-            after_decimal_1000 =
-                    static_cast<int>( after_decimal / std::pow( 10, after_decimal_digits - 4 ) );
-        }
-
-        int32_t mils = before_decimal * 10000 + after_decimal_1000;
-        return ConvertToKicadUnit( mils );
+        wxLogError( wxString::Format( "Cannot convert '%s' into double", prefix ) );
+        return 0;
     }
 
-    wxLogError( wxString::Format( "Unit '%s' does not end with mils" ), value );
-    return 0;
+    return ConvertToKicadUnit( mils * 10000 );
 }
 
 wxString ALTIUM_PARSER::PropertiesReadString( const std::map<wxString, wxString>& aProperties,

common/plugins/altium/altium_parser.h

@@ -123,9 +123,9 @@ public:
 
     static int32_t ConvertToKicadUnit( const double aValue )
     {
-        double int_limit = std::numeric_limits<int>::max() * 0.7071;    // 0.7071 = roughly 1/sqrt(2)
+        const double int_limit = ( std::numeric_limits<int>::max() - 1 ) / 2.54;
 
-        return KiROUND( Clamp<double>( -int_limit, aValue * 2.54, int_limit ) );
+        return KiROUND( Clamp<double>( -int_limit, aValue, int_limit ) * 2.54 );
     }
 
     static int PropertiesReadInt(

qa/common/CMakeLists.txt

@@ -45,6 +45,8 @@ set( common_srcs
 
     libeval/test_numeric_evaluator.cpp
 
+    plugin/altium/test_altium_parser.cpp
+
     view/test_zoom_controller.cpp
 )
 

qa/common/plugin/altium/test_altium_parser.cpp

@@ -0,0 +1,123 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2021 KiCad Developers, see CHANGELOG.TXT for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+/**
+ * @file test_altium_parserr.cpp
+ * Test suite for #ALTIUM_PARSER
+ */
+
+#include <unit_test_utils/unit_test_utils.h>
+
+#include <common/plugins/altium/altium_parser.h>
+
+struct ALTIUM_PARSER_FIXTURE
+{
+    ALTIUM_PARSER_FIXTURE() {}
+};
+
+
+/**
+ * Declares the struct as the Boost test fixture.
+ */
+BOOST_FIXTURE_TEST_SUITE( AltiumParser, ALTIUM_PARSER_FIXTURE )
+
+struct READ_KICAD_UNIT_CASE
+{
+    wxString input;
+    int      exp_result;
+};
+
+/**
+ * A list of valid test strings and the expected results
+ */
+static const std::vector<READ_KICAD_UNIT_CASE> read_kicad_unit_property = {
+    // Empty (use default)
+    { "", 0 },
+    // Some simple cases
+    { "0mil", 0 },
+    { "1mil", 25400 },
+    { "+1mil", 25400 },
+    { "-1mil", -25400 },
+    // Decimal Places
+    { "0.1mil", 2540 },
+    { "-0.1mil", -2540 },
+    { "0.01mil", 254 },
+    { "-0.01mil", -254 },
+    { "0.001mil", 25 },
+    { "-0.001mil", -25 },
+    { "0.0001mil", 3 },
+    { "-0.0001mil", -3 },
+    { "0.00001mil", 0 },
+    { "-0.00001mil", -0 },
+    { "0.00002mil", 1 },
+    { "-0.00002mil", -1 },
+    // Big Numbers
+    { "10mil", 254000 },
+    { "-10mil", -254000 },
+    { "100mil", 2540000 },
+    { "-100mil", -2540000 },
+    { "1000mil", 25400000 },
+    { "-1000mil", -25400000 },
+    { "10000mil", 254000000 },
+    { "-10000mil", -254000000 },
+    // Edge Cases
+    { "84546.6002mil", 2147483645 },
+    { "-84546.6002mil", -2147483645 },
+    // Clamp bigger values
+    { "84546.6003mil", 2147483646 },
+    { "-84546.6003mil", -2147483646 },
+    { "100000mil", 2147483646 },
+    { "-100000mil", -2147483646 },
+    { "1000000mil", 2147483646 },
+    { "-1000000mil", -2147483646 },
+    { "10000000mil", 2147483646 },
+    { "-10000000mil", -2147483646 },
+    // Incorrect suffix
+    { "100", 0 },
+    { "100mils", 0 },
+    // Incorrect prefix
+    { "+-100mil", 0 },
+    { "a100mil", 0 },
+};
+
+
+/**
+ * Run through a set of test strings, clearing in between
+ */
+BOOST_AUTO_TEST_CASE( Results )
+{
+    for( const auto& c : read_kicad_unit_property )
+    {
+        BOOST_TEST_CONTEXT( c.input + " -> " + wxString::Format( wxT( "%i" ), c.exp_result ) )
+        {
+            std::map<wxString, wxString> properties = { { "TEST", c.input } };
+
+            int result = ALTIUM_PARSER::PropertiesReadKicadUnit( properties, "TEST", "0mil" );
+
+            // These are all valid
+            BOOST_CHECK_EQUAL( result, c.exp_result );
+        }
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()