kicad-developers team mailing list archive

[Oliver Walters <oliver.henry.walters@xxxxxxxxx>]

Wayne,

Please find attached an updated patch set which also fixes 3D model units
for the STEP exporter tool.



On Thu, Nov 23, 2017 at 1:59 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:

> Consistency.  Currently we have two separate units in kicad board and
> footprint files.  This change unifies that issue.  I know it's annoying
> but it is a decision that I made to make the file formatting more
> consistent.  Yes, it is a change that will break some users 3D models
> but I think the long term benefit outweighs the short term annoyance.
>
> On 11/22/2017 09:51 AM, José Ignacio wrote:
> > I have several footprints that use manufacturer's models, where offsets
> > and rotations are necessary. I really fail to see the point of breaking
> > people's designs and libraries needlessly.
> >
> > On Nov 22, 2017 7:07 AM, "Wayne Stambaugh" <stambaughw@xxxxxxxxx
> > <mailto:stambaughw@xxxxxxxxx>> wrote:
> >
> >     What is wrong with just reading the footprint in mm rather than
> >     converting from decimils from now on?  It's only going to be a one
> time
> >     issue when a user adds a footprint that has not been converted to mm
> to
> >     a board.
> >
> >     On 11/22/2017 06:16 AM, Oliver Walters wrote:
> >     > Wayne,
> >     >
> >     >
> >     > I believe he has a point. The footprint files do not have version
> >     > information so if you load and save a footprint multiple times, the
> >     > "offset" (if non zero) will continuously be multiplied by 2.54x
> >     >
> >     > I think there are two ways forward:
> >     >
> >     > 1. Revert my patch and live with the file format unit inconsistency
> >     > 2. I can provide a patch for my original idea of writing "offset"
> >     > instead of "at". We make a clean break and "at" is legacy and
> always
> >     > read as inches. "offset" is new and is mm.
> >     >
> >     > Let me know what you want to do.
> >     >
> >     > Thanks,
> >     > Oliver
> >     >
> >     > On Wed, Nov 22, 2017 at 8:25 PM, easyw <easyw@xxxxxxxxxxxx
> >     <mailto:easyw@xxxxxxxxxxxx>
> >     > <mailto:easyw@xxxxxxxxxxxx <mailto:easyw@xxxxxxxxxxxx>>> wrote:
> >     >
> >     >     Hi Wayne,
> >     >
> >     >         I'm not sure I understand what the issue is.  Once an
> >     offset is
> >     >         changed
> >     >         to mm when either a footprint in a board or a library is
> >     parsed, why
> >     >         would it not be saved as mm.  If it isn't, then this is a
> >     bug.
> >     >         Once the
> >     >         footprint offset is converted to mm, there should be no
> >     >         expectation that
> >     >         it will be correct for older versions of KiCad.  Is there
> >     >         something else
> >     >         at play here?
> >     >
> >     >
> >     >     this issue is related to the footprint editor...
> >     >
> >     >     1) The fp exporter button exports correctly the footprint with
> >     >     offset in mm
> >     >     2) The fp importer button imports always reading the data as
> >     >     deci-mils and multiplies it internally
> >     >     3) To fix this issue the patch needs to manage the footprint
> >     >     importer code to read the values in mm instead of deci-mils.
> >     >
> >     >         What if you open the same file again, how can it tell it's
> >     in mm
> >     >         or inches?
> >     >
> >     >     @Jose ... this is an issue already addressed...
> >     >     The decision to change offset values to mm will break previous
> >     >     footprints that have non zero offset.
> >     >     But I think this has been considered a 'small' disturb for
> users
> >     >     when the patch has been committed, as stated in a previous
> mail:
> >     >
> >     >         This is not a big issue because the only effects the
> >     footprints
> >     >         embedded
> >     >         in the board.  Users with custom footprint libraries that
> >     contain 3D
> >     >         model offsets will just have to fix the offsets.  I'm
> guessing
> >     >         this is a
> >     >         fairly small number of users.
> >     >
> >     >     https://lists.launchpad.net/kicad-developers/msg31589.html
> >     <https://lists.launchpad.net/kicad-developers/msg31589.html>
> >     >     <https://lists.launchpad.net/kicad-developers/msg31589.html
> >     <https://lists.launchpad.net/kicad-developers/msg31589.html>>
> >     >
> >     >     M
> >     >
> >     >
> >     >     On 11/22/2017 2:19 AM, Wayne Stambaugh wrote:
> >     >
> >     >         I'm not sure I understand what the issue is.  Once an
> >     offset is
> >     >         changed
> >     >         to mm when either a footprint in a board or a library is
> >     parsed, why
> >     >         would it not be saved as mm.  If it isn't, then this is a
> >     bug.
> >     >         Once the
> >     >         footprint offset is converted to mm, there should be no
> >     >         expectation that
> >     >         it will be correct for older versions of KiCad.  Is there
> >     >         something else
> >     >         at play here?
> >     >
> >     >         On 11/21/2017 04:26 PM, Oliver Walters wrote:
> >     >
> >     >             Wayne,
> >     >
> >     >             Not sure how you want to handle this but I feel that
> >     making
> >     >             a clean
> >     >             break and using "offset" for mm solves all the issues
> >     >             associated with
> >     >             embedded footprints without version info, as Maurice
> says
> >     >             above. Let me
> >     >             know if want me to implement.
> >     >
> >     >             On Wed, Nov 22, 2017 at 8:24 AM, easyw
> >     <easyw@xxxxxxxxxxxx <mailto:easyw@xxxxxxxxxxxx>
> >     >             <mailto:easyw@xxxxxxxxxxxx <mailto:easyw@xxxxxxxxxxxx
> >>
> >     >             <mailto:easyw@xxxxxxxxxxxx <mailto:easyw@xxxxxxxxxxxx>
> >     <mailto:easyw@xxxxxxxxxxxx <mailto:easyw@xxxxxxxxxxxx>>>> wrote:
> >     >
> >     >                  Hi,
> >     >                  first headache symptom...
> >     >
> >     >                  Testing conditions:
> >     >                  latest KiCad patched
> >     >                  Application: pcbnew
> >     >                  Version: (2017-11-21 revision 8de70f3)-master,
> >     release
> >     >             build
> >     >
> >     >                  If you edit a footprint adding 3D models offset
> and
> >     >             then export it,
> >     >                  it will be saved with the new mm convention...
> >     >                  but when re-imported it will be read with
> >     deci-mils and
> >     >             displayed
> >     >                  with wrong convention...
> >     >                  Moreover if the imported footprint will be
> inserted
> >     >             into the board,
> >     >                  the footprint will conserve the wrong values...
> >     >                  Those wrong values will be then saved with the new
> >     >             kicad_pcb board....
> >     >
> >     >
> >     >     _______________________________________________
> >     >     Mailing list: https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/~kicad-developers>
> >     >     <https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/~kicad-developers>>
> >     >     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >     >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
> >     >     Unsubscribe : https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/~kicad-developers>
> >     >     <https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/~kicad-developers>>
> >     >     More help   : https://help.launchpad.net/ListHelp
> >     <https://help.launchpad.net/ListHelp>
> >     >     <https://help.launchpad.net/ListHelp
> >     <https://help.launchpad.net/ListHelp>>
> >     >
> >     >
> >
> >     _______________________________________________
> >     Mailing list: https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/~kicad-developers>
> >     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >     Unsubscribe : https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/~kicad-developers>
> >     More help   : https://help.launchpad.net/ListHelp
> >     <https://help.launchpad.net/ListHelp>
> >
>

