kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

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>
> 
> 
>