sparkfun / Si4703_FM_Tuner_Evaluation_Board

Example code and board files for the Si4703 FM Tuner Evaluation Board
Other
41 stars 31 forks source link

Correctly implemented the seek/tune interrupt handling. #14

Open cmumford opened 4 years ago

cmumford commented 4 years ago

The prior implementation was never correctly checking the interrupt pin (_stcIntPin) when seeking or tuning to a new channel. It was only checking that the pin variable, which contains the GPIO pin number, was non-zero - never the actual state of the pin. This resulted in the the code immediately concluding that the seek/tune operation was complete, that is unless _stcIntPin was set to zero, whereby the device would spin until reset by the Arduino watchdog timer.

This implementation installs an interrupt handler to properly detect that the Si4703's GPIO2 pin has been set low. This change also allows _stcIntPin to be set to a value of -1 to disable interrupt handling. The implementation will then poll the tuner to detect that seek/tune has completed. This might be desireable when the user does not want to connect the Si4703 GPIO2 pin to the controller (for whatever reason).

abraxa commented 7 months ago

Thanks for the change, I'm trying to use it for my project. Unfortunately, your change causes some issues.

For example, ICACHE_RAM_ATTR is not a valid attribute on the RP2040 Arduino platform and several warnings are generated which stem from naming issues. Especially

SparkFunSi4703.cpp: In member function 'void Si4703_Breakout::readRDS(char*, long int)': SparkFunSi4703.cpp:135:45: warning: left shift count >= width of type [-Wshift-count-overflow] 135 | if(si4703_registers[STATUSRSSI] & (1<<RDSR)){

stands out because your new declaration of "#define RDSR 0x8000 // (1 << 15)" conflicts with "static const uint16_t RDSR = 15;". If you're still interested in getting the PR merged, I think it would make sense to fix these.

cmumford commented 7 months ago

It's been almost four years, was submitted before the RP2040 was released, and I don't have Arduino installed anymore. You're welcome to close this PR if you want.