From 43590f966f1300b9157377e895c58f8836c36bf9 Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@xxxxxxxxx>
Date: Mon, 27 Nov 2017 08:20:02 +1100
Subject: [PATCH 2/2] Decode "offset" in STEP export tool

- Read "at" as inches
- Read "offset" as mm
---
 pcbnew/kicad_plugin.cpp             |  2 ++
 utils/kicad2step/pcb/kicadmodel.cpp | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp
index 2c75f27..79ef1cf 100644
--- a/pcbnew/kicad_plugin.cpp
+++ b/pcbnew/kicad_plugin.cpp
@@ -1142,7 +1142,9 @@ void PCB_IO::format( MODULE* aModule, int aNestLevel ) const
 
             /* Write 3D model offset in mm
              * 4.0.x wrote "at" which was actually in inches
+             * 5.0.x onwards, 3D model offset is written using "offset"
              */
+
             m_out->Print( aNestLevel+2, "(offset (xyz %s %s %s))\n",
                           Double2Str( bs3D->m_Offset.x ).c_str(),
                           Double2Str( bs3D->m_Offset.y ).c_str(),
diff --git a/utils/kicad2step/pcb/kicadmodel.cpp b/utils/kicad2step/pcb/kicadmodel.cpp
index 6f680e0..61395ba 100644
--- a/utils/kicad2step/pcb/kicadmodel.cpp
+++ b/utils/kicad2step/pcb/kicadmodel.cpp
@@ -80,12 +80,37 @@ bool KICADMODEL::Read( SEXPR::SEXPR* aEntry )
         std::string name = child->GetChild( 0 )->GetSymbol();
         bool ret = true;
 
+        /*
+         * Version 4.x and prior used 'at' parameter,
+         * which was specified in inches.
+         */
         if( name == "at" )
+        {
             ret = Get3DCoordinate( child->GetChild( 1 ), m_offset );
+
+            if( ret )
+            {
+                m_offset.x *= 25.4f;
+                m_offset.y *= 25.4f;
+                m_offset.z *= 25.4f;
+            }
+        }
+        /*
+         * From 5.x onwards, 3D model is provided in 'offset',
+         * which is in millimetres
+         */
+        else if( name == "offset" )
+        {
+            ret = Get3DCoordinate( child->GetChild( 1 ), m_offset );
+        }
         else if( name == "scale" )
+        {
             ret = Get3DCoordinate( child->GetChild( 1 ), m_scale );
+        }
         else if( name == "rotate" )
+        {
             ret = GetXYZRotation( child->GetChild( 1 ), m_rotation );
+        }
 
         if( !ret )
             return false;
-- 
2.7.4

From 69367cf1954b01f74da30e6ac988a714243f1dc5 Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@xxxxxxxxx>
Date: Thu, 23 Nov 2017 21:21:48 +1100
Subject: [PATCH 1/2] CHANGE file format for 3D model offset

- Reverts behaviour of 3e71ed2421f5df433c11a554e0756f6898e25b06
- 3D model offset now explicitly written in mm, using "offset" tag
- Any read "at" tag is converted from inches to mm
- This will not break any 3D model offset when reading files
- It will however render files incompatible with old versions
---
 pcbnew/kicad_plugin.cpp |  6 ++++--
 pcbnew/kicad_plugin.h   |  3 ++-
 pcbnew/pcb_parser.cpp   | 35 ++++++++++++++++++++---------------
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp
index c4477ad..2c75f27 100644
--- a/pcbnew/kicad_plugin.cpp
+++ b/pcbnew/kicad_plugin.cpp
@@ -1140,8 +1140,10 @@ void PCB_IO::format( MODULE* aModule, int aNestLevel ) const
             m_out->Print( aNestLevel+1, "(model %s\n",
                           m_out->Quotew( bs3D->m_Filename ).c_str() );
 
-            // Model offset is in mm
-            m_out->Print( aNestLevel+2, "(at (xyz %s %s %s))\n",
+            /* Write 3D model offset in mm
+             * 4.0.x wrote "at" which was actually in inches
+             */
+            m_out->Print( aNestLevel+2, "(offset (xyz %s %s %s))\n",
                           Double2Str( bs3D->m_Offset.x ).c_str(),
                           Double2Str( bs3D->m_Offset.y ).c_str(),
                           Double2Str( bs3D->m_Offset.z ).c_str() );
diff --git a/pcbnew/kicad_plugin.h b/pcbnew/kicad_plugin.h
index 87dc736..7423b4f 100644
--- a/pcbnew/kicad_plugin.h
+++ b/pcbnew/kicad_plugin.h
@@ -45,7 +45,8 @@ class NETINFO_MAPPING;
 //#define SEXPR_BOARD_FILE_VERSION    20170123  // EDA_TEXT refactor, moved 'hide'
 //#define SEXPR_BOARD_FILE_VERSION    20170920  // long pad names and custom pad shape
 //#define SEXPR_BOARD_FILE_VERSION    20170922  // Keepout zones can exist on multiple layers
-#define SEXPR_BOARD_FILE_VERSION      20171114  // Save 3D model offset in mm, instead of inches
+//#define SEXPR_BOARD_FILE_VERSION    20171114  // Save 3D model offset in mm, instead of inches
+#define SEXPR_BOARD_FILE_VERSION      20171123  // 3D model offset written using "offset" parameter
 
 #define CTL_STD_LAYER_NAMES         (1 << 0)    ///< Use English Standard layer names
 #define CTL_OMIT_NETS               (1 << 1)    ///< Omit pads net names (useless in library)
diff --git a/pcbnew/pcb_parser.cpp b/pcbnew/pcb_parser.cpp
index 494b91e..6bb40ff 100644
--- a/pcbnew/pcb_parser.cpp
+++ b/pcbnew/pcb_parser.cpp
@@ -360,25 +360,30 @@ MODULE_3D_SETTINGS* PCB_PARSER::parse3DModel()
                 Expecting( T_xyz );
 
             /* Note:
-             * Prior to SEXPR_BOARD_FILE_VERSION 20171114,
-             * 3D model offset was read and written in inches
-             *
-             * Now, the offset is explicitly written in mm.
-             * If a board is read with an older version,
-             * the offset is converted from inches to mm
+             * Prior to KiCad v5, model offset was designated by "at",
+             * and the units were in inches.
+             * Now we use mm, but support reading of legacy files
              */
 
+            n3D->m_Offset.x = parseDouble( "x value" ) * 25.4f;
+            n3D->m_Offset.y = parseDouble( "y value" ) * 25.4f;
+            n3D->m_Offset.z = parseDouble( "z value" ) * 25.4f;
+            NeedRIGHT();
+            break;
+
+        case T_offset:
+            NeedLEFT();
+            token = NextTok();
+
+            if( token != T_xyz )
+                Expecting( T_xyz );
+
+            /*
+             * 3D model offset is in mm
+             */
             n3D->m_Offset.x = parseDouble( "x value" );
             n3D->m_Offset.y = parseDouble( "y value" );
             n3D->m_Offset.z = parseDouble( "z value" );
-
-            if(m_requiredVersion < 20171114L)
-            {
-                n3D->m_Offset.x *= 25.4f;
-                n3D->m_Offset.y *= 25.4f;
-                n3D->m_Offset.z *= 25.4f;
-            }
-
             NeedRIGHT();
             break;
 
@@ -409,7 +414,7 @@ MODULE_3D_SETTINGS* PCB_PARSER::parse3DModel()
             break;
 
         default:
-            Expecting( "at, scale, or rotate" );
+            Expecting( "at, offset, scale, or rotate" );
         }
 
         NeedRIGHT();
-- 
2.7.4