Pcbnew: Geda plugin fixes.

* Prevent cache from being reloaded on every footprint file read.
* Fix a bug forming wxFileName in GPCB_FP_CACHE::Load().
* Fix invalid file name time stamp debug assertion.
* Use FPID to set loaded footprint name.
* Remove file name <> from exception strings per UIPolicy.

pcbnew/gpcb_plugin.cpp

@@ -135,12 +135,19 @@ GPCB_FPL_CACHE_ITEM::GPCB_FPL_CACHE_ITEM( MODULE* aModule, const wxFileName& aFi
     m_module( aModule )
 {
     m_file_name = aFileName;
-    m_mod_time.Now();
+
+    if( m_file_name.FileExists() )
+        m_mod_time = m_file_name.GetModificationTime();
+    else
+        m_mod_time.Now();
 }
 
 
 bool GPCB_FPL_CACHE_ITEM::IsModified() const
 {
+    if( !m_file_name.FileExists() )
+        return false;
+
     return m_file_name.GetModificationTime() != m_mod_time;
 }
 
@@ -207,9 +214,35 @@ public:
 
     void Remove( const wxString& aFootprintName );
 
-    wxDateTime GetLibModificationTime();
+    wxDateTime GetLibModificationTime() const;
+
+    /**
+     * Function IsModified
+     * check if the footprint cache has been modified relative to \a aLibPath
+     * and \a aFootprintName.
+     *
+     * @param aLibPath is a path to test the current cache library path against.
+     * @param aFootprintName is the footprint name in the cache to test.  If the footprint
+     *                       name is empty, the all the footprint files in the library are
+     *                       checked to see if they have been modified.
+     * @return true if the cache has been modified.
+     */
+    bool IsModified( const wxString& aLibPath,
+                     const wxString& aFootprintName = wxEmptyString ) const;
 
-    bool IsModified();
+    /**
+     * Function IsPath
+     * checks if \a aPath is the same as the current cache path.
+     *
+     * This tests paths by converting \a aPath using the native separators.  Internally
+     * #FP_CACHE stores the current path using native separators.  This prevents path
+     * miscompares on Windows due to the fact that paths can be stored with / instead of \\
+     * in the footprint library table.
+     *
+     * @param aPath is the library path to test against.
+     * @return true if \a aPath is the same as the cache path.
+     */
+    bool IsPath( const wxString& aPath ) const;
 };
 
 
@@ -220,8 +253,11 @@ GPCB_FPL_CACHE::GPCB_FPL_CACHE( GPCB_PLUGIN* aOwner, const wxString& aLibraryPat
 }
 
 
-wxDateTime GPCB_FPL_CACHE::GetLibModificationTime()
+wxDateTime GPCB_FPL_CACHE::GetLibModificationTime() const
 {
+    if( !m_lib_path.DirExists() )
+        return wxDateTime::Now();
+
     return m_lib_path.GetModificationTime();
 }
 
