Skip to content

Add scoop functionality to EcoBin classes#133

Merged
Stu142 merged 3 commits into
Stu142:masterfrom
graelo:feat/ecobin-scoop
Mar 3, 2026
Merged

Add scoop functionality to EcoBin classes#133
Stu142 merged 3 commits into
Stu142:masterfrom
graelo:feat/ecobin-scoop

Conversation

@graelo

@graelo graelo commented Sep 28, 2025

Copy link
Copy Markdown
Contributor

Hi, many thanks for this Addon, I love it. I was just missing the scoop for EcoBins.

This PR extends scoop support to both EcoBin and CustomEcoBin classes, bringing feature parity with standard storage bins.

Changes

  • EcoBin: Added scoop properties and shape generation logic
  • CustomEcoBin: Added scoop properties with proper custom shape integration (scoop is trimmed by inside_wall_negative

Both classes now expose Scoop (boolean) and ScoopRadius (length) properties in the UI. Default scoop value set to False for backward compatibility

Implementation

The implementation reuses the existing make_scoop() function and follows established patterns used by other features like label shelves. For CustomEcoBin, the scoop is properly cut by the inside wall negative to handle custom shapes correctly.

Testing

  • Code passes linting (ruff) and type checking (mypy)
  • I could not identify any breaking changes to existing functionality
  • Maintains backward compatibility with existing EcoBin objects

This addresses the feature gap where scoops were available in standard bins but missing from eco bins.

Here's an EcoBin with a scoop (radius: 12mm)

image

The underside is unaffected:
image

It also works for 1x1 EcoBins
image

and custom EcoBins
image

@greg19 greg19 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, cool to have it, thanks for contributing 🙂

Comment thread freecad/gridfinity_workbench/features.py Outdated
@graelo

graelo commented Sep 28, 2025

Copy link
Copy Markdown
Contributor Author

Question for you @greg19:

I'm getting this FA100 warning from Ruff, and I'm wondering what's the most appropriate way for this repo to fix it:

freecad/gridfinity_workbench/feature_construction.py:196:58: FA100 Add `from __future__ import annotations` to simplify `typing.Optional`
    |
196 | def make_scoop(obj: fc.DocumentObject, *, usable_height: Optional[int] = None) -> Part.Shape:
    |                                                          ^^^^^^^^ FA100

I think we have to keep it work for Python 3.10 because I believe that's what FreeCAD still uses, so I went for Optional for the type hint. Would you prefer me to apply the advice for FA100?

Sorry, FreeCAD uses 3.11, I can go with the new notation.

@graelo

graelo commented Sep 28, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback, it's indeed better now.

@graelo graelo requested a review from greg19 September 28, 2025 09:34
Comment thread freecad/gridfinity_workbench/feature_construction.py Outdated
@graelo graelo requested a review from greg19 September 28, 2025 15:08
@graelo

graelo commented Oct 10, 2025

Copy link
Copy Markdown
Contributor Author

Hello @Stu142, a kind ping for this PR.
In any case, take care.

@Stu142

Stu142 commented Oct 10, 2025

Copy link
Copy Markdown
Owner

@graelo thanks for the contribution I'll have time to take a look at it this weekend, I've been unable to find time over the summer but things are cooling down now and I have some catching up to do.

@greg19

greg19 commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

@graelo can you resolve the conflicts?

@graelo

graelo commented Jan 22, 2026

Copy link
Copy Markdown
Contributor Author

Hi, sure I’ll do this within a day.

@graelo graelo force-pushed the feat/ecobin-scoop branch from 1bc9a7f to eadd3ec Compare January 22, 2026 18:56
@graelo

graelo commented Jan 22, 2026

Copy link
Copy Markdown
Contributor Author

Hi @greg19, I rebased on the latest master and fixed the conflicts. Cheers

@greg19 greg19 mentioned this pull request Feb 9, 2026
@graelo

graelo commented Feb 21, 2026

Copy link
Copy Markdown
Contributor Author

I'll rebase soon.

@graelo

graelo commented Feb 21, 2026

Copy link
Copy Markdown
Contributor Author

Note: when running ruff check locally, I'm still seeing issues that are probably not linked to my PR.

@Stu142

Stu142 commented Feb 28, 2026

Copy link
Copy Markdown
Owner

@graelo I have the same problems with the ruff check in the import block as well always have since this was implemented by a contributor and was unable to find a fix. I just had to leave it as is and manually make fixes.

@graelo

graelo commented Feb 28, 2026

Copy link
Copy Markdown
Contributor Author

Hi @Stu142, thanks! I was anyway rebasing following the previous PR merge. Here's the updated version. Ruff and Mypy checks passed locally. We'll see how it goes!

@graelo

graelo commented Feb 28, 2026

Copy link
Copy Markdown
Contributor Author

I found that I had been having issues with Ruff import sorting locally due to the fact that macOS filesystem is case insensitive. On macOS (case-insensitive filesystem), ruff classifies FreeCAD as first-party because it matches the project's freecad/ directory. This causes it to expect a blank line between import Part (third-party) and import FreeCAD (first-party), and to sort Part before FreeCAD. On Linux CI (case-sensitive), both are third-party, so ruff expects them in the same block, alphabetically sorted — and flags the macOS ordering as I001.

The fix adds known-third-party = ["FreeCAD", "FreeCADGui", "Part"] to the isort config so classification is consistent across platforms.

I squashed the commits. Now I need to test this still works. I'll report soon here.

@graelo

graelo commented Feb 28, 2026

Copy link
Copy Markdown
Contributor Author

Yup, it still works! You can see the thin stacking lip from Glenn in the last hour and the scoop applied to an ecobin.

image

and still no artifacts below, it seems correct from all sides

image

and it seems to work well with the think stacking lip too

image

graelo added 2 commits March 1, 2026 00:52
Add [tool.ruff.lint.isort] known-third-party config so FreeCAD, FreeCADGui,
and Part are classified consistently on macOS and Linux. Revert import
reordering to alphabetical within a single block.

Remove float() wrappers from usable_height in make_scoop() which stripped
FreeCAD Quantity units, causing "Unit mismatch in plus operation" at runtime.
Use targeted type: ignore[arg-type] comments for the freecad-stubs false
positives instead.
@graelo graelo force-pushed the feat/ecobin-scoop branch from e81acb5 to 5c883b9 Compare March 3, 2026 22:02
@graelo

graelo commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Hi, I simply rebased on the latest master. Thanks in advance!

@Stu142

Stu142 commented Mar 3, 2026

Copy link
Copy Markdown
Owner

Sorry for the extended delay to merge it seems to work great.

I found that I had been having issues with Ruff import sorting locally due to the fact that macOS filesystem is case insensitive. On macOS (case-insensitive filesystem), ruff classifies FreeCAD as first-party because it matches the project's freecad/ directory. This causes it to expect a blank line between import Part (third-party) and import FreeCAD (first-party), and to sort Part before FreeCAD. On Linux CI (case-sensitive), both are third-party, so ruff expects them in the same block, alphabetically sorted — and flags the macOS ordering as I001.

Interesting I am on windows but I think others were linux so there may be something similar going on I'll have to look into it.

@Stu142 Stu142 merged commit a6f73e8 into Stu142:master Mar 3, 2026
4 checks passed
@graelo

graelo commented Mar 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks!

@graelo graelo deleted the feat/ecobin-scoop branch March 4, 2026 16:57
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.

3 participants