Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix some bug, remove temp file and add metric measures #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gianninou
Copy link

  • Adding the metric measures in millimeters
  • Some fixes regarding Inkscape were added
  • Removing temp files
  • Adding a name with the key spec in the final name

@choller choller self-requested a review January 14, 2025 12:42
Copy link
Owner

@choller choller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your changes. As mentioned in the review, I'm in the process of merging other changes to this repository as well and it might take me a while to resolve everything.

The reason is that I launched this as an online tool now at https://autokey.own-hero.net/ that doesn't require any dependencies and runs in the browser.

subprocess.check_call([
"potrace",
"tmp.pbm",
"--tight", "-s",
"-o", "tmp.svg"
])
subprocess.check_call(["inkscape", "-h", str(img.shape[0]), "-b", "white", "-e", "tmp.png", "tmp.svg"])
subprocess.check_call(["inkscape", "-h", str(img.shape[0]), "-b", "white", "--export-type", "png", "--export-filename", "tmp.png", "tmp.svg"])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently newer inkscape uses --export-filename while the older (Ubuntu LTS) does not support that. We likely need some logic to determine which switches to use. I can implement that later. That applies for all the inkscape invocations.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using inkscape from Kali, so maybe another version (Inkscape v1.2.2 today), but the web version fix that point :)

@@ -241,7 +242,7 @@ def empty():
if mw <= 0:
mw = 0

svg = cv2.copyMakeBorder(svg, mh/2, mh/2 + mh % 2, mw/2, mw/2 + mw % 2, cv2.BORDER_CONSTANT, value=(255, 255, 255, 255))
svg = cv2.copyMakeBorder(svg, int(mh/2), int(mh/2 + mh % 2), int(mw/2), int(mw/2 + mw % 2), cv2.BORDER_CONSTANT, value=(255, 255, 255, 255))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this cause any errors previously? would be good to know :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes, but to be honest I did some update some months ago and maybe I had an error.
If you are waiting for int only (not float), you can keep it in case, or you can remove it and see if it's working for you.

@@ -261,6 +262,15 @@ def empty():


cv2.destroyAllWindows()
# Remove tmp file, that should be moved in another location...
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the tempfiles actually helpful in case something crashed. But maybe we can put this behind a flag. I will think about it :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, an option should be great, or even a folder, you can choose what do you want ;)

@@ -94,7 +94,7 @@ union() {
keytipcuts();

if (!blank) {
keycombcuts(lasercut);
keycombcuts(lasercut, realdim);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume your goal here was to allow the combination to be specified in mm rather than code? I would recommend doing this by specifying a system that has a cutspace resolution that is a 0.1mm (there is nothing under that precision happening usually with keys) and then just multiplying your values by 10 and going off the highest possible cut.

I will think about adding this, however I am currently refactoring the combination system anyway, using 1 as the shallowest cut like most key programs do, so the codes stay comparable. It could take a while for me to merge things.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what profile is the svg (i.e. what lock/brand did it belong to?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That point was the main point for my PR in fact.
My goal was to create a key from a picture with the help of the KeyDecoder tool : https://play.google.com/store/apps/details?id=com.keydecoder

So, I would like to have the possibility to enter the measure from the botom of the key in mm.
So yes 0.1mm should be enougth.

So if you are refactoring all the tool, if you can think regarding this issue and adding a profile named "mm" or something like that it could be good.

As I'm not a key expert, I'm not able to say "this key with this profile possess these definitions", I'm juste able to get the tooth length in mm, the space between the shoulder and the first pin, and le length of the key. From that, I Would be able to create the same key.

Regarding the SVG, I don't know which profile is it. In the same way, I would like to create the profile from a picture and the key from a picture (with the measure from the KeyDecoder application) without knowing the brand, type, ... so from a "key noob" point of view ^^

I hope it was clear :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants