kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

Hi Mike,

Thanks for your time on this.

I have some comments below.

> This is a first approach.
>
> It's what I understood from your last email, my idea was more complicated.
>
> summary:
>
> 1)  Added to new plugin types:

I don' think we don't need these 2 new enum values.  This support goes into LEGACY and
KICAD plugins, respectively.  The idea is to bind the library technology with its
corresponding board technology, in the collection of API functions.  This way a developer
with knowledge on one kind of foreign file format set, can do a single plugin and get
everything in there.


> +        LEGACY_LIBRARY,                 //< Legacy Pcbnew library file format
> +        KICAD_LIBRARY,                  //< S-expression based library format
>
> 2)  Added a new load method to io_mgr.h (plugin is the same without the PCB_FILE_T)
>
> +    static MODULE* LoadModule( PCB_FILE_T aFileType, 
> +                                const wxString& aLibraryPath,
> +                                wxString& aModuleName,
> +                                BOARD* aAppendToBoard = NULL,
> +                                PROPERTIES* aProperties = NULL);
> +
>  
> I kept appendToBoard (makes sense), and properties (in the future it could be loading
> options for the plugin, like http credentials, or whatever).
>
> librarypath could be either: directory (new format), file (other formats), http uris
> (future?)
>  
> 3) Added SaveModule (io_mgr + plugin, as "2")
>
> +     static void SaveModule( PCB_FILE_T aFileType, const wxString& aLibraryPath,
> +                              MODULE* aModule, PROPERTIES* aProperties = NULL);
>
> I think it needs no explanation
>
> 4) Added ListModules  method (io_mgr + plugin, as "2,3"):
>
> +    static wxArrayString ListModules ( PCB_FILE_T aFileType,
> +                                       const wxString& aLibraryPath,
> +                                       PROPERTIES* aProperties = NULL);
> +    
>
> that will return an array of names with all the available modules in that library.
>
> Notes:
>    
>     a)  This design would be simple, and must work
>     a.1) The doxygen doc headers are compiled and checked.
>
> Drawbacks:
>     b)  Some plugins would only know how to open PCB files, some plugins will only open
> MODULES/libraries
>          b.1) Could make sense some interface to check the plugin capabilities or make a
> enum type for modules/libraries instead.


The implementor of the plugin can indeed implement a subset of all the PLUGIN API
functions.  Non-implemented ones return an error code.  The developer using the plugin
will know this in advance for the time being, since he can see the source code, and not
jump off a cliff not knowing his parachute package has no fabric in it.

The PLUGIN api is a way or organizing functionality, is not a way of providing surprises.


>     c)  There is the case of opening/saving single componente files (.cmp?), that would
> be almost the same plugin.

I don't want to support that, unless it can be done with the plugin with special status,
which is the "KICAD" plugin, and I think this is simply

SaveModule() on the KICAD plugin.  It comes out in s-expression format, ALWAYS.  And
because the "library" argument is actually only a directory for that plugin, you have your
CMP file, but it is always s-expresion.


>
> Future ideas (that I might want to add for py-scripting) -out of scope right now-:
>      e)  Dynamic plugin registration, then a python PLUGIN could get itself registered
> on IO_MGR to read/write formats: that would lead to faster format importer/exporters
> development.


Supporting a python plugin does not mandate dynamic C++ registration, if the python plugin
is known ahead of time.  However, it is not clear to me that keeping an "in process"
plugin in python is sensible.  The purpose of a plugin is to continually access a file of
a particular format category, over and over, without actually committing to converting it
over.


If you want to write a file *converter*, and want to do it in python, it does not have to
be a PLUGIN.  But if you want to seemlessly read and write EAGLE files, everyday of the
week from KiCad, C++ is the better choice for that plugin.

If you want to do something "out of process", like dynamically fabricate footprints using
an expert system, then I say you still have a C++ plugin in process, but its out of
process counterpart can be in python.


More comments in patch.


>
>
>
>
> 2012/4/12 Miguel Angel Ajo Pelayo <miguelangel@xxxxxxx <mailto:miguelangel@xxxxxxx>>
>
>
>
>     2012/4/12 Dick Hollenbeck <dick@xxxxxxxxxxx <mailto:dick@xxxxxxxxxxx>>
>
>         The changes to PLUGIN should be "additional functions for MODULEs".  I like what
>         we have
>         for BOARDs now.
>
>
>     Ok, I thought about this idea at first, but tried to make it too abstract ;), it's
>     simpler this way.
>      
>
>
>         The new work entails abstracting the *loading* and *saving* of a MODULE from/to
>         a "MODULE
>         library" (aka MODULE collection).
>
>         So I would expect to see MODULE* in most of the new function prototypes.
>
>         Along with loading and saving, one may have to know which MODULEs are in a
>         library, so in
>         comes MODULE *enumerating or listing*.
>
>
>     That would be extra functions for enumeration/listing, for example
>
>     List(library_name)=>list of[module names]
>      
>
>
>         I'm not saying anything positive or negative about caching, or when or if it should
>         happen.  I am open to suggestions on that.
>
>
>     Not ready to say anything about that, probably is unneeded to change anything there
>     at this moment. May be in the future with a library manager / HTTP_LIB /whatever,
>     could start making sense.
>      
>
>
>         I am not really even saying that this goal is achievable, since I have not yet spent
>         sufficient time on it myself.  In the near term, if we can jam in this PLUGIN
>         interface,
>         and re-position PCBNEW on top of it with respect to MODULE access, then we would
>         then
>         pickup the following types of MODULEs, even without converting them before using
>         them:
>
>
>         *) legacy KiCad modules
>         *) s-expression module in nanometers
>         *) Eagle module libraries ?
>         *) GEDA libraries ?
>
>
>
>     I think it's totally achievable.
>


