kicad-developers team mailing list archive

[Maciej Sumiński <maciej.suminski@xxxxxxx>]

In my opinion #3 is acceptable. It does not require any extra changes in
the existing scripts and it fixes the problem until we come up with a
better scripting interface. If nobody has objections, I am going to
commit the patch.

Regards,
Orson

On 03/06/2018 10:02 AM, miles mccoo wrote:
> Thanks all for your replies.
> 
> I like the plugin mechanism. It does some nice things for python folks.
> Refresh, undo, garbage collection (I think). I want to see it succeed.
> 
> From reading Orson's mail, I think I wasn't entirely clear, so let me try
> to fix that first.
> 
> 
> The plugin mechanism tries to track the changes of a python plugin. In the
> c++ world, developers are expected to inform BOARD_COMMIT that something
> has changed. The plugin infrastructure does this for you. One could compare
> it to garbage collection. My interpretation of how this is done is simple:
> before a plugin runs, make a list of all design objects. zones, tracks,
> modules, nets... after the plugin completes, do a "diff".
> 
> This is mostly fine with the exception of removed items.
> 
> The problem with removed items is how do you do a diff on a deleted, memory
> managed, cleaned up object? Perhaps that memory now contains a different
> kind of object. Perhaps it's been reallocated to a similar object as before.
> 
> 
> 
> *as it stands now, any plugin that removes items from a board item
> container will likely fail*. It's not quite a crash, but it has a similar
> feel to the user.
> 
> 
> 
> Solutions. I can think of four.
> 
> solution 1. Why is the plugin mechanism in the business of tracking
> changes? Shouldn't BOARD_COMMIT just magically happen whenever a c++
> instance is modified. I brought this up in a previous thread[1] and various
> reasons were given why this isn't desirable.
> 
> solution 2. Same as #1 except BC magic only happens on python API calls.
> The plugin mechanism would no longer do diffs. Just add BC checkpoints.
> This is probably a lot of work. I might be willing to do this work but it
> would take time. [2]
> 
> solution 3. easy to implement. turn off deletion on calls to remove if
> currently running a plugin
> 
>    - plugin gets a boolean: isPluginActive. set/unset around the call to
>    actual python run code.
>    - add pcbnew.isPluginActive() to python api. (I could imagine this could
>    have other uses)
>    - the remove code looks at isPluginActive to decide whether to set
>    isown. (that code is actually python, not swig)
> 
> I have #3 implemented and my plugins no longer crash. *See attached patch
> for if folks agree it is an acceptable stopgap.*
> 
> solution 4. probably not the direction I would go but a way to enable
> python memory management and do the plugin diff thing. Basically, give each
> object some sort of unique id. (could be useful for other things). In
> addition to holding a list of object pointers, plugin infrastructure would
> hold a list of object ids.
> 
> 
> *Given the ease with which a plugin can hit this issue and given how long
> it would take to get #2 right, I guess I'm recommending the changes of that
> attached patch for #3.*
> 
> Miles
> 
> [1] https://lists.launchpad.net/kicad-developers/msg32063.html
> [2] I think my personal opinion is that #1 is superior over #2. Python
> people shouldn't have to care but why is it that c++ should or want to?
> Yes, I read the arguments from the previous thread but I'm not convinced.
> I'm also just a kicad spectator, so there's a lot I don't know.
> 
> 
> 
> 
> 
> On Fri, Mar 2, 2018 at 2:47 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
> wrote:
> 
>> LOL, I just replied to Miles.  Thanks Orson for helping out!
>>
>> On 3/2/2018 8:36 AM, Maciej Sumiński wrote:
>>> Hi Miles,
>>>
>>> I suppose the silence in the thread indicates there are not many
>>> developers knowing the Python scripting interface inside out. Since you
>>> are both studying the scripting interface and developing own scripts, it
>>> is quite possible you are the most competent person to give us an advice
>>> on how to proceed. See some comments below, but I am neither a Python
>>> script developer nor a scripting interface maintainer, so I might be
>>> lacking some basic knowledge here.
>>>
>>> On 02/28/2018 05:12 PM, miles mccoo wrote:
>>>> So I'm plugin-ifying my python scripts (the mechanism is awesome). One
>> of
>>>> the plugins deletes some stuff and that is causing trouble.
>>>>
>>>>
>>>>
>>>> I'm not sure how to fix the root cause. Hence this mail.
>>>>
>>>>
>>>>
>>>> The plugin just deletes Edge.Cuts[1]:
>>>> for d in board.GetDrawings():
>>>>             if (d.GetLayerName() == 'Edge.Cuts'):
>>>>                 board.Remove(d)
>>>>
>>>>
>>>>
>>>> in board_item_container.i, I see this (with stuff deleted):
>>>> %rename(RemoveNative)    BOARD_ITEM_CONTAINER::Remove;
>>>>     def Remove(self,item):
>>>>         self.RemoveNative(item)
>>>>         item.thisown=1
>>>>
>>>>
>>>> Setting thisown tells, python "you own it". Delete it when you're done.
>>>> Which it does.
>>>>
>>>>
>>>> The problem this causes is that the plugin mechanism saves a list of all
>>>> objects before running the plugin and then checks if any of them has a
>> null
>>>> list after (ie is it still in the design).
>>>
>>> Is this mechanism implemented to handle memory leaks? If so, would not
>>> it be sufficient to stick to 'thisown' flag on Remove() calls or is
>>> there another way objects might be destroyed using Python scripts?
>>>
>>>> Since the object has been deleted by python, the plugin stuff gets
>> confused.
>>>>
>>>>
>>>> *So, the question is how to fix this?*
>>>>
>>>>
>>>> It appears that the plugin infrastructure will delete for you (that's
>> what
>>>> I'm guessing), so the thisown setting shouldn't be done.
>>>>
>>>>
>>>> On the other hand, if running code from within a standalone script (ie
>> from
>>>> regular python shell), now thisown'ing it would yield a memory leak.
>>>>
>>>>
>>>>
>>>> Perhaps the plugin stuff should have some sort of flag indicating
>> "you're
>>>> in a plugin". Then the thisown setting could be conditional.
>>>
>>> If the object listing mechanism is required for other reasons, then I
>>> suppose it is second best idea. Generally speaking, I would like to make
>>> the scripting interface convenient for the users, so they do not need to
>>> worry about whether their scripts are run standalone or as a plugin.
>>> Let's hide the dirty magic from them and make the coding process
>> enjoyable.
>>>
>>> Regards,
>>> Orson
>>>
>>>> But I'm just a spectator. *I'm happy to put in the time to fix this but
>>>> need guidance on what approach to take.*
>>>>
>>>>
>>>> Miles
>>>>
>>>>
>>>>
>>>> [1] full plugin text
>>>> import pcbnew
>>>>
>>>> class RemoveBoundaryPlugin(pcbnew.ActionPlugin):
>>>>     def defaults(self):
>>>>         self.name = "Remove boundary"
>>>>         self.category = "A descriptive category name"
>>>>         self.description = "This plugin reads a dxf file and converts
>> it to
>>>> a zone"
>>>>
>>>>     def Run(self):
>>>>         board = pcbnew.GetBoard()
>>>>
>>>>         for d in board.GetDrawings():
>>>>             print("{}".format(str(d)))
>>>>             #print("on layer {} {} {}".format(d.GetLayerName(),
>>>>             #                                 str(d.GetStart()),
>>>>             #                                 str(d.GetEnd())))
>>>>             if (d.GetLayerName() == 'Edge.Cuts'):
>>>>                 board.Remove(d)
>>>>
>>>> RemoveBoundaryPlugin().register()
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> 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
>>>
>>
>>
>> _______________________________________________
>> 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
>

Attachment: signature.asc
Description: OpenPGP digital signature