FS#12370 - RDS support for Si4701/Si4703 (Sansa Clip Zip proof-of-concept)

Attached to Project: Rockbox
Opened by Bertrik Sikken (bertrik) - Sunday, 06 November 2011, 12:15 GMT
Last edited by Bertrik Sikken (bertrik) - Friday, 30 December 2011, 00:01 GMT
Task Type Patches
Category FM Tuner
Status Closed
Assigned To No-one
Operating System Sansa AMSv2
Severity Low
Priority Normal
Reported Version Release 3.9
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Attached patch is a proof-of-concept for RDS support for Si4701/Si4703 FM tuners. This makes the RDS station name (8 chars) and the "RadioText" (64 chars max.) show up in the WFMS on the sansa clip zip.

This is done as follows:
* The si4700 fm tuner driver is extended with functionality to attach an interrupt to the RDS-ready GPIO of the si4701/3. This interrupt wakes up a thread that reads the RDS payload from the tuner chip and calls an RDS parser to extract the relevant info. This extension is target-specific.
* A basic RDS parser is added. It aggregates the separate RDS packets into complete RDS messages and keeps a copy of the most recent valid messages. This module is designed to be hardware/target-independent.

Stuff that needs to be done:
* #ifdef the target-specific stuff in the si4700 driver
* #ifdef HAVE_RDS_CAP the RDS-specifc stuff
* Somehow the WFMS shows the most recent RDS info only after pressing a button
* There are now two threads accessing the si4700 hardware: the ui thread through the tuner_get() call, and the rds thread when responding to an RDS event from the si4700.
This task depends upon

Closed by  Bertrik Sikken (bertrik)
Friday, 30 December 2011, 00:01 GMT
Reason for closing:  Accepted
Additional comments about closing:  Latest patch from this task has been committed.
Comment by Michael Sevakis (MikeS) - Sunday, 06 November 2011, 13:24 GMT
Other threads have accessed the tuners in small ways for awhile when changing inputs. They probably should be mutexed as a rule. Same goes for audio drivers.
Comment by Michael Sevakis (MikeS) - Sunday, 06 November 2011, 18:14 GMT
Added support for i.MX31 with a thread for the time being just to see it go. Really hate that flag setting stuff (rds_event) so I took it out :-D -- too much ping pong. FM Radio debug screen shows the info on update instead. Thread sync tuner driver. Proper masking/unmasking of GPIO interrupts to avoid a mad amount of spurious ones while tuner isn't powered. Split things apart into low-level hardware support (preliminary crap in my mind). Probably needs more sensible abstraction yet.

What's odd is 4701 isn't supposed to have RDS according to the docs that I obtained but it works anyway. Go figure!

EDIT: Missed discarding a return point and left the mutex locked in si4700_set. Replaced attachment.
Comment by amaury pouly (pamaury) - Sunday, 06 November 2011, 19:00 GMT
The datasheet says that if too many errors were corrected, one need to discard the packet. Concretly, in si4700_rds_read_raw, one need to read register 0xA and if bits 9:10=0b11 then it means 6+ errors were corrected and the function should return false.
Comment by Michael Sevakis (MikeS) - Sunday, 06 November 2011, 22:42 GMT
This one corrects the mutex init oversight as reasonably as possible and brings back the RADIO_EVENT for now. Still the skin doesn't seem to get prompt updates of it's dynamic fields for RDS on gigabeat S.

@amaury: Are you saying verbose mode is obligatory (needed to read the bits)? I thought it would not pull the line low unless it had a valid packet. I did try that technique and still had errors anyway.

Could there be a standards difference here in the US/Canada region from where you're at? For basic info, I seem to get alot of nonsense or parts of radio text as they arrive.

