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

Create generator for wall mounted (spice) rack #573

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

phillipthelen
Copy link
Contributor

I still want to tweak this a bit and maybe add some more customisation options, but wanted to create this PR to get some feedback on if the implementation looks ok.

This adds a new generator that creates a wall mounted rack/shelf to store p.e. spices. (at least that is why I created it, because I needed something like that in my kitchen).

Features:

  • Customisable height per row
  • different top edges for different mounting options
  • customisable shelf depth, wall height and also back wall height to add more stability if necessary.

Photo of my Prototype (not suitable to be the example picture. I will add better photos once the PR is done:
camphoto_1932422408

@florianfesti
Copy link
Owner

Sorry for the late response. This summer was a bit rough.

This looks actually pretty good already and has a surprisingly rich set of features. Some minor nitpicks:

  • The settings for the available top edges are missing. The lines can be copied from the _template.py - assuming I added the more recent additions like the "y" edge there already...
  • --vertical_edges should probably be renamed and then also used for the edges of the side wall. That way the trays can be made much more robust.

If you don't want to do those yourself after half a year I can handle them. Having a good photo would be very nice though. Just attaching it here is fine with me, though.

@phillipthelen
Copy link
Contributor Author

No problem.

I am still interested in finishing this up.

  • I'm not sure which settings you mean. The ones in these lines? The finger joint settings I added, but I didn't feel like the others would be applicable? Or what do you think?
  • Looking at the code after 6 months, I'm not entirely sure what the --vertical_edges setting even does, since it doesn't seem to be used in the code 😅 Iiirc right now the connections for the side walls to the back plate are always with finger holes in the back plate. Your suggestion would be to rename the setting and use it for that connection toggle between holes and regular joints, right?

Thanks for taking the time to look through my PR

@florianfesti
Copy link
Owner

After starting to write an answer I figured it would be quicker to just make the changes myself.

The vertical_edges setting ususally turns the F edges at the corners of a box into h edges (finger holes) for more stability. It's not necessary to add this feature before merging this PR. This can still be done later on. Either by you or by myself or not at all. It's not too difficult but touches a few places and needs special care at the bottom to not mess things up - especially when flat_bottom is selected.

From my POV this is ready to be merged. If you don't mind I would just squash the patches and merge.

@florianfesti
Copy link
Owner

Ok, added the parameter back. Set the default to finger holes as this is much stronger. It also will make the bottom flat by default without any drawbacks.

flat_bottom and F side_edges now puts the holes of the bottom row right at the edge - basically emulating an F edge.

@florianfesti florianfesti marked this pull request as ready for review February 9, 2024 17:38
@florianfesti florianfesti merged commit 36f6803 into florianfesti:master Feb 9, 2024
7 checks passed
@florianfesti
Copy link
Owner

OK, merge after a bit of tweaking of the sizes when using outside.

If you can submit a nice sample picture either as an PR or attaching here or in #140 that would be very much appreciated.

Other than that: Thanks for your patience and your work!

@florianfesti florianfesti mentioned this pull request Feb 9, 2024
@florianfesti
Copy link
Owner

Picture got added with 8411b7f

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