MaslowCNC / Firmware

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

distPerRot Not Used Anymore and Other Questions #461

Open madgrizzle opened 6 years ago

madgrizzle commented 6 years ago

First, it appears that the software no longer uses distPerRot in any calculations ever since RleftChainTolerance and RrightChainTolerance were introduced. However, we do need a new value that's similar to distPerRot. We need an accurate chain pitch diameter which is calculated as chain pitch / sin(180/number of teeth). I don't know the impact of changing setting names and what it does to eeprom or whatever.

How should we proceed with making the chain pitch diameter available?

BarbourSmith commented 6 years ago

Isn't there a new variable called something like distPerRotR or something like that which computes the distance per rotation without the chain correction factor?

blurfl commented 6 years ago

Keep in mind that while there are different ways to measure the radius depending on whether you're interested in the location of the pin or the link of the chain, the chain that the sprocket stores is affected by the tolerance factor even when it is stored on the teeth as the teeth gather in or release the chain.

madgrizzle commented 6 years ago

Bar, I don't see a disPerRotR.. just distPerRot and it's not used in any routines any longer.

Blurfl, just when I thought it was safe to go back into the water.. I don't know for certain which is correct (to use chain tolerance factors or not) and it's not a big source of error either way. I feel somewhat confident that when we calculate the chain over sprocket, the diameter (and hence radius) we should be using chain chain pitch / sin(180/number of teeth), rather than the current method of using chain pitch number of teeth chain compensation. Maybe we should still use the RleftChainTolerance and RrightChainTolerance values to calculate the chain stored, but the radius.. my head hurts.

BarbourSmith commented 6 years ago

I think I'm thinking of these lines in Settings.cpp:

    sysSettings.distPerRotLeftChainTolerance = 63.5;    // float distPerRotLeftChainTolerance;
    sysSettings.distPerRotRightChainTolerance = 63.5;    // float distPerRotRightChainTolerance;

They were added in #422

I guess maybe my pointing that out was redundant because them being added is the reason distPerRot is not used any more.

Maybe the right place to start is to remove distPerRot, that seems like a safe pull request because if we're not using it it shouldn't be in there

madgrizzle commented 6 years ago

Yeah, I just didn't know if that if just deleting it will screw something up with eeprom.. I don't know how the values are saved to eeprom is all.

blurfl commented 6 years ago

The thing is that the sprocket moves 10 links of chain per rotation. If the chain is stretched or worn, it is still 10 links that are moved. The radius doesn’t matter, but we shouldn’t use either radius to calculate how much chainis moved.

BarbourSmith commented 6 years ago

I agree @blurfl we shouldn't be using the radius, we should be using the number of teeth to make that calculation

Looking at settings.cpp it looks like we write the whole structure directly to the eeprom with 140: EEPROM.put(340, sysSettings); so taking the variable out of the structure would give us trouble, but we should be safe to take it out anywhere else it exists and a comment in the structure that it is a legacy variable would be nice

davidelang commented 6 years ago

On Tue, 21 Aug 2018, Scott Smith wrote:

The thing is that the sprocket moves 10 links of chain per rotation. If the chain is stretched or worn, it is still 10 links that are moved. The radius doesn’t matter, but we shouldn’t use either radius to calculate how much chainis moved.

if the chain is worn/stretched, the sprocket moves it 10 links, and those 10 links are going to measure 6.35mm each, because the sprocket prevents the links from sitting further apart than they were designed to be.

the wear is extra space between the pin and the hole resulting in extra length when under tension, when you are on the sprocket, there is no tension, and the chain is held by the outside, not the force of the pin against the hole.

so the correction factor should only apply to the chain after it leaves the sprocket.

David Lang

madgrizzle commented 6 years ago

distPerRot is still used to set kinematics.R for use in quadrilateral calculations.

c0depr1sm commented 6 years ago

Thanks to madgrizzle's commits in his branch. We can see how this can get in order. I think we also need to clean up to increases readability and that might help more contributors jump in. So I forked, started reviewing the code, changing labels and adding the improvements I see in the MaslowCNC master, as well as preparations by contributors. We'll see where that goes...

BarbourSmith commented 6 years ago

Yes!!!! Clean up and documentation is a GREAT thing to do!