MitjaNemec / Kicad_action_plugins

Kicad action plugins
414 stars 62 forks source link

Replicate layout rotates all components and does not layout tracks. #2

Closed sww1235 closed 6 years ago

sww1235 commented 6 years ago

Hello:

I am grateful that you took on the challenge of updating mmccoo's replicate layout script and implemented it as a proper module. Your version at least partially works 👍 .

I attempted to use the master branch as of f763c1ac7c3588657a535bd5bdce89d99291dc90 and had the following behavior with version 2017-09-29-revision b6d54ac for Mac OS X.

I installed the plugin where it seems things work for now on mac, which is /Applications/KiCad/Kicad.app/Contents/SharedSupport/scripting/plugins with the replicatelayout directory inside plugins.

It showed up in the plugins menu just fine and the dialog box appeared when I ran it. I entered a y offset but no x offset and selected linear offset.

When I clicked ok, I got the traceback inserted at the end in another dialog box, and when I clicked ok on it, the footprints were moved to their proper place but no tracks were replicated and all the replicated footprints had been rotated 45 degrees.

I also ran the replicatelayout.py file separately without moving it with the following result:

Ω:python3 replicatelayout.py all
  File "replicatelayout.py", line 666
    print "passed all tests"
                           ^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(print "passed all tests")?

It almost seems like part of the linear replicator is getting called, and part of the polar replicator is getting called.

From looking back through the Kicad launchpad git history, it seems like this commit broke the script, which was probably released hours after they compiled the windows daily release for 2017-9-19.

Traceback (most recent call last):

  File "/Applications/Kicad/kicad.app/Contents/SharedSupport/scripting/plugins/replicate_layout/action_replicate_layout.py", line 232, in Run
    polar)

  File "/Applications/Kicad/kicad.app/Contents/SharedSupport/scripting/plugins/replicate_layout/replicatelayout.py", line 595, in replicate_layout
    self.replicate_tracks(x_offset, y_offset, polar)

  File "/Applications/Kicad/kicad.app/Contents/SharedSupport/scripting/plugins/replicate_layout/replicatelayout.py", line 443, in replicate_tracks
    net_pairs, net_dict = self.get_net_pairs(sheet)

  File "/Applications/Kicad/kicad.app/Contents/SharedSupport/scripting/plugins/replicate_layout/replicatelayout.py", line 277, in get_net_pairs
    pad_name = p_pad.GetPadName()

  File "/Applications/Kicad/kicad.app/Contents/Frameworks/python/site-packages/pcbnew.py", line 9338, in <lambda>
    __getattr__ = lambda self, name: _swig_getattr(self, D_PAD, name)

  File "/Applications/Kicad/kicad.app/Contents/Frameworks/python/site-packages/pcbnew.py", line 83, in _swig_getattr
    raise AttributeError("'%s' object has no attribute '%s'" % (class_type.__name__, name))

AttributeError: 'D_PAD' object has no attribute 'GetPadName'
sww1235 commented 6 years ago

3 fixes the Traceback issue for me and allows tracks to be routed properly, but does not fix the issue of footprints being rotated 45 degrees. The below images were created using the patched version in #3

Footprint layout before running script:

screen shot 2018-01-31 at 22 32 28

Settings used:

screen shot 2018-01-31 at 22 33 20

Footprint layout after running script:

screen shot 2018-01-31 at 22 33 36

All the board files used are at https://github.com/sww1235/edc-mux-board

MitjaNemec commented 6 years ago

Hi,

thanks for the feedback. First a little context. I've needed the replication script for linear replication and this part was tested quite thoroughly. As I had a bit of time extra I added circular (polar) replication, but I haven't tested it on a real project. Also linear replication was not tested properly once circular replication was added.

The stand-alone run of the "replicatelayout.py" is intended exactly for the purposes of testing itself as the pcbnew API changes quite often. Which is what you found out and corrected.

The stand-alone run assumes python 2.7 more specifically python packaged along Kicad. This is the case why you get SyntaxError: Missing parentheses in call to 'print'. Did you mean print(print "passed all tests")? error.

As for the linear replication also running circular replication I will need a day or two to set up a VM environment where I can different version of KiCad as I would like to keep 2017-09-19 on my development machine. As I don't have access to 2017-09-29 I plan to test it with current nightly (probably 2018-01-30

MitjaNemec commented 6 years ago

I've just tested it against 2018-01-14 and it works fine. Also I don't have any issues with linear replication rotating components. The issue with GetPadName and GetName was resolved in cb3d28f11 on 2017-10-09. So I will have to consider whether I accept or reject your pull request.

In the mean time I think I found a bug so I will prepare a test branch that I would really appreciate if you test it.

MitjaNemec commented 6 years ago

Can you try the branch Bugfix_circular_in_y_linear? You will have to apply your patch also

sww1235 commented 6 years ago

Hello:

I tried the Bugfix_circular_in_y_linear branch and it fixed my issue on 2017-9-29. I will try it on current nightly as well, as there have been several nice macOS fixes released recently that I want to try.

It doesn't matter to me if you apply my patch or not. I was just trying to be helpful.

I will report back if the patch works on the current nightly for macOS

sww1235 commented 6 years ago

Tested the Bugfix_circular_in_y_linear branch on macOS nightly 2018-02-01 and everything worked, except the radio buttons at the top to select linear or circular were overlapping:

screen shot 2018-02-01 at 09 55 11

If I did not click on the radio buttons, but instead assumed that linear was selected and entered my y offset, everything worked fine.

MitjaNemec commented 6 years ago

Thanks for testing.

I merged your patch to the branch "Bugfix_circular_in_y_linear" and committed additional GUI bugfix, but I can not really test it as even the previous GUI code and the corrected one worked fine on Windows platform. Please send me feedback as it is highly appreciated.

I know that wxWidgets behave differently across platforms (even though they should not) and most issues are on macOs, so I might not be able to fix this

sww1235 commented 6 years ago

I just tested the GUI code and I am still experiencing the same issue. Everything else still works though, as long as you don't click the polar radio button.

screen shot 2018-02-02 at 10 26 19

MitjaNemec commented 6 years ago

Closing this issue, as I've opened a new one for the GUI issue