XRobots / openDog

CAD and code for each episode of my open source dog series
GNU General Public License v3.0
830 stars 189 forks source link

The attempt to avoid dividing by zero is incorrect #15

Open tspink opened 6 years ago

tspink commented 6 years ago

https://github.com/XRobots/openDog/blob/98050a469b15a26739f628e98b33c294fa8206f7/Part11/Dog011/Dog011.ino#L391

The check you have on Line 389, to avoid dividing by zero on Line 391 is incorrect. You are ensuring that the dividend (i.e. the number to be divided, X) is not zero, when you should be ensuring that the divisor (i.e. the number that divides the dividend, Z) is not zero.

Therefore the check should be to ensure that Z does not become zero. There is nothing wrong with the dividend (in this case X) becoming zero. As the X approaches zero, so does the value of the fraction - which is correct.

I note that Line 395 then uses the result of the hipHypotenuse, so it is this division that may need to be protected.

(Note: this error is made in multiple places)

XRobots commented 6 years ago

That's not where the divide by zero occurs. Z is never zero, but X and Y are zero in the centre of the travel for those axis, which means zero gets passed into the rest of the code and the trig fails elsewhere.

tspink commented 6 years ago

OK, that's fair enough. Maybe just extend the comment to explain this, as it's not immediately clear that's what the guard is for!

Following the flow of X, in the example I linked, means that if X is zero, hipAngle1 becomes zero, then sin(hipAngle1) would be zero, leading to a divide-by-zero on the hipHypotenuse calculation.

Thanks for clarifying! Tom