Rockbox

Tasklist

FS#5347 - FM radio for X5: almost there

Attached to Project: Rockbox
Opened by Rani Hod (RaeNye) - Thursday, 11 May 2006, 23:33 GMT
Task Type Patches
Category FM Tuner
Status Closed
Assigned To No-one
Operating System iAudio X5
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

It appears that the Philips FM tuner driver works for the D&A 202 too;
Only missing is the part that calls the audio multiplexer and enables "recording" on the TLV320.

(So how do I know it's working at all? Since I can scan for FM presets and it locks on actual stations!)

Note that the patch might not compile cleanly against current cvs since I changed many files and only uploaded those relevant to this patch. YMMV.
This task depends upon

Closed by  Linus Nielsen Feltzing (linusnielsen)
Friday, 21 July 2006, 08:43 GMT
Reason for closing:  Accepted
Additional comments about closing:  I fixed the context menu bug and committed the patch. Thanks for your patience.
Comment by Rani Hod (RaeNye) - Friday, 12 May 2006, 18:49 GMT
OK, now I can hear the radio as well.
Volume doesn't change, though.
Comment by Tahn Obu (Tobu) - Monday, 15 May 2006, 01:17 GMT
Apparently the patch doesn't use the unified format all the way; remember to pass -u so that it can apply.
Comment by Rani Hod (RaeNye) - Sunday, 09 July 2006, 23:20 GMT
Resynced to current CVS, also cleaned up a little.

Note that this patch now includes the recording patch v2 (FS #5355) as a part of it (both patches touch the very same things, so it would be a PITA to apply one on top of the other anyway). Thanks Andy.

Known bugs:
1. If you exit the FM radio by visiting the main menu and exiting it rather than by pressing the POWER button, the FM mux is still on so you won't hear the digital audio. To fix this, exit FM mode properly.
2. The context menu (long SELECT) seems to automatically select the first menu item when enterring it. It is a result of making SELECT button function as an alternate MENU_ENTER button, and I don't know how to fix this. To circumvent it, do a long SELECT press and without releasing the joystick, press DOWN (or UP). Ugly but usable.
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 11 July 2006, 09:09 GMT
Good work!

It was a little hard to review the patch, since you have made lots of irrelevant changes to the code, especially whitespace changes in i2c.c and pcm_record.c. Could you please update your patch to include only relevant changes?
Comment by Rani Hod (RaeNye) - Tuesday, 11 July 2006, 12:46 GMT
Actually i2c-coldfire.c has changed quite a bit, so it's better to view it as a whole. The whitespace changes are minor.
The changes in pcm_record.c are from the recording patch, so I just synced them.

However, I manually edited the .patch to remove the ~10 affected lines. I didn't check it still applies correctly, as I'm not on my env.
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 11 July 2006, 13:02 GMT
The whole idea of CVS is to track the changes, so it's still important to only change the relevant stuff. I'm not really sure why you feel compelled to change the indentation in i2c-coldfire.c for no reason. If you had left the indentation as it was, it would have been much easier to see the changes.

Besides, the CONTRIBUTING file says that you should keep the formatting when you edit a file, which means that you should not change the indent level.
Comment by Rani Hod (RaeNye) - Tuesday, 11 July 2006, 16:32 GMT
But I changed the entire i2c-coldfire implementation!
I'm not compelled to maintain indentation level when /rewriting whole functions/, do I?
Comment by Dave Chapman (linuxstb) - Tuesday, 11 July 2006, 16:47 GMT
I also prefer two space indents, but the CONTRIBUTING file compels us all to indent with four spaces:

"Always indent your code with four spaces. Don't use TAB characters, as that will mess up code display in CVS, printing, and a zillion other places."
Comment by Rani Hod (RaeNye) - Wednesday, 19 July 2006, 01:14 GMT
Ready for commit:
- resynced to CVS
- tabs eliminated (I still have to figure out why vi puts tabs in it)
- tested compilation on x5, h1xx, h3xx, ipod photo and archos player

Loading...