BelfrySCAD / BOSL2

The Belfry OpenScad Library, v2.0. An OpenSCAD library of shapes, masks, and manipulators to make working with OpenSCAD easier. BETA
https://github.com/BelfrySCAD/BOSL2/wiki
BSD 2-Clause "Simplified" License
1.01k stars 115 forks source link

[BUG] planetary_gears() Documentation error #1470

Closed dan-p3rry closed 2 months ago

dan-p3rry commented 2 months ago

Describe the bug The description of sun_ring and ring_sun should be swapped as far as which one is >1 and <1. (The documentation sounds correct, but I have to set sun_ring=6 to get ring gear 6x the sun gear.)
Also in this section, not exactly an error, the variables ring_carrier, sun_carrier, etc are inconsistent with the text, where the term "planet" is used.

Thanks a lot for these gear functions. I was really beating my head trying to figure this stuff out. Turns out I was close, but not converging.

adrianVmariano commented 2 months ago

Gears are remarkably complex and subtle. Are you saying that when you set sun_ring=6 you actually get a sun/ring ratio of 1/6? Because that sounds like an implementation error.

dan-p3rry commented 2 months ago

No, I am saying that sun_ring=6 works, but the documentation says: set sun/ring ratio to this value in a sun driven system, must have absolute value smaller than 1

That bold text should say "must have absolute value greater than 1". Setting the value to less than one causes an error in the console.

(I would argue that the arguments sun_ring and ring_sun are named incorrectly (should be swapped), because the sun is always smaller than the ring, but fixing that would break someone's code if they updated BOSL2.)

Dan

On Wed, Sep 11, 2024 at 12:53 AM adrianVmariano @.***> wrote:

Gears are remarkably complex and subtle. Are you saying that when you set sun_ring=6 you actually get a sun/ring ratio of 1/6? Because that sounds like an implementation error.

— Reply to this email directly, view it on GitHub https://github.com/BelfrySCAD/BOSL2/issues/1470#issuecomment-2342353494, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVJPTT2UKW46OCRPD6CJORLZV6A7HAVCNFSM6AAAAABN7DROWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBSGM2TGNBZGQ . You are receiving this because you authored the thread.Message ID: @.***>

dan-p3rry commented 2 months ago

Re-reading your response, yes you are correct that setting sun_ring = 6 actually generates sun/ring ratio of 1/6. Dan

On Wed, Sep 11, 2024 at 11:44 AM Dan Perry @.***> wrote:

No, I am saying that sun_ring=6 works, but the documentation says: set sun/ring ratio to this value in a sun driven system, must have absolute value smaller than 1

That bold text should say "must have absolute value greater than 1". Setting the value to less than one causes an error in the console.

(I would argue that the arguments sun_ring and ring_sun are named incorrectly (should be swapped), because the sun is always smaller than the ring, but fixing that would break someone's code if they updated BOSL2.)

Dan

On Wed, Sep 11, 2024 at 12:53 AM adrianVmariano @.***> wrote:

Gears are remarkably complex and subtle. Are you saying that when you set sun_ring=6 you actually get a sun/ring ratio of 1/6? Because that sounds like an implementation error.

— Reply to this email directly, view it on GitHub https://github.com/BelfrySCAD/BOSL2/issues/1470#issuecomment-2342353494, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVJPTT2UKW46OCRPD6CJORLZV6A7HAVCNFSM6AAAAABN7DROWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBSGM2TGNBZGQ . You are receiving this because you authored the thread.Message ID: @.***>

adrianVmariano commented 2 months ago

That is a bug then and will need to be fixed.

adrianVmariano commented 2 months ago

I investigated more and it appears to me that setting sun_ring=6 produces the specified transmission ratio that the sun gear rotates six times for every single rotation of the ring gear.

dan-p3rry commented 2 months ago

Ok, so we are back to just the documentation error then.

On Wed, Sep 11, 2024, 2:11 PM adrianVmariano @.***> wrote:

I investigated more and it appears to me that setting sun_ring=6 produces the specified transmission ratio that the sun gear rotates six times for every single rotation of the ring gear.

— Reply to this email directly, view it on GitHub https://github.com/BelfrySCAD/BOSL2/issues/1470#issuecomment-2344709509, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVJPTT255FKCWM3Z35OIVITZWCWXHAVCNFSM6AAAAABN7DROWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBUG4YDSNJQHE . You are receiving this because you authored the thread.Message ID: @.***>

adrianVmariano commented 2 months ago

Yeah, I have corrected the docs and also clarified that "ratio" means "transmission ratio".

dan-p3rry commented 2 months ago

Thanks for the fix