mathertel / Radio

An Arduino library to control FM radio chips like SI4703, SI4705, RDA5807M, TEA5767.
http://mathertel.github.io/Radio
BSD 3-Clause "New" or "Revised" License
300 stars 91 forks source link

Proposal for Issue 5 #17

Closed DerKleineLeif closed 4 years ago

DerKleineLeif commented 6 years ago

Register for CTRL within seek function is not cleared if bool toNextSender is "true". Proposed solution: "registers[RADIO_REG_CTRL] &= (~RADIO_REG_CTRL_SEEK);" should be outside (before) if-statement.

wmarkow commented 6 years ago

I took a look at the proposed fix. I have retested it with my project. It works nice. I think it is a good fix because its logic is implemented in the seekUp and seekDown functions. Even those calls:

registers[RADIO_REG_CTRL] &= (~RADIO_REG_CTRL_SEEK);

in setBassBoost, setMono and setMute are not needed anymore with this fix.

The bit SEEK of register 02H (RADIO_REG_CTRL) is cleared anyway (according to the specs) after the search is completed. My concern was that this 02H register could be read back somehow from the chip (while the chip is in SEEKing state), so the SEEK bit would be anyway set back to one; then we would have the issue again. But according to my static source code analysis looks, like 02H (RADIO_REG_CTRL) register is never read back from the chip. Please correct me if I'm wrong.

wmarkow commented 6 years ago

One more thing, @DerKleineLeif, in your proposal in seekUp method you clear RADIO_REG_CTRL_SEEK flag twice: once before if and second inside of if. I believe that the second clear is not necessary.

DerKleineLeif commented 6 years ago

Hello @wmarkow,

you're right in seekUp register is been cleared twice which is some kind of unnessecary - my fault. A more clean way to handle the registers would be to first read the register from chip, then modify and write back. With this the chance that registers in memmory and on chip will diverge would go down.

For me it's just to much modification on this topic.

Thank you for your feedback

Cheers! Leif