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

Eco bin label #29

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

Eco bin label #29

wants to merge 1 commit into from

Conversation

Ruudjhuu
Copy link

An attempt to add LabelShelf to eco bin. In my opinion some Propperties should be tied to "make" functions as they have a hard dependency. I tried to do this for the LabelShelf by using a class as container. Let me know what you think about this approach. I am happy to apply this to more "features" so reusing these in different kind of bins would be easier.

If not, I'll create a new PR to do this in the current style.

This will partially fix #23

Example of LabelShelf for the EcoBin:
image

@Stu142
Copy link
Owner

Stu142 commented Dec 17, 2024

Would you be able to explain the concept differences between what I did and what you have done here. I don't have any formal programming training and am a bit lost by some of the terminology you are using here.

@Ruudjhuu
Copy link
Author

I'll Do my best.

Ideal Situation

From the perspective of a bin (EcoBin,PartsBin,SimpelStorageBin) the ideal situation is that the bin performs one call to the feature. For example

class BinType:
    def __init__(self):
        SomeFeature.apply()
        SomeOtherFeature.apply()

With this construction, if you want to extend another bin type with SomeFeature. The only thing that needs to be done is adding this call to the new bin type.
The bin does not have any knowledge of the internals of the feature no knowledge about its properties, and the way the feature is constructed. A bug in SomeFeature can be fixed by only touching the internal code of the SomeFeature class and automatically works for all bin types.

Current Situation

The current situation in the master branch has "feature knowledge" in the code of the Bin classes. For example the property LabelShelfPlacement is defined in PartsBin.add_bin_properties, SimpelStorageBin.add_bin_properties and when labels are added to EcoBin also in EcoBin.add_bin_properties. This property is only used by MakeLabelShelf defined in another file.

#features.py
class BinType:
    def __init__(self):
        #creation of property
        obj.addProperty("LabelShelfPlacement")

#feature_construction.py
def MakeLabelShelf(obj):
    # usage of property, and thus hard dependency
    obj.addProperty("LabelShelfPlacement")

When LabelShelfPlacement is changed, code changes to 3 bins and MakeLabelShelf are needed which are 2 files for 1 small piece of behaviour. There is a risk one of these places are forgotten and the functionality of MakeLabelShelf works for 2 bins but is broken for another bin. This can be solved by some object owning MakeLabelShelf and has the responsibility of creating the property LabelShelfPlacement it depends on. which will look like:

class LabelShelf():
  def add_properties(obj):
    obj.addProperty("LabelShelfPlacement")

  def MakeLabelShelf(obj):
    obj.addProperty("LabelShelfPlacement")
        # Construct the labelshelf

What I created

Due to how FreeCad works, the property generation must happen in the class initialization phase. The construction of the object must happen in another function. In the Bin.__init__ function a "feature" class is constructed and added to a list. When the 3d shape is created, the bin loops over the feature list and creates them callinig "Make()". The Bin classes have just one mention of LabelShelf with the assumption that a feature object implements the Make() function (this is where the abstractmethod comes in). If this idea is implemented for all features it will look something like this:

class SomeBin():
    def __init__(self,obj):
        self.features = [ LabelShelf(obj),
                      Scoop(obj),
                      MagnetHoles(obj),
                      ScrewHoles(obj),
                      SomeRandomOtherFeature(obj) ]

    def generate_gridfinity_shape(obj):
        # Some bin specific 3d shape creation

        # Add generic features
        for feature in self.features:
            feature_obj = feature.Make(obj)
            if feature_obj:
                total_obj = total_obj.Fuse(feature_obj)

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.

Feature Request: Label Holder (Shelf) and Scoop for EcoBin
2 participants