50an6xy06r6n / hotswap_pcb_generator

OpenSCAD script for generating 3D-printable hotswap keyboard PCBs.
GNU General Public License v3.0
432 stars 34 forks source link

Previous commit completely broke switch/pcb generation #21

Closed BlueDrink9 closed 9 months ago

BlueDrink9 commented 9 months ago

Rendering default layout switch.scad and pcb.scad with 7f01171 creates something completely non-functional. I'm on openscad 2021.01 btw, in case it looks fine on your end

I presume from the commit message that this was an experimental change, but I spent ages trying to debug thinking it was some lingering change in my config. Would you consider doing experimental changes in a development branch, and leaving main more stable?

50an6xy06r6n commented 9 months ago

Ah sorry about that. I changed some parameters to test something and forgot to change them back. If you change pcb_thickness to 4 and pcb_type to printed I think that should fix the issue.

In general I think this library could use some better pattern for managing parameters like that on a per-project basis, but I'll try to be more careful about committing changes to parameters.scad.

In terms of targeting changes on a different branch, I'm not sure this repo has enough activity to have a fully separate develop branch, as it is useful to have people report regressions caused by new features. I did go ahead and create a release for the previous stable version, so if you run into issues down the road you can try to revert to that.

I'll have a fix up for this shortly.

50an6xy06r6n commented 9 months ago

@BlueDrink9 I just put up a new commit that I think should fix your issue (along with a bunch of other things I found). Let me know if you're still having issues.

After thinking about it some more, I will try to start putting incremental changes in a develop branch, and bring those into main after I've had time to test it a bit and make sure I didn't break basic model generation.

BlueDrink9 commented 9 months ago

Thanks! I'll PR my enhancements hopefully this week or so. I'll look at what could be shifted out of params as part of that, if you'd like?

Already built one very custom board with this, but it required a big bunch of tweaks in the main code so won't be adding it as an example. Starting on another board shortly, and I wanted to try address some of the issues I had with the switch connection

50an6xy06r6n commented 9 months ago

I already have some thoughts on how to improve the modularity, so don't worry about putting that in your changes, since I'll probably override them 😛 But if you want to discuss via issues I'd be happy to engage.

I welcome any PRs you have for enhancements, but fair warning that I don't have much of a process figured out for how I incorporate external changes. I am curious what issues you had with the switch connections and what your solution was.

BTW would love to see pictures of the board you built! This project has been kind of sitting around for a couple years and I could tell people were using it, but I haven't seen much of what folks have actually made.

50an6xy06r6n commented 9 months ago

Actually if you want to discuss, let's do it in https://github.com/50an6xy06r6n/hotswap_pcb_generator/issues/13, since I apparently realized this was an issue 2.5 years ago

BlueDrink9 commented 9 months ago

Yeah definitely have plans to post pics and tag you, I'm well aware that it feels good to see what people use your work for! Recent build though and I have some other stuff to get out first before I do the write-up though

BlueDrink9 commented 9 months ago

@BlueDrink9 I just put up a new commit that I think should fix your issue (along with a bunch of other things I found). Let me know if you're still having issues.

After thinking about it some more, I will try to start putting incremental changes in a develop branch, and bring those into main after I've had time to test it a bit and make sure I didn't break basic model generation.

Getting a new error now:

[WARNING: Ignoring unknown variable 'stab_2u' in file param_processing.scad, line 27]
[TRACE: called by 'set_defaults' in file param_processing.scad, line 27]

Fixed by adding include <stabilizer_spacing.scad> to default_layout.scad

50an6xy06r6n commented 9 months ago

Ah looks like I forgot to include the stabilizer presets file somewhere

On Sat, Jan 6, 2024, 7:21 PM Silico_Biomancer @.***> wrote:

@BlueDrink9 https://github.com/BlueDrink9 I just put up a new commit that I think should fix your issue (along with a bunch of other things I found). Let me know if you're still having issues.

After thinking about it some more, I will try to start putting incremental changes in a develop branch, and bring those into main after I've had time to test it a bit and make sure I didn't break basic model generation.

Getting a new error now:

[WARNING: Ignoring unknown variable 'stab_2u' in file param_processing.scad, line 27] [TRACE: called by 'set_defaults' in file param_processing.scad, line 27]

— Reply to this email directly, view it on GitHub https://github.com/50an6xy06r6n/hotswap_pcb_generator/issues/21#issuecomment-1879924942, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARM6VBSOXBHZT3OHWWSXNTYNIIEXAVCNFSM6AAAAABBPMQT5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZZHEZDIOJUGI . You are receiving this because you commented.Message ID: @.***>

BlueDrink9 commented 9 months ago

Fixed by adding include to default_layout.scad

50an6xy06r6n commented 9 months ago

Ok pushed another change to include the presets in the appropriate places

santiagoluz commented 9 months ago

I decided to build a custom keyboard and I was excited that I found this project. Then I started using it forthe first time and it wasn't generating the PCB the same way as described in the build buide. I thought I was doing something wrong. I came back to this yesterday and I realized there was an update to the repo. Thanks a lot for both of you, one for reporting it and the author for fixing (and building) it. :)

BlueDrink9 commented 9 months ago

@50an6xy06r6n is it intentional that the current default layout defines an empty layout? Currently if you open any of the files like pcb.scad or layout.scad, nothing is rendered.

50an6xy06r6n commented 9 months ago

Yeah the default layout is now mostly there to define variables that might not be used so stuff doesn't break. Probably should change the name at some point