INTI-CMNB / KiAuto

A bunch of scripts to automate KiCad processes
Apache License 2.0
71 stars 19 forks source link

Issue with zooming on 3DRender #13

Closed mdeweerd closed 2 years ago

mdeweerd commented 2 years ago

Continued from #10 .

I am not sure that you got my mail on your '*br' address regarding my zoom issue.

Adding a small sleep before starting the zoom in the loop (not just before the loop) circumvents the issue in the docker container:

diff --git a/src/pcbnew_do b/src/pcbnew_do
index d1d7e7d..ff44756 100755
--- a/src/pcbnew_do
+++ b/src/pcbnew_do
@@ -465,6 +465,7 @@ def capture_3d_view(cfg):
             zoom = -zoom
             zoom_b = '5'
         for _ in range(zoom):
+            sleep(0.05)
             xdotool(['click', zoom_b])
             wait_3d_ready()

What can I do to help identify this issue?

set-soft commented 2 years ago

Is strange. I verified that moving wait_3d_ready outside the loop doesn't work on my system. I tried with a heavy board and it works for small zoom values, like 5, but with 7 fails. What about moving this sleep to the last line of wait_3d_ready. Perhaps we must wait some time after the 3D viewer is focused before sending the click. I added such a delay, let me know if it helps.

mdeweerd commented 2 years ago

It doesn't work with the code update.

For info, initially I added the delay after the zoom action in the loop, but that did not help. This worked only when adding it (also) before the zoom action and I confirmed that there was no need to add an extra delay after the zoom action.

It also fails with the original code on a board with few components but in both cases I have the housings as virtual 3D models, but in both cases the small delay resolves it.

set-soft commented 2 years ago

It doesn't work with the code update.

Ok, another mystery, but a delay there doesn't harm, I added it. Let me know if it solves your problems.

mdeweerd commented 2 years ago

I autoupdate the docker container (and it updated), but it did not work - I checked pcbnew_do and it did not have the delay.

duo_ph_rx
dev: Pulling from setsoft/kicad_auto
Digest: sha256:1e890a397ead1954ae6b07ebec6b87cc9f0de1724b25fdf861dfd8b9db66a530
Status: Image is up to date for setsoft/kicad_auto:dev

Then in the container:

root@8e7213045b1a:~/workdir# ls -lrt /usr/bin/pcbnew_do
-rwxr-xr-x 1 root root 38684 Nov 30 15:39 /usr/bin/pcbnew_do
root@8e7213045b1a:~/workdir# md5sum /usr/bin/pcbnew_do
94a835744321219962fb483c760a95b0  /usr/bin/pcbnew_do

I updated my clone of KiAuto, and used that instead using the volume mapping.

I trusted the image, but may be I shouldn't.
Maybe you should throw a file with the git hash in the container somewhere to check that everything is as expected, and even an md5 of all files, with a small script to check for differences.

Here's a script that I just made to help. When called the first time, it will create the reference file (useful in when creating the docker image), when called a next time, it will compare MD5SUMS. It shows the 'md5' of the checksum files so that we can also verify that the right checksum file is used.

#!/bin/bash
REF_FILE=/var/local/kibot_md5.txt
OUT_FILE=/tmp/kibot_md5_tmp.txt
[ -r ${REF_FILE} ] || OUT_FILE=${REF_FILE}

(
  find /usr/local/lib/python*/dist-packages/kibot -type f -exec md5sum {} \; ;
  md5sum /usr/bin/pcbnew_do /usr/bin/eeschema_do /usr/local/bin/ki*
) \
| sort -u -k 2,2 \
> ${OUT_FILE}

md5sum ${REF_FILE}
md5sum ${OUT_FILE}

diff -c0 ${REF_FILE} ${OUT_FILE}

Not - I just got an update of the image - I'll check and notify a new message.

mdeweerd commented 2 years ago

In the new container image, pcbnew_do is not the one I got from the git repository. The latter works.

I got:

dev: Pulling from setsoft/kicad_auto
Digest: sha256:29fd4875072973d4706e3359fdb160f6a7fcb892ca5aaaf2df4fabb40ab42835
Status: Image is up to date for setsoft/kicad_auto:dev
docker.io/setsoft/kicad_auto:dev
set-soft commented 2 years ago

The docker.io/setsoft/kicad_auto:dev is development for KiBot, not KiAuto. To test KiAuto you should install the code from git.

mdeweerd commented 2 years ago

Ok, noted. Then it's logical and it's ok from the git clone.

set-soft commented 2 years ago

Ok, then I'm closing it.