kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

Jean-Samuel,

I think you are on the correct path.

I guess you now understand that the user can add options of his own choosing in the
options editor, simply by adding a new row to that dialog under fp_lib_table dialog.

We had to cross this bridge anyways as part of my "modular-kicad" blueprint milestone
number C).

The only think I see different about what you are doing, is that I was expecting to wrap
the python C API calls into C++ classes.  I just spent a couple of hours seeing if boost
python gives us any help in this regard, and IT DOES NOT.


So before this is over KiCad may end up being one of the best places to look for how to
embed python in a C++ program.  Right now, the XBMC program mixes together C++ and python
in a very advanced way.  That source code may have good examples in it.  I have not had
time yet, but you see its listed in milestone C).


There are many things to consider, one of which is that you cannot know if you are the
topmost, pre-eminent interpreter, or need to be a sub-interpreter, without first asking if
such a main interpreter has already been created.


Before I offer any additional help on this, I will ask that you submit a patch to the
mailing list of your current work, or push a branch.  That will indicate your intention to
contribute this work, and will justify my continued help, because as I said, I need this
and more in milestone C).


If you send a patch, I will put it into a new pushed branch, and be in a position to help.


Thanks,

Dick


On 03/10/2014 08:44 AM, Jean-Samuel Reynaud wrote:
> Hi Dick, Hi list
> 
> I'm currently modify the plugin to met your points.
> 
> => now the plugin don't load python module each time
> => the lib path is now handled directly onto the python module and is
> not used anymore as the python module path.
> 
> => I had created a python sub-interpreter (associated into the adapter)
> This sub interpreter is a property of the adapter. So there are one
> interpreter per adapter instance.
> 
> So on the contructor I have:
> ...
>     m_pythread = Py_NewInterpreter();
> ...
> 
> then when I use it I do
> ....
>     PyEval_AcquireLock();
>     PyThreadState_Swap(m_pythread);
> // speak with python/module...
>      PyThreadState_Swap(g_PythonMainTState);
>      PyEval_ReleaseLock();
> 
> Do you think it is ok ?
> 
> I understand that this is the opposite way of your idea (python at top
> and C++ at the bottom) but I don't know if it's possible (in current
> state) to do this by an other way.
> 
> Regards
> 
> Le 09/03/2014 20:22, Dick Hollenbeck a écrit :
>> 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
>>
>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>