kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

On 10/04/2013 03:41 PM, Dick Hollenbeck wrote:
> On 10/04/2013 03:30 PM, Dick Hollenbeck wrote:
>> On 10/04/2013 03:15 PM, Wayne Stambaugh wrote:
>>> On 10/4/2013 3:22 PM, Dick Hollenbeck wrote:
>>>>
>>>>>
>>>>> Thank you for your amazing effort on the GitHub plugin.  I am looking
>>>>> forward to taking it for a test drive.  I hope to finish up the
>>>>> FP_LIB_TABLE code and documentation by the end of the weekend.  With all
>>>>> of the pieces in place, KiCad should have a really good footprint
>>>>> library management system.
>>>>>
>>>>> Wayne
>>>>
>>>>
>>>> Thank you Wayne!
>>>>
>>>> I gave you bug https://bugs.launchpad.net/bugs/1235317   :)
>>>
>>> I noticed that :)  I took a quick look at the module editor and saving a
>>> library may not be the only place where this will be a problem.  The
>>> current code attempts to determine the correct plugin by the library
>>> path using IO_MGR::GuessPluginTypeFromLibPath().
> 
> Probably no environment substitution on lib path, if you want a quick hack.
> 
> The GITHUB plugin is not useable until the stuff below is complete however, and it will
> get done in any case:  The library browser needs to be based on the nickname now, for
> reasons I've mentioned in the last week.


Wayne,

Attached is the full implementation of the new API functions.  We just have to hook them
into the UI now.

The PLUGIN is instantiated in line 465 of the patched source.

There was no need to support PLUGIN::RELEASER::operator=(), I made it private to get the
compiler to whine.  Plugins are instantiated lazily, at line 465.

This is what we need to keep from destroying the PLUGINs between API calls and destroying
our CACHE work.

Dick


This one fixes a comment which is important to the understanding of the wxASSERT() in each
API function.  FP_LIB_TABLE::FindRow() does not ever return NULL, so the comment is was
wrong.  If it returns at all, the result is not NULL.

It also fully kills off FP_LIB_TABLE::PluginFind()



=== modified file 'common/fp_lib_table.cpp'
--- common/fp_lib_table.cpp	2013-10-04 22:40:25 +0000
+++ common/fp_lib_table.cpp	2013-10-05 00:20:16 +0000
@@ -59,6 +59,46 @@
 }
 
 
+wxArrayString FP_LIB_TABLE::FootprintEnumerate( const wxString& aNickname )
+{
+    const ROW* row = FindRow( aNickname );
+    wxASSERT( (PLUGIN*) row->plugin );
+    return row->plugin->FootprintEnumerate( row->GetFullURI(), row->GetProperties() );
+}
+
+
+MODULE* FP_LIB_TABLE::FootprintLoad( const wxString& aNickname, const wxString& aFootprintName )
+{
+    const ROW* row = FindRow( aNickname );
+    wxASSERT( (PLUGIN*) row->plugin );
+    return row->plugin->FootprintLoad( row->GetFullURI(), aFootprintName, row->GetProperties() );
+}
+
+
+void FP_LIB_TABLE::FootprintSave( const wxString& aNickname, const MODULE* aFootprint )
+{
+    const ROW* row = FindRow( aNickname );
+    wxASSERT( (PLUGIN*) row->plugin );
+    return row->plugin->FootprintSave( row->GetFullURI(), aFootprint, row->GetProperties() );
+}
+
+
+void FP_LIB_TABLE::FootprintDelete( const wxString& aNickname, const wxString& aFootprintName )
+{
+    const ROW* row = FindRow( aNickname );
+    wxASSERT( (PLUGIN*) row->plugin );
+    return row->plugin->FootprintDelete( row->GetFullURI(), aFootprintName, row->GetProperties() );
+}
+
+
+bool FP_LIB_TABLE::IsFootprintLibWritable( const wxString& aNickname )
+{
+    const ROW* row = FindRow( aNickname );
+    wxASSERT( (PLUGIN*) row->plugin );
+    return row->plugin->IsFootprintLibWritable( row->GetFullURI() );
+}
+
+
 void FP_LIB_TABLE::Parse( FP_LIB_TABLE_LEXER* in ) throw( IO_ERROR, PARSE_ERROR )
 {
     /*
@@ -335,9 +375,9 @@
 }
 
 
-const FP_LIB_TABLE::ROW* FP_LIB_TABLE::findRow( const wxString& aNickName )
+FP_LIB_TABLE::ROW* FP_LIB_TABLE::findRow( const wxString& aNickName ) const
 {
-    FP_LIB_TABLE* cur = this;
+    FP_LIB_TABLE* cur = (FP_LIB_TABLE*) this;
 
     do
     {
@@ -412,7 +452,7 @@
 const FP_LIB_TABLE::ROW* FP_LIB_TABLE::FindRow( const wxString& aLibraryNickName )
     throw( IO_ERROR )
 {
-    const ROW* row = findRow( aLibraryNickName );
+    ROW* row = findRow( aLibraryNickName );
 
     if( !row )
     {
@@ -421,10 +461,14 @@
         THROW_IO_ERROR( msg );
     }
 
+    if( !row->plugin )
+        row->setPlugin( IO_MGR::PluginFind( row->type ) );
+
     return row;
 }
 
 
+/*
 PLUGIN* FP_LIB_TABLE::PluginFind( const wxString& aLibraryNickName )
     throw( IO_ERROR )
 {
@@ -436,6 +480,7 @@
 
     return plugin;
 }
+*/
 
 
 const wxString FP_LIB_TABLE::ExpandSubstitutions( const wxString aString )

=== modified file 'include/fp_lib_table.h'
--- include/fp_lib_table.h	2013-10-04 22:40:25 +0000
+++ include/fp_lib_table.h	2013-10-05 00:17:30 +0000
@@ -147,6 +147,8 @@
             description = r.description;
             properties  = r.properties ? new PROPERTIES( *r.properties ) : NULL;
 
+            setPlugin( NULL );  // do not copy the PLUGIN, it is lazily created.
+
             return *this;
         }
 
