mathertel / RotaryEncoder

RotaryEncoder Arduino Library
Other
342 stars 107 forks source link

Added the ability to obtain the encoder RPM. #22

Closed rprediger closed 3 years ago

rprediger commented 3 years ago

Added the ability to obtain the encoder RPM. Added verification of time between steps in case of encoder stop

mathertel commented 3 years ago

Thanks for this getRPM function. The change also changes the behavior of the existing getMillisBetweenRotations() function and potentially breaks compatibility.

After merging I will re-establish the old getMillisBetweenRotations implementation and move the new implementation into getRMP.

rprediger commented 3 years ago

Glad to help! I just realized that the number of steps is understood which limits the library. I am unaware of other sensors with different number of steps, but I believe there must be. I will make this correction and send it to you, if you wish.

mathertel commented 3 years ago

I did it. Just take the head version.

Llaves commented 3 years ago

maybe it's a language issue, but in the US, RPM is revolutions per minute, ie, full turns. As implemented, the function is returning ticks per minute. To get RPM (at least as I interpret the term), you need to know how many ticks per revolution the encoder generates and divide the computed value by that number.

rprediger commented 3 years ago

maybe it's a language issue, but in the US, RPM is revolutions per minute, ie, full turns. As implemented, the function is returning ticks per minute. To get RPM (at least as I interpret the term), you need to know how many ticks per revolution the encoder generates and divide the computed value by that number.

Hello @Llaves thanks for your question, but reviewing the code I am still convinced that the function performs the calculation correctly. The function calculates the RPM (revolutions per minute) based on the time of the last step. However, if the pulses have ceased, the function assumes the time between the last tick and the current time to calculate the RPM. The function also assumes that the encoder has 20 pulses per revolution. For example: If the last pulse took 25ms then one rotation takes 25ms 20 pulses = 500ms = 0.5s (period) To transform the period into revolutions per second (RPS or frequency), it is done (1 / period) and finally to transform RPS into RPM it is multiplied by 60. RPS = 1 / 0.5 = 2; RPM = 2 60 = 120; NOTE: As time is measured in milliseconds, it is necessary to convert to seconds, so the calculation looks like this: RPM = (1 / ((last tick time 20) / 1000)) 60; Making the simplifications we have: RPM = 60000 / (last tick time * 20);

Llaves commented 3 years ago

The concern I have with this code is that it contains a magic number - 20. Not every encoder has 20 ticks per rev. The dirt-cheap common ones used in many Arduino projects do have 20 counts, but that's no always the case. It would be better to make this a member variable of the class, include a setter, and perhaps initialize to a default value of 20.

rprediger commented 3 years ago

Hello @Llaves, I have already expressed my concern about this here:

Glad to help! I just realized that the number of steps is understood which limits the library. I am unaware of other sensors with different number of steps, but I believe there must be. I will make this correction and send it to you, if you wish.

And @mathertel said that he had already solved it here:

I did it. Just take the head version.

So I didn't want to make any other suggestions related to that.

Llaves commented 3 years ago

Sorry - I should have read the thread more carefully. But now I'm confused. @mathertel says it's solved in the head version. There is only a single branch on github, the master branch. In this branch the magic number 20 still appears. Am I looking in the wrong place? The last commit for the .cpp file is Jan 29. Your comment about number of steps was posted on Jan 30, as was the post that it was corrected.

Llaves commented 3 years ago

@rprediger wrote:

I am unaware of other sensors with different number of steps, but I believe there must be.

This style of encoder is commonly used in CNC controllers for manually jogging the machine. It has 100 pulses per rev.

This style can be used for high accuracy sensing of shaft position (where "high" is a relative term.) It generates 600 pulses/rev.