← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] expose BOARD_COMMIT to python

 

Is the patch required? It appears that I don't need it.

However...

I would like to know what formatting errors I made in the patch. Avoid it
next time.

For the other folks involved in the earlier mail threads, they seemed to
think exposing this is a good idea.

   - Perhaps they still do and so this patch would be helpful for them
   - Perhaps they never did see the need, in which case I would say, "Grrr!"


Miles

On Mon, Feb 26, 2018 at 1:59 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:

> Miles,
>
> Does this mean your patch is not required?  If it's not necessary, then
> I would prefer to not merge it.
>
> Wayne
>
> On 2/26/2018 5:29 AM, miles mccoo wrote:
> > So I think I may have discovered something that renders this patch
> > irrelevant.
> >
> > I was trying to debug the behavior I mentioned in the first mail of this
> > thread, where undo via BOARD_COMMIT doesn't work right when running code
> > from within an external plugin.
> >
> > Turns out that the external plugin code handles undo, redraw,... for me.
> > Trying to handle undo twice is what caused my issue.
> >
> > The funny thing is that I worked on exposing the BOARD_COMMIT stuff as a
> > prep step to plugin'ifying a bunch of my utilities.
> >
> >
> >
> > Miles
> >
> >
> > On Mon, Feb 26, 2018 at 10:23 AM, miles mccoo <mail@xxxxxxxxxx
> > <mailto:mail@xxxxxxxxxx>> wrote:
> >
> >
> >     On whether it's appropriate to expose BOARD_COMMIT to python, the
> >     class originally came to my attention in this message (though not by
> >     a core kicad developer?):
> >     https://lists.launchpad.net/kicad-developers/msg32275.html
> >     <https://lists.launchpad.net/kicad-developers/msg32275.html>
> >
> >
> >     As a follow on to that suggestion, I started this thread which was
> >     premised on the idea of exposing BOARD_COMMIT.
> >     https://lists.launchpad.net/kicad-developers/msg32063.html
> >     <https://lists.launchpad.net/kicad-developers/msg32063.html>
> >
> >     Most of the discussion in that thread centers around understanding
> >     BC, so perhaps it wasn't noticed that exposing BC to python is the
> >     point.
> >
> >
> >     If it's preferred to omit from python, please advise on a)how to
> >     best get redraw to work? My last patched improved, but didn't
> >     entirely fix redraw. b)undo would be nice to have in external
> >     plugins. How best to achieve this?
> >
> >
> >
> >
> >
> >     On formatting, I looked at it again and I see two errors. In one
> >     function, I'm missing two leading spaces. On another, I have a curly
> >     on the same line as the if. (or perhaps the curly shouldn't be
> there?)
> >
> >     Is there more to it? I'm happy to resubmit but would prefer only one
> >     iteration (assuming the consensus is that BOARD_COMMIT should be
> >     exposed)
> >
> >
> >     Miles
> >
> >
> >
> >
> >     for reference:
> >      #include <macros.h>
> >      #include <stdlib.h>
> >      #include <pcb_draw_panel_gal.h>
> >     +#include <board_commit.h>
> >
> >      static PCB_EDIT_FRAME* s_PcbEditFrame = NULL;
> >
> >     @@ -57,6 +58,10 @@ void ScriptingSetPcbEditFrame( PCB_EDIT_FRAME*
> >     aPcbEditFrame )
> >          s_PcbEditFrame = aPcbEditFrame;
> >      }
> >
> >     +PCB_EDIT_FRAME* GetPcbEditFrame()
> >     +{
> >     +  return s_PcbEditFrame;
> >     +}
> >
> >      BOARD* LoadBoard( wxString& aFileName )
> >      {
> >     @@ -135,3 +140,12 @@ void UpdateUserInterface()
> >          if( s_PcbEditFrame )
> >              s_PcbEditFrame->UpdateUserInterface();
> >      }
> >     +
> >     +
> >     +BOARD_COMMIT*
> >     +NewBoardCommit()
> >     +{
> >     +    if( s_PcbEditFrame ) {
> >     +        return new BOARD_COMMIT( s_PcbEditFrame );
> >     +    }
> >     +}
> >
> >
> >
> >     On Sat, Feb 24, 2018 at 3:02 PM, Wayne Stambaugh
> >     <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>> wrote:
> >
> >         Miles,
> >
> >         I'm not opposite to exposing the BOARD_COMMIT object but there
> >         are some serious ramifications of doing so.  Although we already
> >         expose objects that can crash the board editor from python
> >         scripting so I don't know that one more will be all that
> >         significant.  Can anyone else think of a reason not to expose
> >         the BOARD_COMMIT object to the python scripting? Before I merged
> >         your patch I am going to ask you to reformat the c++ code in the
> >         pcbnew_scripting_helpers.cpp file.  If you are unsure of the
> >         code formating, please take a look at the kicad coding policy[1].
> >
> >         Thanks,
> >
> >         Wayne
> >
> >         [1]:
> >         http://docs.kicad-pcb.org/doxygen/md_Documentation_
> development_coding-style-policy.html
> >         <http://docs.kicad-pcb.org/doxygen/md_Documentation_
> development_coding-style-policy.html>
> >
> >         On 02/24/2018 05:04 AM, miles mccoo wrote:
> >
> >             Back in November I was in a thread about board_commit:
> >             https://lists.launchpad.net/kicad-developers/msg32063.html
> >             <https://lists.launchpad.net/kicad-developers/msg32063.html>
> >
> >             The topic had come up in another patch:
> >             https://lists.launchpad.net/kicad-developers/msg32134.html
> >             <https://lists.launchpad.net/kicad-developers/msg32134.html>
> >
> >
> >             See attached path file. added needed code to SWIG to enable
> >             using BOARD_COMMIT
> >
> >
> >             As a basic test, I ran the attached commit.py [1] on the
> >             attached commit.kicad_pcb. It just moves some modules and
> >             adds some vias. undo seems to work fine and (perhaps more
> >             important) the canvas updates correctly (there are some bugs
> >             in fresh that I haven't figured out)
> >
> >             I have also updated some of my other scripts to use it and
> >             almost all seems well.
> >             *
> >             *
> >             *The thing that doesn't work...*
> >
> >
> >             If I run a script as an external plugin, undoing, brings me
> >             back to an empty canvas. I don't know how to debug that.
> >             Some guidance could be helpful. I don't think this is a
> >             problem in the python interface, but rather some
> >             multi-threading thing in BOARD_COMMIT itself.
> >
> >             Here's a demo of the problem:
> >             https://youtu.be/YR-9dx_lkUo
> >
> >             place_by_sch described in the video is here:
> >             https://github.com/mmccoo/kicad_mmccoo/blob/master/
> place_by_sch/place_by_sch.py
> >             <https://github.com/mmccoo/kicad_mmccoo/blob/master/
> place_by_sch/place_by_sch.py>
> >
> >             The board_commit stuff isn't in that version[2]
> >
> >
> >             Miles
> >
> >
> >             [1] I have a feeling the py file will be filtered, so here
> it is
> >             toplayer = layertable['F.Cu']
> >             bottomlayer = layertable['B.Cu']
> >             for mod in board.GetModules():
> >                  print("mod {}".format(mod.GetReference()))
> >                  newvia = pcbnew.VIA(board)
> >                  board.Add(newvia)
> >                  newvia.SetNet(gnd)
> >                  nc = gnd.GetNetClass()
> >                  newvia.SetWidth(nc.GetViaDiameter())
> >                  newvia.SetPosition(mod.GetPosition())
> >                  newvia.SetLayerPair(toplayer, bottomlayer)
> >                  newvia.SetViaType(pcbnew.VIA_THROUGH)
> >                  bc.Added(newvia)
> >                  bc.Modify(mod)
> >                  mod.SetPosition(pcbnew.wxPoint(mod.GetPosition().x +
> >             pcbnew.Millimeter2iu(10),
> >                                                 mod.GetPosition().y +
> >             pcbnew.Millimeter2iu(10)))
> >
> >
> >             bc.Push("moved stuff")
> >
> >
> >             [2]
> >             diff --git a/place_by_sch/place_by_sch.py
> >             b/place_by_sch/place_by_sch.py
> >             index 22bc21c..5f70354 100644
> >             --- a/place_by_sch/place_by_sch.py
> >             +++ b/place_by_sch/place_by_sch.py
> >             @@ -7,7 +7,9 @@ import re
> >               def PlaceBySch():
> >                   board = pcbnew.GetBoard()
> >             -
> >             +    print("getting board commit")
> >             +    bc = pcbnew.NewBoardCommit()
> >             +
> >                   board_path = board.GetFileName()
> >                   sch_path = board_path.replace(".kicad_pcb", ".sch")
> >             @@ -114,9 +116,10 @@ def PlaceBySch():
> >                       # oldvalue * 25.4 / 10e4
> >                       newx = locs[ref][0] * 25.4 * 1000.0
> >                       newy = locs[ref][1] * 25.4 * 1000.0
> >             +        bc.Modify(mod)
> >                       mod.SetPosition(pcbnew.wxPoint(int(newx),
> int(newy)))
> >                       mod.SetOrientation(locs[ref][2]*10)
> >                       print("placing {} at {},{}".format(ref, newx,
> newy))
> >             -
> >             -    pcbnew.Refresh();
> >             +    bc.Push("place_by_sch")
> >             +    #pcbnew.Refresh();
> >
> >
> >
> >
> >             for my personal reference: this is my sixth patch
> >
> >
> >
> >             _______________________________________________
> >             Mailing list: https://launchpad.net/~kicad-developers
> >             <https://launchpad.net/~kicad-developers>
> >             Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >             <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >             Unsubscribe : https://launchpad.net/~kicad-developers
> >             <https://launchpad.net/~kicad-developers>
> >             More help   : https://help.launchpad.net/ListHelp
> >             <https://help.launchpad.net/ListHelp>
> >
> >
> >         _______________________________________________
> >         Mailing list: https://launchpad.net/~kicad-developers
> >         <https://launchpad.net/~kicad-developers>
> >         Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >         Unsubscribe : https://launchpad.net/~kicad-developers
> >         <https://launchpad.net/~kicad-developers>
> >         More help   : https://help.launchpad.net/ListHelp
> >         <https://help.launchpad.net/ListHelp>
> >
> >
> >
>

Follow ups

References