-
Notifications
You must be signed in to change notification settings - Fork 21
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
base: master
Are you sure you want to change the base?
Conversation
gianninou
commented
Jan 14, 2025
- 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
There was a problem hiding this 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"]) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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... |
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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 :)