This means you are starting to see "surgical points of insertion" of your new functions
into existing footprint library management code?


>     The question is now, can we do it for MODULEs from libraries, and do it in PLUGIN,
>     or does
>
>         it take a lot more?
>
>
>     I think it could, I'll finish my proposal and submit the patch to get opinions.
>



> io_mgr_lib.patch
>
>
> === modified file 'pcbnew/io_mgr.h'
> --- pcbnew/io_mgr.h	2012-04-07 18:05:56 +0000
> +++ pcbnew/io_mgr.h	2012-04-12 22:33:58 +0000
> @@ -49,6 +49,8 @@
>      {
>          LEGACY,                         //< Legacy Pcbnew file formats prior to s-expression.
>          KICAD,                          //< S-expression Pcbnew file format.
> +        LEGACY_LIBRARY,                 //< Legacy Pcbnew library file format
> +        KICAD_LIBRARY,                  //< S-expression based library format
>          // add your type here.
>  
>          // EAGLE,
> @@ -144,6 +146,92 @@
>       */
>      static void Save( PCB_FILE_T aFileType, const wxString& aFileName,
>                        BOARD* aBoard, PROPERTIES* aProperties = NULL );
> +    
> +    
> +    
> +    /**
> +     * Function ListModules
> +     * finds the requested PLUGIN and if found, calls the PLUGIN->ListModules(..) 
> +     * function on it using the arguments passed to this function.  
> +     * After the PLUGIN->ListModules() function returns, the PLUGIN 
> +     * is Released() as part of this call.
> +     *
> +     * @param aFileType is the PCB_FILE_T of file to load.
> +     *
> +     * @param aLibraryPath is the path to the library file or directory.
> +     *
> +     * @param aProperties is an associative array that can be used to tell the
> +     *  plugin how to access the library file.
> +     *  The caller continues to own this object (plugin may not delete it), and
> +     *  plugins should expect it to be optionally NULL.

I suppose we have to decide how and where we are going to support "searching".

The aProperties argument may be one possibility, or we can do this higher up, in the C++
MODULE realm.

The latter keeps the plugin simpler.  Leaving it out of searching, but requires that every
module be instantiated to do a rigorous search.

Check client library code and let me know what you think.


> +     *
> +     *  @return wxArrayString - is the array of available modules names inside  
> +     *   a library
> +     *
> +     * @throw IO_ERROR if the PLUGIN cannot be found, library cannot be found,
> +     *  or module cannot be loaded.
> +     */
> +    
> +    static wxArrayString ListModules ( PCB_FILE_T aFileType,
> +                                       const wxString& aLibraryPath,
> +                                       PROPERTIES* aProperties = NULL);
> +    
> +    /**
> +     * Function LoadModule
> +     * finds the requested PLUGIN and if found, calls the PLUGIN->LoadModule(..) 
> +     * function on it using the arguments passed to this function.  
> +     * After the PLUGIN->LoadModule() function returns, the PLUGIN 
> +     * is Released() as part of this call.
> +     *
> +     * @param aFileType is the PCB_FILE_T of file to load.
> +     *
> +     * @param aModuleName is the name of the module to load.
> +     *
> +     * @param aAppendToBoard is an existing BOARD to append the module to, 
> +     *         use NULL if you don't want the module added to a BOARD
> +     * 
> +     * @param aProperties is an associative array that can be used to tell the
> +     *  saver how to save the file, because it can take any number of
> +     *  additional named tuning arguments that the plugin is known to support.
> +     *  The caller continues to own this object (plugin may not delete it), and
> +     *  plugins should expect it to be optionally NULL.
> +     *
> +     * @return MODULE* - caller owns it unless it's added to a BOARD, in that
> +     *  case BOARD will take care of deletion when BOARD is deleted. Never NULL 
> +     *  because exception thrown if error.
> +     *    
> +     * @throw IO_ERROR if the PLUGIN cannot be found, library cannot be found,
> +     *  or module cannot be loaded.
> +     */
> +    static MODULE* LoadModule( PCB_FILE_T aFileType, 
> +                                const wxString& aLibraryPath,
> +                                wxString& aModuleName,

I don't understand why you kept this argument, aAppendToBoard:

> +                                BOARD* aAppendToBoard = NULL,
> +                                PROPERTIES* aProperties = NULL);
> +
> +    /**
> +     * Function SaveModule
> +     * will write a module to an existing library, or just create the library
> +     * if it doesn't exist
> +     *
> +     * @param aFileType is the PCB_FILE_T of file to save.
> +     *
> +     * @param aLibraryPath is the path of the library where we want the module
> +     *  to be stored in.
> +     *
> +     * @param aModule is the module object that we want to store in the library.
> +     *   The caller continues to own the MODULE.
> +     * 
> +     * @param aProperties is an associative array that can be used to tell the
> +     *  saver how to save the file, because it can take any number of
> +     *  additional named tuning arguments that the plugin is known to support.
> +     *  The caller continues to own this object (plugin may not delete it), and
> +     *  plugins should expect it to be optionally NULL.
> +     *
> +     * @throw IO_ERROR if there is a problem saving or exporting.
> +     */
> +     static void SaveModule( PCB_FILE_T aFileType, const wxString& aLibraryPath,
> +                              MODULE* aModule, PROPERTIES* aProperties = NULL);
>  };
>  
>  
> @@ -237,6 +325,81 @@
>      virtual void Save( const wxString& aFileName, BOARD* aBoard,
>                         PROPERTIES* aProperties = NULL );