@@ -232,7 +268,7 @@ void GPCB_FPL_CACHE::Load()
 
     if( !dir.IsOpened() )
     {
-        THROW_IO_ERROR( wxString::Format( _( "footprint library path <%s> does not exist" ),
+        THROW_IO_ERROR( wxString::Format( _( "footprint library path '%s' does not exist" ),
                                           m_lib_path.GetPath().GetData() ) );
     }
 
@@ -244,21 +280,16 @@ void GPCB_FPL_CACHE::Load()
 
     do
     {
-        wxFileName fn = fpFileName;
-        fn.SetPath( m_lib_path.GetPath() );
+        wxFileName fn( m_lib_path.GetPath(), fpFileName );
 
         // reader now owns fp, will close on exception or return
         FILE_LINE_READER reader( fn.GetFullPath() );
         std::string      name = TO_UTF8( fn.GetName() );
-        MODULE*          module = parseMODULE( &reader );
+        MODULE*          footprint = parseMODULE( &reader );
 
-        // Set the module name to the file name sans path and extension.
-        if( module->Reference().GetText().IsEmpty() )
-        {
-            module->Reference().SetText( fn.GetName() );
-        }
-
-        m_modules.insert( name, new GPCB_FPL_CACHE_ITEM( module, fn.GetName() ) );
+        // The footprint name is the file name without the extension.
+        footprint->SetFPID( fn.GetName() );
+        m_modules.insert( name, new GPCB_FPL_CACHE_ITEM( footprint, fn.GetName() ) );
 
     } while( dir.GetNext( &fpFileName ) );
 
@@ -290,29 +321,56 @@ void GPCB_FPL_CACHE::Remove( const wxString& aFootprintName )
 }
 
 
-bool GPCB_FPL_CACHE::IsModified()
+bool GPCB_FPL_CACHE::IsPath( const wxString& aPath ) const
 {
-    if( !m_lib_path.DirExists() )
+    // Converts path separators to native path separators
+    wxFileName newPath;
+    newPath.AssignDir( aPath );
+
+    return m_lib_path == newPath;
+}
+
+
+bool GPCB_FPL_CACHE::IsModified( const wxString& aLibPath, const wxString& aFootprintName ) const
+{
+    // The library is modified if the library path got deleted or changed.
+    if( !m_lib_path.DirExists() || !IsPath( aLibPath ) )
         return true;
 
-    for( MODULE_ITER it = m_modules.begin();  it != m_modules.end();  ++it )
+    // If no footprint was specified, check every file modification time against the time
+    // it was loaded.
+    if( aFootprintName.IsEmpty() )
     {
-        wxFileName fn = it->second->GetFileName();
-
-        if( !fn.FileExists() )
+        for( MODULE_CITER it = m_modules.begin();  it != m_modules.end();  ++it )
         {
-            wxLogTrace( traceFootprintLibrary, wxT( "Footprint cache file '%s' does not exist." ),
-                        fn.GetFullPath().GetData() );
-            return true;
+            wxFileName fn = m_lib_path;
+
+            fn.SetName( it->second->GetFileName().GetName() );
+            fn.SetExt( KiCadFootprintFileExtension );
+
+            if( !fn.FileExists() )
+            {
+                wxLogTrace( traceFootprintLibrary,
+                            wxT( "Footprint cache file '%s' does not exist." ),
+                            fn.GetFullPath().GetData() );
+                return true;
+            }
+
+            if( it->second->IsModified() )
+            {
+                wxLogTrace( traceFootprintLibrary,
+                            wxT( "Footprint cache file '%s' has been modified." ),
+                            fn.GetFullPath().GetData() );
+                return true;
+            }
         }
+    }
+    else
+    {
+        MODULE_CITER it = m_modules.find( TO_UTF8( aFootprintName ) );
 
-        if( it->second->IsModified() )
-        {
-            wxLogTrace( traceFootprintLibrary,
-                        wxT( "Footprint cache file '%s' has been modified." ),
-                        fn.GetFullPath().GetData() );
+        if( it == m_modules.end() || it->second->IsModified() )
             return true;
-        }
     }
 
     return false;
@@ -811,9 +869,9 @@ void GPCB_PLUGIN::init( const PROPERTIES* aProperties )
 }
 
 
-void GPCB_PLUGIN::cacheLib( const wxString& aLibraryPath )
+void GPCB_PLUGIN::cacheLib( const wxString& aLibraryPath, const wxString& aFootprintName )
 {
-    if( !m_cache || m_cache->GetPath() != aLibraryPath || m_cache->IsModified() )
+    if( !m_cache || m_cache->IsModified( aLibraryPath, aFootprintName ) )
     {
         // a spectacular episode in memory management:
         delete m_cache;
@@ -823,8 +881,8 @@ void GPCB_PLUGIN::cacheLib( const wxString& aLibraryPath )
 }
 
 
-wxArrayString GPCB_PLUGIN::FootprintEnumerate( const wxString& aLibraryPath,
-                                               const PROPERTIES*     aProperties )
+wxArrayString GPCB_PLUGIN::FootprintEnumerate( const wxString&   aLibraryPath,
+                                               const PROPERTIES* aProperties )
 {
     LOCALE_IO   toggle;     // toggles on, then off, the C locale.
 
@@ -852,7 +910,7 @@ MODULE* GPCB_PLUGIN::FootprintLoad( const wxString& aLibraryPath, const wxString
 
     init( aProperties );
 
-    cacheLib( aLibraryPath );
+    cacheLib( aLibraryPath, aFootprintName );
 
     const MODULE_MAP& mods = m_cache->GetModules();
 
@@ -868,7 +926,8 @@ MODULE* GPCB_PLUGIN::FootprintLoad( const wxString& aLibraryPath, const wxString
 }
 
 
-void GPCB_PLUGIN::FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName, const PROPERTIES* aProperties )
+void GPCB_PLUGIN::FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName,
+                                   const PROPERTIES* aProperties )
 {
     LOCALE_IO   toggle;     // toggles on, then off, the C locale.
 
@@ -878,7 +937,7 @@ void GPCB_PLUGIN::FootprintDelete( const wxString& aLibraryPath, const wxString&
 
     if( !m_cache->IsWritable() )
     {
-        THROW_IO_ERROR( wxString::Format( _( "Library <%s> is read only" ),
+        THROW_IO_ERROR( wxString::Format( _( "Library '%s' is read only" ),
                                           aLibraryPath.GetData() ) );
     }
 
@@ -897,7 +956,7 @@ bool GPCB_PLUGIN::FootprintLibDelete( const wxString& aLibraryPath, const PROPER
 
     if( !fn.IsDirWritable() )
     {
-        THROW_IO_ERROR( wxString::Format( _( "user does not have permission to delete directory <%s>" ),
+        THROW_IO_ERROR( wxString::Format( _( "user does not have permission to delete directory '%s'" ),
                                           aLibraryPath.GetData() ) );
     }
 
@@ -905,7 +964,7 @@ bool GPCB_PLUGIN::FootprintLibDelete( const wxString& aLibraryPath, const PROPER
 
     if( dir.HasSubDirs() )
     {
-        THROW_IO_ERROR( wxString::Format( _( "library directory <%s> has unexpected sub-directories" ),
+        THROW_IO_ERROR( wxString::Format( _( "library directory '%s' has unexpected sub-directories" ),
                                           aLibraryPath.GetData() ) );
     }
 
@@ -924,7 +983,7 @@ bool GPCB_PLUGIN::FootprintLibDelete( const wxString& aLibraryPath, const PROPER
 
             if( tmp.GetExt() != KiCadFootprintFileExtension )
             {
-                THROW_IO_ERROR( wxString::Format( _( "unexpected file <%s> was found in library path '%s'" ),
+                THROW_IO_ERROR( wxString::Format( _( "unexpected file '%s' was found in library path '%s'" ),
                                                   files[i].GetData(), aLibraryPath.GetData() ) );
             }
         }
@@ -942,7 +1001,7 @@ bool GPCB_PLUGIN::FootprintLibDelete( const wxString& aLibraryPath, const PROPER
     // we don't want that.  we want bare metal portability with no UI here.
     if( !wxRmdir( aLibraryPath ) )
     {
-        THROW_IO_ERROR( wxString::Format( _( "footprint library <%s> cannot be deleted" ),
+        THROW_IO_ERROR( wxString::Format( _( "footprint library '%s' cannot be deleted" ),
                                           aLibraryPath.GetData() ) );
     }
 

pcbnew/gpcb_plugin.h

@@ -62,12 +62,14 @@ public:
         return wxT( "fp" );
     }
 
-    wxArrayString FootprintEnumerate( const wxString& aLibraryPath, const PROPERTIES* aProperties = NULL);
+    wxArrayString FootprintEnumerate( const wxString& aLibraryPath,
+                                      const PROPERTIES* aProperties = NULL);
 
     MODULE* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
                            const PROPERTIES* aProperties = NULL );
 
-    void FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName, const PROPERTIES* aProperties = NULL );
+    void FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName,
+                          const PROPERTIES* aProperties = NULL );
 
     bool FootprintLibDelete( const wxString& aLibraryPath, const PROPERTIES* aProperties = NULL );
 
@@ -83,17 +85,16 @@ public:
 
 protected:
 
-    wxString        m_error;        ///< for throwing exceptions
-    const PROPERTIES*     m_props;        ///< passed via Save() or Load(), no ownership, may be NULL.
-    GPCB_FPL_CACHE* m_cache;        ///< Footprint library cache.
-    int             m_ctl;
-
-    LINE_READER*    m_reader;       ///< no ownership here.
-    wxString        m_filename;     ///< for saves only, name is in m_reader for loads
+    wxString          m_error;        ///< for throwing exceptions
+    const PROPERTIES* m_props;        ///< passed via Save() or Load(), no ownership, may be NULL.
+    GPCB_FPL_CACHE*   m_cache;        ///< Footprint library cache.
+    int               m_ctl;
+    LINE_READER*      m_reader;       ///< no ownership here.
+    wxString          m_filename;     ///< for saves only, name is in m_reader for loads
 
 private:
     /// we only cache one footprint library for now, this determines which one.
-    void cacheLib( const wxString& aLibraryPath );
+    void cacheLib( const wxString& aLibraryPath, const wxString& aFootprintName = wxEmptyString );
 
     void init( const PROPERTIES* aProperties );
 };