MaslowCNC / Firmware

Maslow Firmware
GNU General Public License v3.0
261 stars 136 forks source link

Simplified Chain Tolerance Correction #471

Closed johnboiles closed 5 years ago

johnboiles commented 5 years ago

This is another bit of code I found in @madgrizzle's development branch. I believe this to be a solid simplification of how the chain tolerance correction works.

In the current system, GroundControl accepts a positive or negative percentage adjustment to adjust the chain tolerance for each chain (e.g. 1.3% if your chain is 1.3% longer than expected), it then turns that into a calculated value for distPerRotLeftChainTolerance by multiplying the adjustment by distPerRot. That value is then sent to the firmware, where it is divided by 2*Pi and stored as RleftChainTolerance. It's then used in place of the sprocket radius R in the firmware for all calculations that involve the sprocket's radius.

With this change, GroundControl continues to accept a positive or negative percentage adjustment for each chain (current settings will continue to work), but does no calculation on that value. Then that adjustment is sent to the firmware as leftChainTolerance. That tolerance value is simply multiplied by the calculated chain length to derive the adjusted chain length.

Additionally, I think this is maybe slightly more accurate. The current chain tolerance adjustment adjusts for the bit of chain that is still on the sprocket (Chain1AroundSprocket). I think the length of the chain around the sprocket is a function of the radius of the sprocket, and not the tolerance in the chain, though it's likely not a big enough value to matter one way or another. It's trivial to make this change account for that as well.

The corresponding GroundControl change is MaslowCNC/GroundControl#783 Discussion in the forums is here: https://forums.maslowcnc.com/t/simplified-chain-tolerance-correction-pull-request/7395

MaslowCommunityGardenRobot commented 5 years ago

Congratulations on the pull request @johnboiles

Now we need to decide as a community if we want to integrate these changes. Vote by giving this comment a thumbs up or a thumbs down. Votes are counted in 48 hours. Ties will not be merged.

I'm just a robot, but I love to see people contributing so I'm going vote thumbs up!

BarbourSmith commented 5 years ago

Great! I'm about to run a longer cut and give this a solid test. Excellent picking!

johnboiles commented 5 years ago

I'm glad you were able to take a look and I'm glad you'll be able to test! Thanks @BarbourSmith

BarbourSmith commented 5 years ago

I am two hours into a test with no issues so far. Gets my 👍 for sure. Great work!

We should keep in mind that this change requires updating Ground Control to work properly so I bet we will see a couple questions in the forums triggered by that.

johnboiles commented 5 years ago

Great! What are you cutting?

BarbourSmith commented 5 years ago

I'm making a skateboard rack for a friend:

image

johnboiles commented 5 years ago

Neat! Looking good. Thanks for sharing

MaslowCommunityGardenRobot commented 5 years ago

Time is up and we're ready to merge this pull request. Great work!