NT7S / Si5351

Library for the Si5351 clock generator IC in the avr-gcc environment
GNU General Public License v3.0
34 stars 11 forks source link

Rational approximation is misused #10

Open EU1KY opened 9 years ago

EU1KY commented 9 years ago

In current code, rational_best_approximation() almost always returns the same values of b and c that were passed to its input as rfrac and denom. The function has no numerical space for approximation: it can only perform GCD leaving ratio exactly the same. This approximation is useless and can be totally omitted, leaving the code with noticeable frequency setting errors.

The solution is to multiply both rfrac and denom by 4000ul to give some space for approximation within 32-bit range without overflowing the UINT32_MAX boundary.

And the ultimate, most precise but computationally heavy solution is to use 64-bit rfrac and denom, calculated from frequencies expressed as double floating points, and multiplied with very very big ULL number. Also, uint64_t internals inside rational_best_approximation(). But this way is hardly possible for 8-bit AVR which will do the computations for ages in this case. On ARM Cortex this way works like a charm, providing exceptional frequency setting accuracy.