@@ -258,13 +260,19 @@
             properties = aProperties;
         }
 
+        void setPlugin( PLUGIN* aPlugin )
+        {
+            plugin.set( aPlugin );
+        }
+
         wxString        nickName;
         wxString        uri;
         LIB_T           type;
         wxString        options;
         wxString        description;
-        const
-        PROPERTIES*     properties;
+
+        const PROPERTIES*   properties;
+        PLUGIN::RELEASER    plugin;
     };
 
     /**
@@ -362,11 +370,7 @@
      */
     std::vector<wxString> GetLogicalLibs();
 
-    //----<read accessors>----------------------------------------------------
-    // the returning of a const wxString* tells if not found, but might be too
-    // promiscuous?
-
-#if 0       // PLUGIN API SUBSET, REBASED ON aNickname
+    //-----<PLUGIN API SUBSET, REBASED ON aNickname>---------------------------
 
     /**
      * Function FootprintEnumerate
@@ -381,7 +385,7 @@
      *
      * @throw IO_ERROR if the library cannot be found, or footprint cannot be loaded.
      */
-    wxArrayString FootprintEnumerate( const wxString& aNickname ) const;
+    wxArrayString FootprintEnumerate( const wxString& aNickname );
 
     /**
      * Function FootprintLoad
@@ -397,7 +401,7 @@
      * @throw   IO_ERROR if the library cannot be found or read.  No exception
      *          is thrown in the case where aFootprintName cannot be found.
      */
-    MODULE* FootprintLoad( const wxString& aNickname, const wxString& aFootprintName ) const;
+    MODULE* FootprintLoad( const wxString& aNickname, const wxString& aFootprintName );
 
     /**
      * Function FootprintSave
@@ -436,8 +440,8 @@
      */
     bool IsFootprintLibWritable( const wxString& aNickname );
 
-#endif
-    //----</read accessors>---------------------------------------------------
+    //-----</PLUGIN API SUBSET, REBASED ON aNickname>---------------------------
+
 
     /**
      * Function InsertRow
@@ -455,13 +459,14 @@
      * Function PluginFind
      * returns a PLUGIN*.  Caller should wrap that in a PLUGIN::RELEASER()
      * so when it goes out of scope, IO_MGR::PluginRelease() is called.
-     */
     PLUGIN* PluginFind( const wxString& aLibraryNickName ) throw( IO_ERROR );
+     */
 
     /**
      * Function FindRow
      * returns a ROW if aNickName is found in this table or in any chained
-     * fallBack table fragment, else NULL.
+     * fallBack table fragment, else NULL.  The PLUGIN is loaded and attached
+     * to the "plugin" field of the ROW if not already loaded.
      */
     const ROW* FindRow( const wxString& aNickName ) throw( IO_ERROR );
 
@@ -564,7 +569,7 @@
      * returns a ROW if aNickName is found in this table or in any chained
      * fallBack table fragment, else NULL.
      */
-    const ROW* findRow( const wxString& aNickName );
+    ROW* findRow( const wxString& aNickName ) const;
 
     void reindex()
     {

=== modified file 'pcbnew/io_mgr.h'
--- pcbnew/io_mgr.h	2013-09-25 19:17:06 +0000
+++ pcbnew/io_mgr.h	2013-10-05 00:14:13 +0000
@@ -416,6 +416,9 @@
     {
         PLUGIN* plugin;
 
+        // private assignment operator so it's illegal
+        RELEASER& operator=( RELEASER& aOther ) { return *this; }
+
     public:
         RELEASER( PLUGIN* aPlugin = NULL ) :
             plugin( aPlugin )
@@ -425,15 +428,28 @@
         ~RELEASER()
         {
             if( plugin )
-                IO_MGR::PluginRelease( plugin );
-        }
-
-        operator PLUGIN* ()
+                release();
+        }
+
+        void release()
+        {
+            IO_MGR::PluginRelease( plugin );
+            plugin = NULL;
+        }
+
+        void set( PLUGIN* aPlugin )
+        {
+            if( plugin )
+                release();
+            plugin = aPlugin;
+        }
+
+        operator PLUGIN* () const
         {
             return plugin;
         }
 
-        PLUGIN* operator -> ()
+        PLUGIN* operator -> () const
         {
             return plugin;
         }