HamRadio360 / Antenna-Analyzer

K6BEZ Antenna Analyzer Build Project
33 stars 18 forks source link

SetDDSFreq calculation ambiguous #7

Open JohnDSutter opened 7 years ago

JohnDSutter commented 7 years ago

On Arduino 1.6.5 it consistently gets the wrong answer giving me a nice ~1Hz signal. The following is a bit more explicit:

void SetDDSFreq(long Freq_Hz) { // Calculate the DDS word - from AD9850 Datasheet uint32_t f = (uint32_t) (((uint64_t)Freq_Hz * 4294967295ULL)/125000000ULL);

dkozel commented 7 years ago

Hi @JohnDSutter. Thanks for the bug report. I should be able to check this this weekend.

etchorner commented 7 years ago

This may be IDE version dependent. On 1.8.1, this bug does not exist. A user on the HR360 forum points out that this bug is likely the result of variable overflow, in the const arithmetic of SetDDSFreq, where the 4294967295 const is already at the limit of a 32 bit variable.

An alternative to that of @JohnDSutter would be to replace the const division term (4294967295/125000000) with the const 34.35973836.

JohnDSutter commented 7 years ago

There is a difference between

(Freq_Hz * 4294967295)/125000000

and

Freq_Hz (4294967295/125000000) => Freq_Hz 34

when using integers

Band

Center Frequency

(Freq_Hz * 4294967295)/125000000

Freq_Hz * (4294967295/125000000)

Actual Frequency

Error (kHz)

% from Center

160

1900000

65283503

64600000

1880107.448

19.89

20%

80

3750000

128849019

127500000

3710738.384

39.26

16%

40

7150000

245672129

243100000

7075141.186

74.86

50%

30

10125000

347892351

344250000

10018993.64

106.01

141%

20

14175000

487049291

481950000

14026591.09

148.41

85%

10

28850000

991278452

980900000

28547947.3

302.05

86%

6

52000000

1786706395

1768000000

51455572.26

544.43

27%

From: Christopher Horner [mailto:notifications@github.com] Sent: Friday, February 03, 2017 1:05 PM To: HamRadio360/Antenna-Analyzer Cc: John Sutter; Mention Subject: Re: [HamRadio360/Antenna-Analyzer] SetDDSFreq calculation ambiguous (#7)

This may be IDE version dependent. On 1.8.1, this bug does not exist. A user on the HR360 forum points out that this bug is likely the result of variable overflow, in the const arithmetic of SetDDSFreq, where the 4294967295 const is already at the limit of a 32 bit variable.

An alternative to that of @JohnDSutter https://github.com/JohnDSutter would be to replace the const division term (4294967295/125000000) with the simple integer 34. Not all version of the IDE may support uint64_t or long long casts for const arithmetic.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HamRadio360/Antenna-Analyzer/issues/7#issuecomment-277361981 , or mute the thread https://github.com/notifications/unsubscribe-auth/ABDiiAwVc6nVOGBar_2BPveamVH2Fh81ks5rY5ZpgaJpZM4LpB0X . https://github.com/notifications/beacon/ABDiiJAorFL2uKqjgo9uUDaa5JgzUQ-Gks5rY5ZpgaJpZM4LpB0X.gif

etchorner commented 7 years ago

@JohnDSutter

Agreed. I think your comment and my edit to my earlier comment crossed in ether. I was having a related discussion on the HR360 forum with a fellow who pointed out the error in my integer multiplier suggestion. My (amended) suggestion is to use 34.35973836 as the tuning word multiplier. Conceptually, the 64 bit cast method you propose may likely work as well, but I'm running 1.8.1 not 1.6.5 so I cannot test it here.

r/ Chris

JohnDSutter commented 7 years ago

If there's room for a floating point library that's just as good.

The difference is negligible.

Thanks,

From: Christopher Horner [mailto:notifications@github.com] Sent: Friday, February 03, 2017 2:08 PM To: HamRadio360/Antenna-Analyzer Cc: John Sutter; Mention Subject: Re: [HamRadio360/Antenna-Analyzer] SetDDSFreq calculation ambiguous (#7)

@JohnDSutter https://github.com/JohnDSutter

Agreed. I think your comment and my edit to my earlier comment crossed in ether. I was having a related discussion on the HR360 forum with a fellow who pointed out the error in my integer multiplier suggestion. My (amended) suggestion is to use 34.35973836 as the tuning word multiplier. Conceptually, the 64 bit cast method you propose may likely work as well, but I'm running 1.8.1 not 1.6.5 so I cannot test it here.

r/ Chris

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HamRadio360/Antenna-Analyzer/issues/7#issuecomment-277375663 , or mute the thread https://github.com/notifications/unsubscribe-auth/ABDiiNpMC-PcytECVkgZ697R82QF3Gweks5rY6VJgaJpZM4LpB0X . https://github.com/notifications/beacon/ABDiiMpPZdR8TGG6wwbaovUsBzGvudclks5rY6VJgaJpZM4LpB0X.gif