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 <stambau...@gmail.com> > 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@lists.launchpad.net >>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>> More help : https://help.launchpad.net/ListHelp >>>> >>> >>> >>> >>> >>> _______________________________________________ >>> Mailing list: https://launchpad.net/~kicad-developers >>> Post to : kicad-developers@lists.launchpad.net >>> Unsubscribe : https://launchpad.net/~kicad-developers >>> More help : https://help.launchpad.net/ListHelp >>> >> >> >> _______________________________________________ >> Mailing list: https://launchpad.net/~kicad-developers >> Post to : kicad-developers@lists.launchpad.net >> Unsubscribe : https://launchpad.net/~kicad-developers >> More help : https://help.launchpad.net/ListHelp >> > > > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp >
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