The last 3 functions should be virtual not static if they are to go into the PLUGIN interface:

>  
> +    
> +    /**
> +     * Function ListModules
> +     * 
> +     * Get an array with the names of all available modules inside a pcb library.
> +     *
> +     * @param aLibraryPath is the path to the library file or directory.
> +     *
> +     * @param aProperties is an associative array that can be used to tell the
> +     *  plugin how to access the library.
> +     *  The caller continues to own this object (plugin may not delete it), and
> +     *  plugins should expect it to be optionally NULL.
> +     *
> +     *  @return wxArrayString - is the array of available modules names inside  
> +     *   a library
> +     *
> +     * @throw IO_ERROR if the PLUGIN cannot be found, library cannot be found,
> +     *  or module cannot be loaded.
> +     */
> +    
> +    static wxArrayString ListModules ( PCB_FILE_T aFileType,
> +                                       const wxString& aLibraryPath,
> +                                       PROPERTIES* aProperties = NULL);
> +     /**
> +     * Function LoadModule
> +     * 
> +     *  will load a MODULE object from the library format this PLUGIN 
> +     * knows about.
> +     *
> +     * @param aModuleName is the name of the module to load.
> +     *
> +     * @param aAppendToBoard is an existing BOARD to append the module to, 
> +     *         use NULL if you don't want the module added to a BOARD
> +     * 
> +     * @return MODULE* - caller owns it unless it's added to a BOARD, in that
> +     *  case BOARD will take care of deletion when BOARD is deleted. Never NULL 
> +     *  because exception thrown if error.
> +     *    
> +     * @param aProperties is an associative array that can be used to tell the
> +     *  saver how to save the file, because it can take any number of
> +     *  additional named tuning arguments that the plugin is known to support.
> +     *  The caller continues to own this object (plugin may not delete it), and
> +     *  plugins should expect it to be optionally NULL.
> +     *
> +     * @throw IO_ERROR if the PLUGIN cannot be found, library cannot be found,
> +     *  or module cannot be loaded.
> +     */
> +    static MODULE* LoadModule( const wxString& aLibraryPath,
> +                                wxString& aModuleName,

OMIT aAppendToBoard

> +                                BOARD* aAppendToBoard = NULL,
> +                                PROPERTIES* aProperties = NULL);
> +
> +    /**
> +     * Function SaveModule
> +     * will write a module to an existing library, or just create the library
> +     * if it doesn't exist
> +     *
> +     * @param aLibraryPath is the path of the library where we want the module
> +     *  to be stored in.
> +     *
> +     * @param aModule is the module object that we want to store in the library.
> +     *   The caller continues to own the MODULE.
> +     * 
> +     * @param aProperties is an associative array that can be used to tell the
> +     *  saver how to save the file, because it can take any number of
> +     *  additional named tuning arguments that the plugin is known to support.
> +     *  The caller continues to own this object (plugin may not delete it), and
> +     *  plugins should expect it to be optionally NULL.
> +     *
> +     * @throw IO_ERROR if there is a problem saving or exporting.
> +     */

Here we get the footprint name from the MODULE?  Seems probably good enough, but slight
chance it will not be.

> +     static void SaveModule( const wxString& aLibraryPath,
> +                              MODULE* aModule, PROPERTIES* aProperties = NULL);
> +    
> +    
>      //-----</PUBLIC PLUGIN API>------------------------------------------------
>  
>      /*  The compiler writes the "zero argument" constructor for a PLUGIN
>