RDS should probably clear itself periodically or it can get stale if nothing new arrives for awhile (bad reception after good reception for one).
Comment by amaury pouly (pamaury) - Sunday, 06 November 2011, 23:46 GMT
No I misread, the verbose mode is not necessary, it could only bring faster updates at best but would lead to more interrupts so not sure if it's worth enabling it.
Comment by Bertrik Sikken (bertrik) - Saturday, 12 November 2011, 22:06 GMT
Attached file updated the v3 patch with the following:
* correct reset of the rds parser at rds_reset (more thorough)
* simplify the mutexes in si4700 a bit, add a mutex for rds_radio_event
* disable RDS before powering down the tuner (errata for si4703 C19)
* simplify si4700_detect a bit
* add #ifdef HAVE_RDS_CAP around the rds specific bits
Comment by Michael Sevakis (MikeS) - Sunday, 13 November 2011, 05:27 GMT
Perhaps a TUNER_CAPS bitmask define should be used to get rid of any target checking inside for things like the mono/stereo interrupt, RDS and special use of the internal oscillator? Of course, that could be done later /
Comment by Michael Sevakis (MikeS) - Wednesday, 16 November 2011, 07:23 GMT
I forgot to mention you should "#define TARGET_EXTRA_THREADS 1" so that mpegplayer suddenly doesn't fail to initialize. Any patch which has special threads for a target or targets should do that.
Comment by Bertrik Sikken (bertrik) - Thursday, 17 November 2011, 22:47 GMT
Attached patch adds the TARGET_EXTRA_THREADS and also fixes the si4700_st prototype in the gigabeat-s fmradio interface code.
Comment by Bertrik Sikken (bertrik) - Sunday, 20 November 2011, 23:03 GMT
Patch update:
* Simplify return value from rds_process from an enum to a bool (maybe we can differentiate later between ps and rt updates)
* Update radio.c so the check for a radio event is moved out of the switch-case which handles buttons, this makes RDS update as soon as it comes in (instead of requiring a button press), I'm not really familiar with the radio code, so please review this.
* Report RDS only if a new message comes in that is actually different from the previous one. This prevents identical incoming RDS messages from resetting the scrolling on the radio text.
Comment by Michael Sevakis (MikeS) - Sunday, 20 November 2011, 23:20 GMT
The RDS tags are supposed to be dynamic but that doesn't seem to be working properly on the normal refresh. Really I think that update flag stuff shouldn't be required. It didn't help with much of anything.
Comment by Bertrik Sikken (bertrik) - Friday, 09 December 2011, 15:27 GMT
RDS support has now been ready for several weeks, but seems to be stuck on the issue that the RDS station name and text isn't getting updated automatically in the WFMS.
The update flag stuff was there before (used also by the ipod fm remote) and I don't want to add another dependency (figuring out whether we need the update flag or not) before we can get RDS in.

I propose to keep the update flag for now, make the RDS tags static (since they don't seem to work as dynamic and nobody's looking at this issue anyway) and force a SKIN_REFRESH_ALL in case the RDS flag indicates new RDS data.

I'm fine with any changes you wish to make later to the update flag mechanism, but at least let's get it in in some working shape.
Comment by Michael Sevakis (MikeS) - Friday, 09 December 2011, 18:28 GMT
Comment by Jonathan Gordon (jdgordon) - Wednesday, 14 December 2011, 02:05 GMT
hehe, someone yelled? :)
I fail to see why the skins not updating correctly is a blocker for this. commit and open a bug.
I'm not even sure if radio stations out here provide RDS so even if i had hardware im not sure i could debug. Does the sim show the same broken behaviour?
Comment by Jonathan Gordon (jdgordon) - Wednesday, 14 December 2011, 02:10 GMT
I also suspect that the %s in the fms on the rds lines is breaking it. skins don't update dynamic text very often.. try removeing them from apps/radio/radio+skin.c lines 57,58
Comment by Rafaël Carré (funman) - Wednesday, 14 December 2011, 02:34 GMT
I have 3 stations broadcasting their names, but there's no dynamic content.
Comment by Michael Sevakis (MikeS) - Wednesday, 14 December 2011, 03:48 GMT
"Not very often" seemed like minutes to me yet we have a nice, quick stereo/mono indicator.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 14 December 2011, 12:54 GMT
r31247 should unblock you :)
Comment by Bertrik Sikken (bertrik) - Saturday, 17 December 2011, 16:23 GMT
I plan to commit this shortly, to get a basic working framework in SVN, with minor issues to be solved later.