search

MaslowCNC / Firmware

Maslow Firmware
GNU General Public License v3.0
263 stars 134 forks source link

Change operation associated with ChainTolerance #479

Closed madgrizzle closed 5 years ago

madgrizzle commented 5 years ago

This was discovered by @c0depr1sm and @joshua in this thread: https://forums.maslowcnc.com/t/holey-triangular-calibration/6240/191. The original implementation of ChainTolerance modified the sprocket's mmPerRotation (increasing it for a positive ChainTolerance value). The effect was to reduce the amount of chain fed out. This was adjusted in a previous PR where the ChainTolerance only modified the ChainStraight length in triangularInverse (because there shouldn't be any chain stretch associated with the wrap around the sprocket). However, the formula used actually increases the amount of chain that is fed out (i.e., multiplying by a factor of 1+ChainTolerance) instead of decreasing it. So, this PR changes the operator such that the ChainTolerance is divided into the ChainStraight rather than multiplies it resulting in a smaller chain length amount being fed out.

MaslowCommunityGardenRobot commented 5 years ago

Congratulations on the pull request @madgrizzle

Now we need to decide as a community if we want to integrate these changes. You should 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 (but my vote won't count...)!

BarbourSmith commented 5 years ago

Excellent catch! 👍 👍 ❤️ ❤️

It was my experience that messing with the chain tolerance generally decreased my accuracy, I wonder if this could be the reason. Has anyone done a comparative calibration test yet?

madgrizzle commented 5 years ago

This was a recent error. I recall you having that issue when using your original implementation which I think didn't have this problem.

c0depr1sm commented 5 years ago

Actually, I did somewhat of comparative test: The Zipper Tree Chalenge was done with an alternate triangular inverse function... including that chain tolerance calculation fix.

BarbourSmith commented 5 years ago

And the results speak for themselves. Very impressive!

MaslowCommunityGardenRobot commented 5 years ago

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