MitjaNemec / Kicad_action_plugins

Kicad action plugins
414 stars 62 forks source link

Replicate layout: It does not fully handle components on both top and bottom layers. #64

Open dinoghi opened 5 years ago

dinoghi commented 5 years ago

Trying to replicate a layout where the components are placed on both layers does not work correctly:

When the anchor and the pivot components are on a different layers the pivot layer is not changed. In the attached file replicateLayout.zip

whe you select R10 (placed on bottom) to replicate the layout, you see the issue.

MitjaNemec commented 5 years ago

I can't open the schematics with 5.1.4. Can you attach the .log file please?

dinoghi commented 5 years ago

In this new attached zip file there is the full project directory including the .log file. (this time not zipped using kicad "zip" icon but manually compressing the whole folder. replicateLayout_5_1_4.zip

Cheers, Dino.

MitjaNemec commented 5 years ago

The archive includes only an empty folder

dinoghi commented 5 years ago

D'oh! ...doing it again (and checking twice, this time...) replicateLayout_5_1_4.zip

MitjaNemec commented 5 years ago

There is something quite strange with this project. I still can not open it. Looking at .sch files I can see the following hierarchy: the root file replicateLayout.sch is almost empty as it consist of only one hierarchical subsheet file5D7697E6.sch This first level subsheet file5D7697E6.sch also consists of several hierarchical subsheets. The sheets are: ReplicateLayout.sch, ReplicateLayout.sch, ReplicateLayout.sch, file5D7683FA.sch, file5D7683FA.sch Thus you have circular reference, which results in infinite hierarchy (ReplicateLayout.sch->file5D7697E6.sch->ReplicateLayout.sch->...), thus the eeschema crashes, and running the plugin results in "maximum recursion depth exceeded"

How did you manage to create it? How do you see the hierarchy? And all this hierarchy issues are diverting the focus from the main issue.

The way I see it you want to place pivot footprint on let's say top layer and anchor footprint on either top or bottom layer. The plugin should consider this and appropriately mirror the replicated section. Obviously this is currently not supported and as a stop-gap, I'll try to add checks for this to notify the user and cancel the plugin. But in the long run this could be implemented but it'll be probably a while before I start working on it.

dinoghi commented 5 years ago

Quite weird: I can still open it using 5.1.4, 5.1.2 and nightly without even a warning, but looking at the .sch file there is the circular reference so the question is "how can it work on my own machine?". Anyway, Now I deleted the unneeded child sheets and made a simplified schematic/layout to check it.

replicateLayout.zip

Cheers, Dino.

dinoghi commented 5 years ago

...May be I found the reason why on your machine the project does not open correctly: I'm using a case sensitive file system (Linux), so "replicateLayout.sch" and "ReplicateLayout.sch" are two different files when unzipped.. If you are using a case-insensitive file system (like dos/windows usually does) those files become the same and the circular reference is created. If you still have a problem, delete the file "ReplicateLayout.sch" (uppercase "R" from zip file). Cheers, Dino

MitjaNemec commented 5 years ago

Aha, there was a nagging message from the unzip tool, but I thought it was overwriting the fp-info-cache so we could have caught this earlier

MitjaNemec commented 5 years ago

I've added the checks for this condition so that the plugin emits meaningful message and stops the plugin.

dinoghi commented 5 years ago

Hi Mitja, I just got it from git and on the same test project as before I get: On stable versions (tested on kicad 5.1.2 and 5.1.4) it seems to work. Develpment version (master branch): selecting R12 and launching the script I get a "Fatal error when running replicator... replicate_layout.log

From the log it seems that one of the used functions changed in the latest dev versions.

MitjaNemec commented 5 years ago

Thanks for reporting.

The Flip method changed signature. Now it takes an additional argument specifying the direction of the flip (bool aFlipLeftRight). I'll try to handle it

MitjaNemec commented 5 years ago

Fix commited if you could please test it I'd appreciate it.

dinoghi commented 5 years ago

Hi Mitja, On 5.1.2 and 5.1.4 works well, on 5.99 (dev version) the script works but I get a segfault on pcbnew exit and issues on undo operations (but this seems to me more likely a bug in the dev version of kicad that, being a dev version, can be unstable) Cheers, Dino.

dinoghi commented 5 years ago

Re-checed better: There is still a difference between the stable and the dev version of kicad: in dev version the rotation of bottom layer components is not correct using anchor-pivot both on the bottom layer (i.e. bottom layer resistors are rotated by 180 degrees. This does not apply for the reference component). In the stable version it seems it is working, so may be there is some other difference in the functions of the in-development kicad version.

In the attached file the project I used to test it and the two .log files renamed adding the kicad version I used for the test.

replicateLayout.zip

MitjaNemec commented 5 years ago

I've wanted to ask you if you could test exactly this case. Anyhow the plugin should work correctly now

dinoghi commented 5 years ago

Now it works for the simple example posted before, even in the dev version. I tried also in a much more complex layout, but in that case again rotates the bottom components by 180 degrees. I'm still not able to see what is that triggers this behaviour. Also I heard kicad develpoers [https://bugs.launchpad.net/bugs/1843700], they say that during 6.0 development plugins written for the 5.x version may have problems since making the python interface backward compatible could turn into a kind of "nightmare of duplicated code & possible bugs" and also "it isn't known yet what the full API will look like in V6", so may be that to be sure the pluigin works in v6 it is better to wait a while to see how the "real" v6 python interface will look like. (Anyway, if I'll find a way to trigger this problem on a reasonably simple project I'll attach to a comment here, so that it could be used as a test case in the future).

MitjaNemec commented 5 years ago

The source of this is KiCad code commit 37af3adffb0e4dc8643f40f644c982c1c44fe2be, which is connected to bug/wish.

Now from my perspective changing legacy Flip(aCentre) with Flip(aCentre, False) should behave the same. There is a possibility that there is a bug in KiCad code, but I'll have to research this more into detail