kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

On 03/09/2014 01:59 PM, Dick Hollenbeck wrote:
> Hi Jean-Samuel,
> 
> I am assuming that you would like this work to be committed as part kicad source?  If not
> please let me know.  With my assumption in place, we'll have to achieve a minimum amount
> of general usability, and that may extend beyond what you need it for.
> 
> Please be aware of this.
> 
> 
>> Hi,
>>
>> For starting I had implement your b) option. For that I start from a copy of guthub
>> plugin. So I add options in all
>> needed files (CMakeLists.txt..), I had create a directory on pcbnew and put the code
>> inside and an example.
>>
>> Out of this directory I had done some small changes:
>> scripting/python_scripting.h
>> => Adding global value g_PythonMainTState as extern (needed for python locks)
>> pcbnew/plugin.cpp
>> => move python_footprint_plugin option to my plugin (as comment suggest). I hope it's ok.
>> pcbnew/io_mgr.h
>> pcbnew/io_mgr.cpp
>> => Declare my plugin on lists
>>
>> On the new directory:
>>
>> pcbnew/fppython/fppython_example.py
>> A plugin example.
>> It's a really basic:
>>  - 2 statics foot prints
>>  - footprint himself are hardcoded on the example
>>  - fake save/delete
>>
>> pcbnew/fppython/fppython_plugin.(cpp|h)
>> The plugin himself
>>
>> To use is:
>> - add a new footprint lib entry,
>> - specify the directory where the plugin is as library path
> 
> 
> The Footprint*() library path argument should always only be used as a pointer to the
> library, not as a pointer to the plugin.
> 
> 
> A normal python environment variable should know where a python module is.  That should be
> the fallback in any case, with possible augmentation as described below, which is only an
> *optional* override for module loading.
> 
> Additionally, you can look for an optional fp_lib_table property, one spelled identically
> to a standard python environment variable name, which if present, can trump the
> environment variable.
> 
> 
> 
>> - specify python_footprint_plugin option as the plugin name (without .py, for example
>> fppython_example)
> 
> good.
> 
> 
>> Some limitations I found inside the core to allow more feature for plugins:
>> - IsFootprintLibWritable don't have a PROPERTY parameter. So since the plugin module name
>> is inside this, I'm unable to load
>> the right python module to let it handle this method
> 
> Your python module loading should happen less frequently.
> 
> 1) The the association between the C++ adapter instance and the python module can and
> should be retentative up until
> it cannot be.
> 
> 2) The association between the module and the library can and should be retentative up
> until it cannot be.
> 
> 
> The easiest way to manage this is to have two member helpers in your adaptor:
> 
> bool PYTHON_ADAPTOR::changedLibPath( const wxString& aCurrentLibPath )
> {
>     if( aCurrentLibPath != m_lib_path )
>     {
> 	m_lib_path = aCurrentLibPath;
> 	return true;
>     }
>     return false;
> }
> 
> PLUS, a similar function for python module name.
> 
> Then only change libraries, when changedLibPath() returns true.  Only change modules when
> changedModule() returns true.
> 
> This way your module and library can both be cached in the PYTHON_ADAPTER.  User's will
> instantiate a new PYTHON_ADAPTER for any number of libraries.
> 
> But 99% of the time, your python interpreter (or sub-interpreter) instantiation should
> survive multiple calls to the adaptor, and the module should be left loaded, and the
> library should be left open.
> 
> We can add PROPERTIES argument to IsFootprintLibWritable() but if you follow my design
> suggestions, you can survive without this additional argument for now.  You will have a
> record of the m_module's last value already.
> 
> 
>> - FootprintLibOptions: Same issue. So the plugin can't provide a list of parameters needed
>> for his own usage. 
> 
> In the C++ why don't you refer people to the pydocs for the module.  I don't want to have
> to load the python module in the OptionsEditor, where the plugin may instantiated just to
> get the supported parameters.  Let's refer people to external documentation in the help
> text for the certainly present python_footprint_plugin option.
> 
> That way each python module can later query for the options it might expect.  The user can
> supply additional options, even though they are not initially part of the supplied
> choices, in the options editor.
> 
> 
> 
>> To work arround this
>> I hard code 4 parameters (fppython_parameter[1-4]) but it is not beautiful...
> 
> Those could be removed, if the user can add his own in the OptionEditor with more
> meaningful names.
> 
> I think we have to prove viability of this entire python plugin module first before we tip
> the boat over.
> 
> 
> Also, remember that its going to be impossible to have more than one running python
> interpreter in a single process.  Python simply does not support that.  It does however
> support "sub-interpreters".  As the top level Kicad GUI transforms into wxPython, you will
> not be able to instantiate a brand new python interpreter in your PYTHON_ADAPTOR, for two
> reasons:
> 
> a) only one interpreter per process, and the main one is way above you.
> 
> b) you should not assume that the user only has one PYTHON_ADAPTOR in the fp_lib_table,
> and you are limited to only one interpreter per process.
> 
> So if you are not already an expert on python "sub-interpreters", you need to make that
> your highest priority, because your work will never get committed if you cannot conform to
> the overall design constraints of the single python interpreter constraint.


Jean-Samuel,

Another thing to be aware of is that currently the FootprintLoad() function has to be
thread safe.  This may not have been the case when I planted the seed for this idea you had.

But FootprintLoad() is currently thread safe for all implementing PLUGINs.  For any
instance of the PYTHON_ADAPTOR, you can be sure that only a single thread will call on
each PYTHON_ADAPTOR instance at a time.

The only place that I am aware of that causes this is

bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* aNickname )

near line 185 in common/footprint_info.cpp.  This code is called when the user presses the
"Load All" button in the footprint picker.

Its possible that a static MUTLOCK will give you the protection you need in the
PYTHON_ADAPTOR.  But this is getting hairy now, and I'm not going to have the time to help
you make python multithreaded.  The MUTLOCK may be a way out, but I don't have the brain
cycles to worry about something as obscure as this.

My vision has python on the top, and C++ underneath.  Your's is upside down from that.


Dick